-
Notifications
You must be signed in to change notification settings - Fork 0
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
DEV-1177: finish up feedback form fixes #88
Conversation
✅ Deploy Preview for hathitrust-firebird-common ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
I could see the autocomplete attributes set correctly, as promised. The values I enter in the form modal are remembered until I submit the form or navigate to another page. Are they supposed to pre-populate if I'm logged in? If so, did not check that.
Focus is returned to GET HELP
when I close or submit the form modal, as promised. This worked well for me whether navigating with mouse, keyboard, or screen reader.
I have 2 bits of feedback, neither of which block approval:
1: As a user relying on keyboard/screen-reader, I think it would be neat if the form modal would close on ESC
, rather than having to tab back to the beginning of the form to close it. This might be accessibility-heresy, so take that as the personal preference it is.
2: When I've submitted my question/feedback, the form modal goes into a "submitted, start over?" state. Fine. But if I go to submit another question/feedback the modal pops up in this "submitted, start over?" state again. It would make more sense if the form reset if I've dismissed it once.
That said, looks good, APPROVE.
Thanks for the review! 🙌 |
@giramesh @angelinanz DEV-1177 fixes are now staged on test.www.hathitrust.org and ready for final review! |
The last couple fixes for the feedback form and feedback modal
issue 1727230: identify input purpose missing for name and email
All three feedback forms were missing "autocomplete" HTML attributes on name and email fields, so I added them. See
FeedbackFormBasic/index.svelte
,FeedbackFormCatalog/index.svelte
, andFeedbackFormContent/index.svelte
.To test: on dev-3, open the feedback form using the GET HELP dropdown in the navbar. Use dev tools to inspect the name input element and see the autocomplete attribute set to "name". The email input element has the autocomplete attribute set to "email".
issue 1727229: when modal is closed, focus isn't returned to trigger
When any modal is closed, in order to remain accessible, the browser/user's focus should be returned to the element that triggered the opening of the modal. In this case, the triggering element is in the navbar dropdown under GET HELP. The feedback form opens when a user clicks "Ask a Question" or "Report a Problem." I originally fixed this so that the focus would return to either of those links, but the fix included not-best-practices javascript in order to reopen the dropdown that would normally close on-click. Deque says it's fine to return the focus to the dropdown link element (GET HELP) instead of the actual triggering element, so I undid all that junk and did something much simpler: added a boolean prop
focusHelpOnClose
to the FeedbackFormModal component to pass down to the Modal component. If that value istrue
, the focus is returned to GET HELP when the modal is closed.To test: on dev-3 (or locally if you want to pull down the branch), use your keyboard to navigate through the navbar to "GET HELP." Hit enter to toggle the dropdown, then select either of the feedback forms. The dropdown should close, a modal should open. Now hit enter again to close the modal (the close button should be auto-focused), and see that the focus ring returns to the GELP HELP link. You can also check that the login modal has similar behavior.