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

issue #89: add event listener for escape key #90

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

carylwyatt
Copy link
Member

@carylwyatt carylwyatt commented Jul 3, 2024

Re: @mwarin's suggestion in PR #88

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.

I did some testing and found an additional bug: my original fix returned the focus to the GET HELP link when you use the CLOSE button on the modal, but the ESC key returned the focus back to the body element.

This event listener (with a few .focus() methods thrown in for fallback/good measure) picks up on ESC keys and invokes the hide() function (which sets the focus back to GET HELP).

This closes issue #89.

To test: head over to dev-3 and tab through the nav menu to GET HELP. Select with ENTER and tab down to Ask a Question. Hit ENTER again to open the modal. Hit ESC to close modal. Focus should be returned to GET HELP in navbar. If you tab through some fields in the form, you have to hit ESC to exit the form field, then ESC again should close the modal and return focus to GET HELP.

Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for hathitrust-firebird-common ready!

Name Link
🔨 Latest commit 736309f
🔍 Latest deploy log https://app.netlify.com/sites/hathitrust-firebird-common/deploys/66859bf920a4220008fc2584
😎 Deploy Preview https://deploy-preview-90--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 carylwyatt requested a review from mwarin July 19, 2024 20:29
Copy link

@mwarin mwarin left a comment

Choose a reason for hiding this comment

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

This now behaves the way I'd expect, which makes it flow so much nicer using screen reader.

APPROVE.

@carylwyatt carylwyatt merged commit b2fe7e1 into main Jul 25, 2024
10 checks passed
@carylwyatt carylwyatt deleted the DEV-1177-feedback-form branch July 25, 2024 13:40
@carylwyatt
Copy link
Member Author

carylwyatt commented Jul 26, 2024

@angelinanz @giramesh This has already been marked as "fixed" with deque because we passed their benchmark, but I agreed with @mwarin's assessment about the ESC key for the feedback form modal. This is staged on test.www.hathitrust.org if you want to give it a spin!

I'll deploy on monday.

@giramesh
Copy link

can confirm that the ESC key for the feedback form modal works as expected on chrome, firefox, safari, edge!
initially on chrome and edge i had to hit 'ESC' twice when in the form field (as the testing instructions mentioned), but after a few tries, just hitting 'ESC' once did the job.

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.

3 participants