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

Site editor: Navigating regions via keyboard shortcuts gets stuck on a region #52920

Closed
afercia opened this issue Jul 25, 2023 · 4 comments · Fixed by #52940
Closed

Site editor: Navigating regions via keyboard shortcuts gets stuck on a region #52920

afercia opened this issue Jul 25, 2023 · 4 comments · Fixed by #52940
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Edit Site /packages/edit-site [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Jul 25, 2023

Description

It appears that in the Site editor when navigating regions via keyboard shortcuts, at some point you get stuck on a region. This is a serious regression in a feature that greatly helps all keyboard users to use the editor efficiently.

As a side note: If I remember correctly there are a few E2E tests in place for the navigate regions feature but these are only for the Post editor. It appears it would be greatly beneficial to add tests for this feature also for the Site editor.

Reminder for testing: The keyboard shortcuts varies depending on the operating system. You can always check what the available keyboard shortcuts are in the editor > Options > Keyboard shortcuts.

On macOS:

Navigate to the next part of the editor.

  • Control + backtick or alternatively Control + Option + N

Navigate to the previous part of the editor.

  • Shift + Control + backtick or alternatively Control + Option + P

Step-by-step reproduction instructions

  • First, go to the Post editor and observe the keyboard shortcuts work as expected:
    • At each key combination press, focus moves to one of the editor main regions.
    • A blue focus outline appears around the currently focused region.
  • Go to the Site editor.
  • On first load, the Editor is in View mode.
  • Use the keyboard shortcuts and observe focus move from the navigation panel to the preview iframe and so on.
  • So far so good.
  • Switch the editor to Edit mode.
  • Try the keyboard shortcut to move to the Next region.
  • You are almost immediately stuck on one of the regions.
  • Try the keyboard shortcut to move to the Previous region.
  • Observe you can navigate 2-3 regions and then you get stuck.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release [Package] Edit Site /packages/edit-site labels Jul 25, 2023
@afercia
Copy link
Contributor Author

afercia commented Jul 25, 2023

Note: when testing with Chrome and the browser dev tools panel is open, the panel steals focus from the page, not sure why. Best way to test with dev tools open, use the alternative shortcurs:

  • Control + Option + N
  • Control + Option + P

@afercia
Copy link
Contributor Author

afercia commented Jul 25, 2023

Regressed in #51956

Turns out that while the intent was good (fix the routing issues on mobile), the change makes useNavigateRegions fail.

When hiding, removing from the DOM, or (such as in this case) making on of these regions inert, the navigation through regions can't work as expected.

These regions need to:

  • be always rendered in the DOM
  • be not hidden via CSS
  • they need some meaningful content

in some case, for example the Settings region or the Save panel region, when the sidebar is not visible we are rendering at least one button within the region. The button is visually hidden but can be revealed via keyboard navigation and opens the related sidebar. This matches the users expectations. In fact, when using an ARIA role region with an aria-label on a certain element, that element becomes an ARIA landmark. Landmarks are a sort of 'contract' with the user. WHen landing on a page, ladnmarks inform users the page contains {n} main regions. At any moment, users expect to be able to find and nvigate these regions. Hiding and making one of the regions inert breaks the contract with the user.
For reference:
Principles: ARIA Landmarks Example
https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/general-principles.html

Quoting from the PR that introduced this regression #51956 (emphasis mine):

Since we're not sure how to fix the routing issues on mobile, we're leaving the sidebar present in the DOM, but hiding it visually and marking it as inert to stop all user interactions on the sidebar.

Stopping user interaction on an ARIA landmark that is also an editor NavigableRegion seems less than ideal.

The sidebar is necessary for routing on mobile so we have to maintain its presence in the DOM. Just hiding it isn't enough though, as it is still able to be reached with keyboard tabs and screen readers. Using the relatively new inert property disables the element from user interaction, so we add that when we don't want the sidebar to be shown.

@afercia afercia self-assigned this Jul 25, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jul 25, 2023
@afercia
Copy link
Contributor Author

afercia commented Jul 28, 2023

To better illustrate the bug, I made an animated GIF.
To reproduce, just go to the Site Editor and try to navigate the editor regions with Control + backtick and Shift + Control + backtick. Observe that at some point the jumping through the regions gets stuck.

navigate-regions

@annezazu
Copy link
Contributor

@jeryj could you perhaps take a look here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Edit Site /packages/edit-site [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants