Skip to content

Desktop: Accessibility: Restore keyboard focus when closing a dialog #10817

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

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Aug 2, 2024

Summary

This pull request improves how the Dialog component handles focus by closing the dialog with the .close() method before removing it from the DOM. This causes the browser to restore keyboard focus to the item focused before the dialog was opened.

Previously, closing a dialog reset focus to the first focusable element on the page.

See also:

Relevant WCAG success criterion

This change is related to success criterion 2.4.3 (Level A). See failure condition 85, example 2.

Remaining issue

Note

Update: A workaround has been implemented for input elements. However, the issue is still present in the Rich Text Editor.

On some platforms (observed on Ubuntu 24.04 and MacOS), if an input is granted focus after closing a dialog, it isn't editable until unfocused, then refocused. In particular, if I:

  1. Focus the title input.
  2. Open Go to Anything.
  3. Close Go to Anything by pressing Escape.
  4. Type.

Then nothing happens. Adding additional styling to highlight the :focused element reveals that the title input does have focus (but doesn't seem to respond to keyboard events). If I then press shift-tab, then tab, the title input is editable.

I can reproduce a similar issue in the Rich Text editor, but not in the Markdown editor.

Testing

This pull request includes an automated test that verifies:

  • Keyboard focus is restored after dismissing Go to Anything.
    • The test only verifies this for the Markdown editor and title inputs.

Existing automated tests verify that:

  • Clicking on the dimmed background of Go to Anything dismisses the Go to Anything dialog.
    • This dismissal is handled by Dialog.tsx.
  • Escape dismisses the Go to Anything dialog.
    • This dismissal is handled in part by Electron and in part by Dialog.tsx.
  • It's possible to run a command from Go to Anything (and that this dismisses the Go to Anything dialog).

The following manual testing has been done on MacOS:

  1. Focus the markdown editor.
  2. Type Test.
  3. Press cmd-p.
  4. Verify that the Go to Anything dialog is focused.
  5. Dismiss the dialog by pressing Escape.
  6. Type ing.
  7. Verify that the markdown editor is still focused and that the cursor is at the end of the word Testing.
  8. Open settings, then the synchronisation tab.
  9. Tab to the "Sync wizard" button.
  10. Press space.
  11. Verify that the sync wizard dialog is open.
  12. Navigate to the "Close" button using tab.
  13. Press space.
  14. Verify that the sync wizard dialog has closed.
  15. Verify that the "Sync wizard" button has keyboard focus.

Comment on lines +45 to +47
return <>
{dialogElement && createPortal(content, dialogElement)}
</>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On Linux, closing a dialog where the previous focus was the note title
input seems to refocus the title, **but not enter edit mode**. This
commit changes the test to focus the note body input instead.
@laurent22 laurent22 merged commit 292d2fb into laurent22:dev Aug 3, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Related to accessibility desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants