Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#3052 Confirmation page for partners requests #3638

Open
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

lokisk1155
Copy link
Collaborator

@lokisk1155 lokisk1155 commented Jun 4, 2023

Resolves #3052

What I did

I made confirmation pages for Essentials Requests for request types: quantity, child, and # of individuals

I followed a similar pattern for both individual and quantity requests using a link tag passing private params to make the request from the confirmation page

For family requests I had to create a hidden form to pass the the child_id's

new version

Screen Shot 2023-07-08 at 10 43 56 AM Screen Shot 2023-07-08 at 10 41 34 AM Screen Shot 2023-07-08 at 10 41 15 AM

previous version

Screen Shot 2023-06-27 at 3 07 17 PM Screen Shot 2023-06-27 at 3 06 56 PM Screen Shot 2023-06-27 at 3 06 41 PM

@lokisk1155 lokisk1155 changed the title [WIP] #3052 Confirmation page for partners requests #3052 Confirmation page for partners requests Jun 5, 2023
@lokisk1155 lokisk1155 changed the title #3052 Confirmation page for partners requests [WIP]#3052 Confirmation page for partners requests Jun 5, 2023
Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some typos. The UI looks OK to me, although I'd be interested to hear what other feedback is forthcoming :)

app/controllers/partners/requests_controller.rb Outdated Show resolved Hide resolved
@lokisk1155 lokisk1155 requested a review from dorner July 8, 2023 14:36
@lokisk1155
Copy link
Collaborator Author

lokisk1155 commented Jul 8, 2023

@dorner Going to throw screenshots up shortly, made the UI have a better visual hierarchy from Daniel's advice

Left the screenshots of the original version up, let me know which one you like better

@lokisk1155 lokisk1155 changed the title [WIP]#3052 Confirmation page for partners requests #3052 Confirmation page for partners requests Jul 8, 2023
@dorner
Copy link
Collaborator

dorner commented Jul 9, 2023

@cielf thoughts? Now that I look at it, I think the "are you sure?" question and the "please confirm this is what you meant to request" seem redundant.

@cielf
Copy link
Collaborator

cielf commented Jul 29, 2023

I agree that it's redundant. From the screenshots, I'd say to take out the "Please confirm...." I haven't kicked the tires yet.

@lokisk1155
Copy link
Collaborator Author

Screen Shot 2023-07-31 at 8 45 03 AM Ok I removed it, final version

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cielf did you want to "kick the tires"?

@cielf
Copy link
Collaborator

cielf commented Aug 4, 2023

@lokisk1155 See this screenshot, from quantity request. There should be a number in "You have requested a total of items".
Screenshot 2023-08-04 at 6 19 08 PM

@cielf
Copy link
Collaborator

cielf commented Aug 4, 2023

And when I hit "go back", it only showed the value that was entered for the first item, which -- and not the total values I entered for that (I had entered 5 L/XL, 15 premie, and an additional 20 L/XL).
Screenshot 2023-08-04 at 6 23 36 PM

The same problem happens on individual.

@cielf cielf self-requested a review August 4, 2023 22:30
@cielf
Copy link
Collaborator

cielf commented Aug 4, 2023

For the child request, can we get the list in alpha order by child (last name, first name), then item name? Will be much easier to check for them
Screenshot 2023-08-04 at 6 26 49 PM

@cielf
Copy link
Collaborator

cielf commented Aug 4, 2023

On Individual, at least, the totals aren't actually totalling, if I put two lines for the same item in.

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lokisk1155 Please address the comments above, then I'll take another look. Thanks

@lokisk1155
Copy link
Collaborator Author

lokisk1155 commented Aug 5, 2023

Screen Shot 2023-08-05 at 1 06 30 PM

@dorner @cielf I ended up refactoring how family_requests is working; I noticed that I was having issues requesting multiple items for the same child -> led to a hard refactor. / is this how it should actually work

Aside from that, I cleaned up a majority of the issues @cielf mentioned. Just need to fix the quantity and individual back button - taking a break but hopefully fixed today

@cielf
Copy link
Collaborator

cielf commented Jan 9, 2024

@lokisk1155 Is this back ready for review? -- it's not 100% clear.

@lokisk1155
Copy link
Collaborator Author

@cielf no I don't think so I gotta get back on it

@cielf
Copy link
Collaborator

cielf commented Jan 14, 2024

@lokisk1155 Please do -- we're going to be making a concentrated effort to get the pile of unresolved PRs resolved once the two big things we are working on at the moment are done -- at that point if there's no action within 2 weeks, we'll likely just take it over -- it'd be nice to have you see it through, though.

@dorner
Copy link
Collaborator

dorner commented Apr 5, 2024

@lokisk1155 can you confirm that you're not going to pick this one up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a confirmation page to partner requests.
3 participants