Description
We are currently using Dialog from primer/react/drafts for our symbols pane on narrow viewports. It works pretty well but we have a problem where clicking on a symbol will scroll the page down/up to a specific line but then once it closes, focus will move back to the symbols button and the scroll position resets to the top of the page. This is because Dialog.js passes the option restoreFocusOnCleanup to the useFocusTrap and in the cleanup, useFocusTrap calls previousElement.focus() .
There's currently no way for us to fix this problem on our own as far as I can tell - we'd need to be able to pass in a prop to not restore focus on change & handle the focus ourselves by assigning focus to the symbol that the user scrolled to. Here's a discussion with a link to a video of the problem https://github.com/orgs/community/discussions/45280
We sent a message in #repos-accessibility and were told that returning focus to the symbol the user selected, not the symbol button, would be the correct accessibility choice.
Steps to reproduce
- Go to https://github.com/github/github/blob/master/packages/repositories/app/models/search/queries/repo_query.rb
- Open devtools and reduce screen width until < 500px
- Click the symbols button to open the symbols dialog
- Click on the last symbol in the list (
topic_not_queries) & click on the definition on line 531
- Notice the content scrolls behind the dialog
- Click the
x button in the top right corner to close the dialog
- Notice that you were scrolled to the top of the page
Version
35.18.0
Browser
Chrome, Safari, Firefox, Edge, iOS Safari
Description
We are currently using Dialog from
primer/react/draftsfor our symbols pane on narrow viewports. It works pretty well but we have a problem where clicking on a symbol will scroll the page down/up to a specific line but then once it closes, focus will move back to the symbols button and the scroll position resets to the top of the page. This is because Dialog.js passes the optionrestoreFocusOnCleanupto theuseFocusTrapand in the cleanup,useFocusTrapcallspreviousElement.focus().There's currently no way for us to fix this problem on our own as far as I can tell - we'd need to be able to pass in a prop to not restore focus on change & handle the focus ourselves by assigning focus to the symbol that the user scrolled to. Here's a discussion with a link to a video of the problem https://github.com/orgs/community/discussions/45280
We sent a message in #repos-accessibility and were told that returning focus to the symbol the user selected, not the symbol button, would be the correct accessibility choice.
Steps to reproduce
topic_not_queries) & click on the definition on line 531xbutton in the top right corner to close the dialogVersion
35.18.0
Browser
Chrome, Safari, Firefox, Edge, iOS Safari