WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
9.99k stars 4.01k forks source link

Keyboard navigation: `Open save panel` appears when there are no changes to save #59215

Open priethor opened 4 months ago

priethor commented 4 months ago

Description

When using keyboard navigation to navigate through toolbars, there is an edge case where the Open save panel button appears even when there are no changes to save.

Step-by-step reproduction instructions

  1. Go to the Site editor
  2. Focus on a block
  3. Navigate to the toolbar through shift + tab
  4. Hit again `shift+tab``

An Open save panel button appears, even if nothing needs saving. It also looks a bit odd when the inspector sidebar is not open.

Screenshots, screen recording, code snippet

https://github.com/WordPress/gutenberg/assets/27339341/6e82b767-7c0c-45bd-8a58-add9bf2bef30

Environment info

WordPress 6.4.3, with or without Gutenberg 17.7

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 commented 3 months ago

Reopening because I think the same change from https://github.com/WordPress/gutenberg/pull/59543 should be made for the 'Open publish panel' button in the Post editor. I'm not sure having these two buttons work inconsistently makes much sense.

Clarification on why we do render these buttons even when there's nothing to publish / save.

Terminology: the ARIA landmarks in the editor are called 'navigable regions'.

When we implemented ARIA landmarks in the editor, it was pointed out a few tims that landmarks can't be dynamically rendered. IN a few words: they can't be sometimes rendered and sometimes not. ARIA landmarks are a sort of 'contract' the user interface establishes with users. Landmarks serve the purpose of identifying the main sevtions of a page (in this case the editor) and allow users to jump through them.

Screenshot:

Screenshot 2024-03-08 at 15 05 13

From that moment on, users expects those landmarks to be always present on the page.

Vice-versa, it would make sense to initially render, say, only 4 landmarks and then dynamically add more landmarks depending on some UI state. Users won't have a clue there are more landmarks than the 4 ones they discovered initially.

As such, ARIA landmarks must always be rendered. They can't 'disappear and reappear' based on some condition. This is a principle that should always be followed.

Note: There is already some inconsistency about this, for example the List View panel is an ARIA landmark labeled 'Document Overview' or 'List View'. The landmark is rendered only when the panel is open, which goes against the principle mentioned above. When open, the List view adds two more ARIA landmarks, which isn't helpful for users. Screenshot:

Screenshot 2024-03-08 at 15 13 05

Given ARIA landmarks should always be rendered, that also means they can't be empty. It wouldn't really make sense to have empty ARIA landmarks.

As such, long time ago it was decided alwasy have some content inside the editor landmarks, even when they are actually collapsed panels. The 'Open publish panel' and the 'Open save panel' were implemented for this reason. This way, even when these ARIA landamrks are collapsed panels, they still contain something. Teh content is actually a toggle button that 'opens the panel' which basically populates the ARIA landmark with the UI content.

A similar mechanism should have been implemented for the 'List View' panel. It should always be rendered. When closed, it should only contain a button to open the List view panel.

W3C examples and general principles about Landmarks use: https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/general-principles.html

ntsekouras commented 3 months ago

Thanks for the detailed explanation @afercia! 🚀