-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Conversation
…rrors from the model validations - do I add my own error handling?
… don't (caybara error when i do)
@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 |
@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. |
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. |
There was a problem hiding this 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"?
@lokisk1155 See this screenshot, from quantity request. There should be a number in "You have requested a total of items". |
On Individual, at least, the totals aren't actually totalling, if I put two lines for the same item in. |
There was a problem hiding this 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
@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 |
@lokisk1155 Is this back ready for review? -- it's not 100% clear. |
@cielf no I don't think so I gotta get back on it |
@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. |
@lokisk1155 can you confirm that you're not going to pick this one up? |
Completed with #3052 . |
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
previous version