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

Remove extraneous elements placed between Tabs and Tab panels #59013

Open
afercia opened this issue Feb 14, 2024 · 17 comments
Open

Remove extraneous elements placed between Tabs and Tab panels #59013

afercia opened this issue Feb 14, 2024 · 17 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Needs design efforts. [Package] Components /packages/components [Package] Edit Post /packages/edit-post [Package] Edit Site /packages/edit-site [Package] Edit Widgets /packages/edit-widgets [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Feb 14, 2024

Description

Splitting this out from #58942

Recent work improved the consistency of various tabbed interfaces in the editor by using the Tabs component. That's a very welcomed improvement.

However, in a few places the Tabs component is used incorrectly. The whole purpose of a tabbed interface is to allow keyboard users to save tab key presses and be able to tab directly from a selected tab to its associated tab panel.

Image courtesy of Heydon Pickering, from his accessible components, to visually illustrate the expected keyboard interaction:

tabs interaction

Placing extraneous focusable elements between the tabs and the tab panels defeats the purpose of a tabbed interface.

There's a few places in the editor where a 'Close' button is placed in the between and adds an unexpected tab stop between the tabs and the tab panels. For example, in all the Settings panels:

Screenshot 2024-02-14 at 12 50 28

Screenshot 2024-02-14 at 12 50 51

Screenshot 2024-02-14 at 13 52 31

And #58942 would reproduce the same problem in the List View panel.

There's two requirements to follow:

  • There must be no focusable or otherwise interactive elements between Tabs and Tab panels.
  • The visual order must match the DOM order.

As such, the only solution to this is to remove the Close button from that spot and place it somewhere else.

It would be great to have the Tabs component to enforce this via code: no extraneous element can be placed between Tabs and Tab panels.

Step-by-step reproduction instructions

  • Go to the Post editor > Settings panel.
  • Use a keyboard.
  • Tab to the first tab.
  • Use the right or left arrow keys to select a tab.
  • Optionally activaate on of the tabs by pressing Enter or Space bar.
  • Press the tab key again.
  • Expected: focus to go to the Tab panel (either the tab panel itself or the first focusable element within it, depending on the configuration).
  • Actual: focus moves to the Close button.

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). Needs Design Needs design efforts. [Package] Components /packages/components [Package] Edit Post /packages/edit-post [Package] Edit Widgets /packages/edit-widgets [Package] Edit Site /packages/edit-site labels Feb 14, 2024
@afercia
Copy link
Contributor Author

afercia commented Feb 14, 2024

Potential options to evaluate:

  • 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.
  • Consider to move the Close button above the tabs, maybe preceded by a heading.
  • Avoid non-standard workarounds like removing the button from the tab order or making it hidden from assistive technologies as that's not the expected interaction with a button and would introduce a new accessibility issue.

@afercia
Copy link
Contributor Author

afercia commented Mar 8, 2024

Cc @andrewfleming @alexstine @joedolson @WordPress/gutenberg-design

In #58940 / #58942 a decisionw as made to not not introeuce a temporary, partial, fix for this issue and rather fix the broader issue with teh Tabs component.
This requires to:

  • Change the base component to not allow any content between Tabs and Tabpanels.
  • A new design. Wherever tabs are used there shouldn't be anything between Tabs and Tabpanels. The DOM order must match the visual order. As such, nothing can be visually placed between Tabs and Tabpanels. For example, some 'X' Close buttons placed after the tabs need to be removed or placed elsewhere.

@alexstine
Copy link
Contributor

@afercia

Specifically to the Settings and List view panels: do these panels really need a Close button?

Yes they do. The controlling toggle is multiple levels above in the DOM and would make this very undiscoverable. I opened an issue for it a long time ago, still no movement.

@richtabor
Copy link
Member

I opened an issue for it a long time ago, still no movement.

I'm not following; there are close buttons for the Inspector and List Views.

@afercia
Copy link
Contributor Author

afercia commented May 6, 2024

I'm not following; there are close buttons for the Inspector and List Views.

As I mentioned in #58942 (comment) the 'inspector', also known as Settings panel, didn't use Tabs. It was then refactored to use Tabs and now it has the same problem: extraneous content between tabs and tab panels.

In the List View, the Close button is only visually placed between the tabs and the tab panels. In the DOM, it is actually placed before the tabs. While that's not correct anyways, because visual order and DOM order mismatch, at least there's no extraneous, unexpected tab stop between the tabs and the tab panels.

I the hope to make this issue more understandable and allow people to follow better, I'd like to illustrate again two fundamental principles:

  • Any extraneous focusable element or even non-focusable content between Tabs and Tab panels breaks the ARIA tabs design pattern. It defeats the purpose of using an ARIA tabs interface.
  • Visual / reading / DOM order must match.

As such, the only way to solve this issue is by changing the design.

This issue was previously reported (partially) in #58940. As of now, that's almost three months ago. A partial fix was proposed in #58942. I closed that PR after agreement to move to this, broader issue and get it prioritized.

Unfortunately, I don't see this issue getting prioritized and getting the support it would deserve from accessibility specialists., and it's still waiting for a first design proposal.

@alexstine
Copy link
Contributor

@richtabor What I'm talking about is an age old bug, basically since the beginning of Gutenberg.

#15322

The settings sidebar with block/post tabs requires a close button because the triggering element is way up at the top of the DOM. This is due to how it was implemented in the wordpress/interface package. I made a lousy attempt at fixing it but never was successful.

#38360

Crazy to think back then I thought that was a good idea. I slap myself for that type of shoddy work today.

So, if we manage to fix the original bug here, the close button might be able to be removed all together. Settings trigger needs to move to the sidebar it controls.

@afercia

Unfortunately, I don't see this issue getting prioritized and getting the support it would deserve from accessibility specialists., and it's still waiting for a first design proposal.

I'll take some blame for not looking at this much but in all fairness, this is not a priority 1 issue for us right now. I'm busy working on regressions and trying to get bad issues fixed before the upcoming release. While this is not an ideal experience, this is not exactly a house on fire issue either.

Thanks.

@alexstine alexstine added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label May 8, 2024
@afercia
Copy link
Contributor Author

afercia commented May 8, 2024

@alexstine I'm not sure I agree. While there are certainly more important issues, this ons is important as well.
You may not know that #61421 just added a new Close button to the inserter, thus adding one more instance of this problem. There is now a new Close button at the top of the main Inserter where visual and DOM order mismatch. It's one more instance of the problem already reported in #58940 for the Liset View, but we closed that issue in favor of this one. No progress since then, and in the meantime new instances of this wrong pattern are being introduced.
I'd say introducing new problems isn't great, and that happens because this issue didn't get the attention and priority it deserves.

@jameskoster
Copy link
Contributor

@alexstine @afercia I'm curious, would be a better experience in terms of a11y if; instead of multiple panels/toggles for 'Settings', 'Styles', plus any other pinned plugins, there was a single 'Plugins sidebar' toggle which opened a drawer containing a tab interface for all plugins?

I ask because it's something I've been thinking about as a potential solution to over-crowding in the top bar, which can be quite problematic when you install / pin a few plugins, and enable 'top toolbar':

Site Editor

@alexstine
Copy link
Contributor

@afercia I agree it's important but sometimes I've got to make tough calls on what gets my attention. For example, the issue about adding inert to blocks is still open and that is a totally unusable experience for non-sighted users where this is a WCAG failure, but arguably not one that blocks someone from accessing the editor.

I totally support anyone else picking this up right now, I'll try to help the best I can given current time constraints.

@jameskoster That's not really a problem for keyboard users, the real problem is the position in the DOM. Fewer toggles might be better but hard to say without seeing the change.

@jeryj
Copy link
Contributor

jeryj commented May 9, 2024

Here's a PR to move all sidebar close buttons to the left side: #61514

Close button is not between the tab and tab panels. Focus order matches visual order.

@jameskoster
Copy link
Contributor

the real problem is the position in the DOM

Doesn't programatically moving focus to the panel on open help? What's the expected behavior here, should the close button be focussed first?

@joedolson
Copy link
Contributor

@jameskoster 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.

@ciampo
Copy link
Contributor

ciampo commented Jul 9, 2024

Catching up through Tabs stuff, and found this issue.

I think that a good fix, at least for the short term, could be to render the "Close" button before the tabs in the DOM order, and use CSS to retain the same current design. I know that it's always better to match keyboard focus order with the visual layout on the page, but this solution would fix the issue while not requiring any design changes.

We could always continue to iterate on a better shared solution, like in #61837 and other similar exploratory PRs.

@jeryj
Copy link
Contributor

jeryj commented Jul 9, 2024

@ciampo

at least for the short term, could be to render the "Close" button before the tabs in the DOM order, and use CSS to retain the same current design.

Agreed. That's what we're doing for now as a stopgap with the <TabbedSidebar /> component: #63184

@afercia
Copy link
Contributor Author

afercia commented Jul 10, 2024

@ciampo @jeryj Sorry but Im not sure I understand how sovlign an issue by creating another issue is okay. The mismatch between visual roder and DOM order is an explicit violation of two WCAG guide lines and it's not an acceptable solution. Not even as a 'stopgap'.

Cc @joedolson I'd like to hear your thoughts.

retain the same current design.

Well, this is one of those cases where the design must be changed.

Cc @WordPress/gutenberg-design We need a new design, thanks,

WCAG reference:
1.3.2 Meaningful Sequence (Level A) https://www.w3.org/TR/WCAG22/#meaningful-sequence
2.4.3 Focus Order (Level A) https://www.w3.org/TR/WCAG22/#focus-order

@joedolson
Copy link
Contributor

It sounds like the only benefit to @ciampo's proposed change is that it would match the behavior of the <TabbedSidebar /> component, in that they would both have the same problem, rather than having slightly different problems.

I'd consider it a stretch to call that a "fix", even temporary, since it's just trading one problem for a different one. Consistency does have benefits, even if it includes errors, because consistency makes it easier for users to learn the quirks of a system.

I think it would be OK to do this, solely on the grounds that it would possibly increase consistency, but would not consider it a fix; just an exchange of problems.

But I don't think we should underestimate the value of consistency.

If the panels had visible headings above the tabs, that would make things a lot easier to design.

@afercia
Copy link
Contributor Author

afercia commented Jul 11, 2024

Fair enough. Then I don't have more objections as long as there is a clear established plan to fix the ARIA tabs pattern.
As I see it, the only way to fix it is by changing the design. I'd greatly appreciate this issue to be prioritized and get a new design. Thanks everyone for your thoughts and feedback.
Cc @WordPress/gutenberg-design

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 Design Needs design efforts. [Package] Components /packages/components [Package] Edit Post /packages/edit-post [Package] Edit Site /packages/edit-site [Package] Edit Widgets /packages/edit-widgets [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants