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

reset active element/focus order state on ui close #158

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

csmccarthy
Copy link
Member

Related Issues

Changelog

This solves a really weird (browser specific) accessibility bug, wherein Chrome users relying on tab focus order for navigation would be routed to the URL bar after modal closure instead of the first interactive/tabindexed element on the page. Various solutions were attempted, such as focusing the document body and blurring the activeElement, but there appeared to be hidden focus order state that wasn't being reset upon a simple element focus call. The only solution I found to work was creating a dummy interactive element with maximum focus priority (i.e. tabindex of 1, at the very start of the body), then focusing and removing it. This essentially puts the focus order pointer at the very start of the list so the next tab will focus the first interactive element just as it would on a fresh page load.

Copy link

height bot commented Feb 23, 2024

This pull request has been linked to and will mark 1 task as "Pending Deploy" when merged:

💡Tip: You can link multiple Height tasks to a pull request.

Copy link

vercel bot commented Feb 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
consent-manager-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 26, 2024 6:14pm

@linh-transcend
Copy link
Member

prob a silly question but from an accessibility standpoint, why is not going to first interactive/tab indexed element on the page not good?

tempInteractiveEl.setAttribute('tabindex', '1');
document.body.prepend(tempInteractiveEl);
tempInteractiveEl.focus();
tempInteractiveEl.remove();
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe a note to describe why this is needed?

also, i'm sorta confused on the last two steps? the temp element is being focused on, so that brings focus to the new temp element for the first interactive/tab indexed element on the page? why then remove? is it so the focus can then be shifted around by the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh youre so right about the comment, i should have added one originally.

basically this function just "resets" focus order state, so that the next time the user presses tab it will go to the first interactive element on the page like it would on page load. we remove it because once its reset the focus state it no longer serves a purpose, and bc if we didnt remove it then we would leave a useless html element lying around in the document body which is super messy

Copy link
Member

Choose a reason for hiding this comment

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

ooooh thanks for explaining the removal!

@linh-transcend linh-transcend requested a review from a team February 26, 2024 15:12
Copy link
Member

@linh-transcend linh-transcend left a comment

Choose a reason for hiding this comment

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

I'm not the best to review but this gives off good code vibes!

Comment on lines +131 to +136
/* If the user previously had something focused before the modal popped up, then focus it.
* Otherwise, we have to do this really janky "reset" behavior with the temporary element
* because Chrome specifically has some weird internal state around focus order that is
* very difficult to interact with. We create an element with maximum focus priority and
* focus it so that when we delete it the user will be at the start of the focus order
* just like if they had freshly loaded the page. */
Copy link
Member

Choose a reason for hiding this comment

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

nice!

@csmccarthy csmccarthy merged commit 6c7bc40 into main Feb 26, 2024
10 checks passed
@delete-merged-branch delete-merged-branch bot deleted the csmccarthy/focus-order-reset branch February 26, 2024 18:20
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