hathitrust / firebird-common

Other
0 stars 0 forks source link

DEV-1177: finish up feedback form fixes #88

Closed carylwyatt closed 4 months ago

carylwyatt commented 4 months ago

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, and FeedbackFormContent/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 is true, 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.

netlify[bot] commented 4 months ago

Deploy Preview for hathitrust-firebird-common ready!

Name Link
Latest commit d6ff376653b8b3ebc2a9be683bdbdbd3209161ce
Latest deploy log https://app.netlify.com/sites/hathitrust-firebird-common/deploys/668310ec3939210008e353e8
Deploy Preview https://deploy-preview-88--hathitrust-firebird-common.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

carylwyatt commented 4 months ago
  1. ESC to close the modal should be default behavior for any modal. I'm surprised Deque didn't catch that! I'll open a new issue.
  2. This is related to another known "state" issue in the feedback forms: if you submit a form with errors in it, you get some messaging about incorrect form fields. If you close the form and open it again, you would expect to see a blank form, but instead, you get a form with "incorrect field" error messages. Resetting the form when the modal is closed seems like a good idea to me, too. Another issue to open!

Thanks for the review! ๐Ÿ™Œ

carylwyatt commented 4 months ago

@giramesh @angelinanz DEV-1177 fixes are now staged on test.www.hathitrust.org and ready for final review!

giramesh commented 4 months ago

Issues: 1727230 | 1727229 -tested the fixes and I can confirm that these are solved!