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

Consistent Sidebar DOM and Open/Closing Interactions #61837

Open
jeryj opened this issue May 21, 2024 · 4 comments
Open

Consistent Sidebar DOM and Open/Closing Interactions #61837

jeryj opened this issue May 21, 2024 · 4 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended

Comments

@jeryj
Copy link
Contributor

jeryj commented May 21, 2024

Description

This has been discussed in various places, but I couldn't find a central area to discuss how the various Panels that are open/closeable from the header should be be implemented in a consistent way. Fixing this would fix deeper issues that are workarounds such as the panel close button between the tabs and tab content, and simplify the codebase by removing a lot of workarounds/hacks that auto-focus panels when they open.

@joedolson mentioned having the Panel DOM be adjacent to the button that activates it:

Programmatically moving focus is a workaround. It solves the problem of finding the panel after opening it, but doesn't provide a clear mechanism to return to the previous location. In general, changes of focus are undesirable unless the change is the only possible option. For example, when opening a modal dialog, focus must be changed, because the trigger is now inert.

But moving focus in other contexts is making assumptions about what the user wants to do. If all the user wanted to do was open the panel - without the intent to perform actions in it - they now have an unpredictable journey to return to their previous location.

When the trigger is in immediate proximity to the triggered container, then that sequence is predictable and short.
#59013 (comment)

If we do this, we could remove the close button, as suggested by @afercia:

Specifically to the Settings and List view panels: do these panels really need a Close button? There's already a toggle button to open and close them.
#59013 (comment)

We shouldn't remove the close buttons unless the panel is next in the DOM to the button that triggers it opens/closes though.

Moving the panels in the DOM to be adjacent to the button that triggers them could introduce some other undesirable interactions though. For example, right now the panels are close in the DOM to the editor canvas. Being able to tab between the editor canvas and the inserter and settings panels is quite nice. If we move these panels to be close to the button that triggers them, should we also add some hidden skip links to allow moving easily between the panels and the editor canvas?

Proposal

  • Move panels to always be adjacent in the DOM to the button that opens them.
  • Remove close button from the panel (fixes Remove extraneous elements placed between Tabs and Tab panels #59013)
  • Add hidden "Skip [Panel name] Content" as first tab stop in the panel. (Otherwise, if you're moving across items in the header, you'd need to tab pas all of the panel content as well).
  • Add hidden "Skip to Editor" Link as the last tab stop in each panel so you can easily move to the editor canvas from the panel.
  • Add hidden "Skip to [Panel name]" links after the Editor Canvas so you can easily navigate to the panels from the editor.

Step-by-step reproduction instructions

  1. Focus Toggle Block Inserter button
  2. Press Enter
  3. Focus is now within the inserter

What I expect to happen

  1. Focus Toggle Block Inserter button
  2. Press Enter
  3. Focus is still on the Toggle Block Inserter button
  4. The Toggle Block Inserter button now has aria-expanded="true" to communicate visible state.
  5. Press Tab to reach the inserter panel content

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

@jeryj jeryj 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). Needs Design Feedback Needs general design feedback. Needs Accessibility Feedback Need input from accessibility labels May 21, 2024
@alexstine
Copy link
Contributor

@jeryj I edited the testing instructions to include a note about aria-expanded on the toggles. Failing to communicate this would confuse users into thinking nothing happened.

I think this is overall a very big lift and not one we should rush into. As imperfect as it is now, I feel like having skip links everywhere is going to make the UI that much harder to use however I do believe it is necessary to have triggering elements controlling the panels.

Thanks.

@afercia
Copy link
Contributor

afercia commented May 22, 2024

Re: The close button ordering issue, I think #61553 would be a more appropriate solution. I would kindly ask designers to not think visually first. Functionality, semantics, and accessibility should always come before any visual consideration. Thanks.

@afercia
Copy link
Contributor

afercia commented May 22, 2024

@joedolson mentioned having the #59013 (comment):

Totally agree, that is a general principle that applies not only to 'panels'. It applies to any UI section that is initially hidden and expands when activating a trigger. Popovers, for example, have the same problem. Dropdown menus as well. Basically, any expandable section of content that resembles a 'disclosure pattern' should follow the principle of making the trigger and panel adjacent in the DOM.

I'm pretty sure this point was reported several times in the history of this project but, alas, often dismissed for other reasons I'm not sure I can support. Developing an user interface should be based on user needs first. Other arguments should come after.

That said, we have another problem to consider.

The editor UI is split into some ARIA landmarks. It is worth reminding ARIA landmarks provide an important mechanism for assistive technology users to get a sense of the main sections of a page and more importantly provide a way to jump from section to section. For example, screen readers provide dedicated shortcuts to jump through landmarks and tools to list all the landmarks in a page. In Gutenberg, we mapped ARIA landmarks to the 'navigable regions' to provide a similar navigational tool to keyboard users who don't use screen readers.

Screenshot to roughly illustrate some of the editor ARIA landmarks:

Screenshot 2024-05-22 at 14 46 40

Moving the panels close to their triggers in the DOM would make this landmark structure almost useless, if not confusing. Some of these landmarks would be basically empty and unused. This is not to say I'm opposed to this proposal but we should definitely take into consideration what to do with the landmarks.

@jeryj
Copy link
Contributor Author

jeryj commented May 22, 2024

I edited the testing instructions to include a note about aria-expanded on the toggles. Failing to communicate this would confuse users into thinking nothing happened.

@alexstine Thank you for catching that! Sorry I accidentally omitted it.

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). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants