WordPress / gutenberg

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

Remove extraneous elements placed between Tabs and Tab panels #59013

Open afercia opened 7 months ago

afercia commented 7 months ago

Description

Splitting this out from https://github.com/WordPress/gutenberg/pull/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 https://github.com/WordPress/gutenberg/pull/58942/ would reproduce the same problem in the List View panel.

There's two requirements to follow:

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

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

Potential options to evaluate:

afercia commented 7 months ago

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

In https://github.com/WordPress/gutenberg/issues/58940 / https://github.com/WordPress/gutenberg/pull/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:

alexstine commented 7 months ago

@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 commented 6 months ago

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

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

As I mentioned in https://github.com/WordPress/gutenberg/pull/58942#issuecomment-1943403214 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:

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

This issue was previously reported (partially) in https://github.com/WordPress/gutenberg/issues/58940. As of now, that's almost three months ago. A partial fix was proposed in https://github.com/WordPress/gutenberg/pull/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 commented 5 months ago

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

https://github.com/WordPress/gutenberg/issues/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.

https://github.com/WordPress/gutenberg/pull/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.

afercia commented 5 months ago

@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 https://github.com/WordPress/gutenberg/pull/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 https://github.com/WordPress/gutenberg/issues/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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

Here's a PR to move all sidebar close buttons to the left side: https://github.com/WordPress/gutenberg/pull/61514

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

jameskoster commented 5 months ago

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

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

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

@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: https://github.com/WordPress/gutenberg/pull/63184

afercia commented 3 months ago

@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 commented 2 months ago

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

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