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

Desktop: Fixes #11443: Accessibility: Do not override focus order when pressing tab/shift-tab on the note list #11446

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Nov 26, 2024

Summary

Fixes #11443. Related to #10795.

Before this pull request, Joplin overrode the default focus order when:

  • Pressing tab from the notebook list (skipping sync, new note, new to-do, search, sort by, etc.).
  • Pressing shift-tab from the note list (skipping sort by, search, new to-do, etc.).

This pull request removes the above two overrides, making it easier for screen reader/keyboard only users to access the note list controls and the sync button.

Replacement shortcuts

To preserve ability to navigate quickly between the notes list and sidebar, new shortcuts have been added:

  • Pressing enter from the notebook list: Navigates to the note list.
  • Pressing shift-enter from the note list: Navigates to the notebook list.

Also, for consistency,

  • Pressing enter from the note list: Navigates to the title of the currently open note (if any).

Additional notes

  • Enter was selected, in part, because it's similar to what's done in VSCode when the file tree is focused. When I 1) focus the file tree and 2) press enter, VSCode jumps focus to the editor (skipping focusable elements in between).
  • Currently (before this PR), checkboxes are toggled in the note list by pressing space (and not enter). (Though perhaps they should be toggled by enter?)

Testing plan

Manual testing -- Fedora 41
  1. Start Joplin (with a notebook and note open).
  2. Focus the note list.
  3. Press tab.
  4. Verify that "Synchronize" is selected.
  5. Press tab.
  6. Verify that the "new note" button is selected.
  7. Press tab.
  8. Verify that the "new to-do" button is selected.
  9. Press tab.
  10. Verify that the search field is selected.
  11. Press tab.
  12. Verify that the search button is selected.
  13. Press tab.
  14. Verify that the "toggle sort order field" button is selected.
  15. Verify that the "reverse sort order" button is selected.
  16. Press tab.
  17. Verify that an item in the note list is selected.
  18. Press tab.
  19. Verify that the title input for the currently open note is focused.
  20. Press shift-tab.
  21. Verify that the note list is focused.
  22. Press shift-tab.
  23. Verify that the "reverse sort order" button is focused.
  24. Press tab.
    • This should focus the note list.
  25. Press shift-enter.
  26. Verify that the notebook list has focus.
  27. Press enter.
  28. Verify that the note list has focus.
  29. Press enter.
  30. Verify that the note title input has focus.

This commit removes an inaccessible shift-tab override that made it
difficult to focus the note list controls. Previously:
- If the notebook list was hidden, pressing shift-tab from the note list
  logged an error (and otherwise did nothing).
- If the notebook list was visible, shift-tab skipped the note list
  controls and jumped to the notebook list.

Note: Other shift-tab overrides still exist that should be
removed for WCAG compliance (e.g. shift-tab from the notebook list) see
the WCAG compliance issue for details.
Comment on lines +57 to +60
} else if (event.code === 'Tab' && event.shiftKey) {
event.preventDefault();

if (event.shiftKey) {
void CommandService.instance().execute('focusElement', 'noteBody');
} else {
void CommandService.instance().execute('focusElement', 'noteList');
}
void CommandService.instance().execute('focusElement', 'noteBody');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This focus override should also be removed, though perhaps in a follow-up pull request.

@laurent22 laurent22 merged commit f776d52 into laurent22:dev Nov 27, 2024
7 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.

Inaccessible Backward Navigation in Note List Using Shift+Tab
2 participants