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

List View: tabs and close button visual order and DOM order mismatch #58940

Closed
afercia opened this issue Feb 12, 2024 · 11 comments
Closed

List View: tabs and close button visual order and DOM order mismatch #58940

afercia opened this issue Feb 12, 2024 · 11 comments
Assignees
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Feb 12, 2024

Description

In the process of reviewing all the CSS order properties in use in the editor, I noticed the List View tabs and close button visual order and DOM order mismatch. For accessibility, the order must always match when it affects 'meaning and operation'. basically, only purely decorative elements can be reordered. Reference:

WCAG 2.2
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

Screenshot to illustrate the reading / DOM order mismatch.

Screenshot 2024-02-12 at 14 50 40

In this specific case, these elements are also interactive, which makes the problem even more relevant.

Instead of using order, the DOM order must be adjusted to match the desired visual order.

Introduced in a3a55bb so this order property is new on trunk and should be fixed for the WordPress 6.5 release.
Worth noting that the order mismatch occurs also in previous versione but for a different reason. The Close button used to be absollutely positioned.

The fix appears to be trivial as the button just needs to be moved after the tabs. Code:

https://github.com/WordPress/gutenberg/blob/trunk/packages/editor/src/components/list-view-sidebar/index.js/#L133-L155

Since it appeaers to be a simple, self-contained fix, I'd like to propose it for WP 6.5 consideration.

Step-by-step reproduction instructions

  • Go to te post editor.
  • Open the List VIew.
  • Inspect the elements in the list view top part.
  • Observe the Close button is first in the DOM but it's visually shown after the tabs.

Test with a screen reader, for example Safari + VoiceOver.

  • Open the list view. Initial focus is set on the first item in the list (which is an a11y problem I will report separately).
  • Press Control + Option + Left arrow key until you reach toe top part of the list view.
  • Observe the first element VoiceOver moves to within the top part of the list view is the 'Outline' tab.
  • Press Control + Option + Left arrow key again.
  • Observe VoiceOver moves to the 'List View' tab.
  • Press Control + Option + Left arrow key again.
  • Observe VoiceOver moves to the Close button, unexpectedly jumping to the right.

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). [Package] Editor /packages/editor [Feature] List View Menu item in the top toolbar to select blocks from a list of links. CSS Styling Related to editor and front end styles, CSS-specific issues. labels Feb 12, 2024
@afercia afercia self-assigned this Feb 12, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Feb 12, 2024
@andrewhayward
Copy link
Contributor

andrewhayward commented Feb 12, 2024

You referenced two success criteria, but I think the ambiguity of both actually make this less clear cut.

1.3.2 Meaningful Sequence:

When the sequence in which content is presented affects its meaning, a correct reading sequence can be programmatically determined.

2.4.3 Focus Order:

If a Web page can be navigated sequentially and the navigation sequences affect meaning or operation, focusable components receive focus in an order that preserves meaning and operability.

I appreciate where you're coming from, as regards the apparent visual order, but I think the programmatically determined order actually makes more sense as things stand. The close button belongs to the entire sidebar, rather than anything relating to the tabs.

As such, the close button should be one of the first things you come to inside the sidebar region. Because the navigation sequence really does affect "meaning or operation", I'm not convinced that its position in the DOM should be between the tabs and the tab panels. So while I accept the focus "unexpectedly jumping to the right" is an issue, I think the current order does a better job of preserving overall meaning, particularly as regards semantic structure, and outweighs the visual inconsistency.

I see this as being akin to a dialog window, with a close button at the top. Even with tabs in the modal, I'd expect them to come after the close button in the DOM, even if visually the button seems to be after the tabs.

A dialog window, showing three tabs above a block of text. A close button is present in the top right, and a 'save changes' button is shown in the bottom right.
<dialog title="Item editor">
  <button>×</button>
  <tabs>
    <tab for="panel-1" selected>Item 1</tab>
    <tab for="panel-2">Item 2</tab>
    <tab for="panel-3">Item 3</tab>
  </tabs>
  <panel id="panel-1">...</panel>
  <panel id="panel-2">...</panel>
  <panel id="panel-3">...</panel>
  <button>Save changes</button>
</dialog>

@colorful-tones
Copy link
Member

This seems to be labeled In Progress and has an attached PR. I suggest we move this into the In Progress column on the WordPress 6.5 Editor Tasks board.

@afercia
Copy link
Contributor Author

afercia commented Feb 14, 2024

@andrewhayward thanks for your feedback

As such, the close button should be one of the first things you come to inside the sidebar region. Because the navigation sequence really does affect "meaning or operation", I'm not convinced that its position in the DOM should be between the tabs and the tab panels.

I agree that there shouldn't be any focusable element between the tabs and the tab panels, I mentioned this point in my last comment on the PR #58942 (comment)

Worth noting this applies to other places in the editor UI, for example the Settings panel which has been recently refactored and now uses Tabs. It appears to be a broader issue that spans across the editor UI.

As such, the only way to fix this is by changing the design. I'd propose to:

  • Move this point to a separate issue.
  • Start a conversation with designers in this project to let them understand the ARIA design patterns must be implemented as detailed in the ARIA Authoring Practices Guide (APG). This is a requirement. Design should adapt to those patterns, not vice-versa.

@andrewhayward
Copy link
Contributor

Worth noting this applies to other places in the editor UI, for example the Settings panel which has been recently refactored and now uses Tabs. It appears to be a broader issue that spans across the editor UI.

Sounds like we should probably close this issue then, and open up another to track the various places where the design/tab order is inconsistent or needs a rethink, so we can have a broader conversation around expectations.

@afercia
Copy link
Contributor Author

afercia commented Feb 14, 2024

I'm not sure we should close this issue. They're separated issues.
This issue and the related PR #58942 would, at least, make the List view tabs implementation and order consistent with the one in the Settings panel.
I'd think having better consistency is a good intermediary step.
I'd rather discuss the broader issue separately.

@andrewhayward
Copy link
Contributor

I'd think having better consistency is a good intermediary step.

If we're going with consistency as an intermediary step, I'd suggest that we adjust the close button in the Settings sidebar to match the Overview panel, not the other way round.

I know it's a little visually incoherent in the Overview panel, but I think that's a lesser issue (and more likely to be understood, as a fairly common albeit confusing pattern) than having the close button in the DOM between the tabs and the associated panels.

@afercia
Copy link
Contributor Author

afercia commented Feb 14, 2024

I created a separate issue for broader discussion: #59013

@afercia
Copy link
Contributor Author

afercia commented Feb 14, 2024

If we're going with consistency as an intermediary step, I'd suggest that we adjust the close button in the Settings sidebar to match the Overview panel, not the other way round.

It can't be done because the component that renders the sidebar (under the hood it's the ComplementaryArea component) receives the Tabs list as the header prop and the Closes button is handled internally.

I know it's a little visually incoherent in the Overview panel, but I think that's a lesser issue (and more likely to be understood, as a fairly common albeit confusing pattern) than having the close button in the DOM between the tabs and the associated panels.

I'm really not sure which is worse between the two bugs 🙃 Focus that randomly jumps from right to left and vice-versa in a very unpredictable way is a terrible experience.

@andrewhayward
Copy link
Contributor

It can't be done...

Oh... hmm. I had a quick look at the ComplementaryAreaHeader component, and while fixing things would certainly be a bit more involved, I'm not sure as we should stop with "can't be done".

Focus that randomly jumps from right to left and vice-versa in a very unpredictable way is a terrible experience.

Don't know as I'd agree with "unpredictable", but it's all semantics at this point! If it was at least consistent everywhere, it probably wouldn't be as bad an experience.

@youknowriad
Copy link
Contributor

Given that RC1 is today I'm removing this one from 6.5.

@afercia
Copy link
Contributor Author

afercia commented Mar 8, 2024

Closing in favor of #59013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants