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

fix: Resolve Issue #987 #996

Closed
wants to merge 4 commits into from
Closed

Conversation

jonnymuir
Copy link

@jonnymuir jonnymuir commented Jan 27, 2025

Make accessible the tablist when the tabs do not all fit into the viewport. Remove more button and related code.
Implement scroll left and right buttons.

Description

This solution addresses the accessibility of the tabs component.
Currently the tabs component uses a "more" button to present tabs which don't fit into the view port. To do this it maintains two lists of tabs and moves them between the lists as the page is resized.
This solution takes a solution similar to the shoelace library https://shoelace.style/components/tab-group and presents left and right scroll buttons outside the tablist (if required), as well as default browser horizontal scroll functionality. This enables us to only hold one set of tabs within the tablist. In addition the tabs are given a tab-index so that keyboard navigation can be used.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Motivation and context

To enable accessibility

How to test?

Run storybook and go to the buttons / tabs section.

Resize the screen to restrict the space for the available tabs. You should see a ">" button appear. If you scroll (either with mouse / touch scroll gestures, or with the button) you will see a "<" appear.

Additionally you can now tab into the list and use space to select the list.

Screenshots (if appropriate)

Showing the right hand scroll button
image showing right hand scroll button

Showing the left hand scroll button
image showing left hand scroll button

Checklist

  • If my change requires a change to the documentation, I have updated the documentation in this pull request. (n/a)
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

Make accessible the tablist when the tabs do not all fit into the viewport.
Remove more button and related code.
Implement scroll left and right buttons.
Copy link

Hi there @jonnymuir, thank you for this contribution! 👍

While we wait for the team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

Unintentional change - not sure why the 14 went to 15.
@nielslyngsoe
Copy link
Member

Hi @jonnymuir

Thanks a lot for your contribution, I understand how this is better from a accessibility perspective, especially for blind people and as well keyboard navigation. But I have one perspective that I like your expertise to reflect upon.

Looking at this screenshot:
image

This approach is a fantastic match for the Document Tabs, as it has a wide panel, going from the conceived left edge all the way to the right. The let and right arrows fit well when they are placed against a edge of some kind.
But the Workspace Views Tabs Navigation (Previously Content Apps) they do not have a area stretching from left to right. To the left they have the name input area, and to the right they have the 'Actions' button.
In this case I do find the More button as a nicer user experience. But maybe there is a third way that should be handled? And how do you feel about keeping the old approach for such cases?

Thanks in advance

@jonnymuir
Copy link
Author

Hi Niels,

Thank you for looking at this. I can see the more button approach is much better for the Workspace Views Tabs Navigation.

I would like to spend a little time working out the best approach. I think you are right, if we could detect which approach worked best in which situation this could work well. Maybe there is a third way which has the best of both approaches: the visual UX of the original + the structural simplicity of the above proposed way. The other option is for me to follow what Bjarne suggested and simply move the more button out of the tabbed list. This could work well but I would like to make sure that the accessible experience works in context (i.e. you can easily find your way round the tabs and more tabs and it actually makes sense, not just passes wcag).

The thinking cap is on. Let me see where I get to. Tabbed interfaces are always the most difficult to get right :D

@jonnymuir
Copy link
Author

I think there is third way - I have a POC which keeps the integrity of tabs within the tab group - but presents the original more symbol in the same way as the original solution (using css to present the overflow as a dropdown menu rather than moving them in the DOM).

It'll take a little time to refine it, and test it behaves correctly / is accessible. I'll try and get this done over the next week.

@jonnymuir
Copy link
Author

jonnymuir commented Feb 24, 2025

The third is too messy to get right. Getting something to appear consistently outside of the normal flow overlayed in the right place is do-able with fixed positioning except when there is a transformed ancestor (e.g. a nearby div with something like transform: scale(1) as is the case in the storybook set-up). Then it gets messy. I know the back office won't have this, but still enough to put me off using it.

Absolute positioning is too restrictive because the containing block is usually to small to contain a sensible size menu.

Using the popover api is promising but having enough control over the positioning to enable me to not mess with the DOM structure (i.e. introduce new divs / components) isn't there (yet - it may be possible with anchor positioning which is on the horizon).

Therefore I suspect the best way forward is to do what Bjarne originally suggested and keep the original solution, but move the more button out of the tablist, meaning that the tabbed interface becomes two things when space restricted - a tablist and a dropdown menu (which itself does use the popover api - but does it as it is intended). I'll spend a bit of time seeing how that feels from an accessibility angle. It will be quite straightforward to make it not report wcag failures, but it may need a bit of thought to make it actually usable / make sense using assistive technologies.

Seems a slight shame the third way didn't work! But hey ho - was fun explore through the box model, positioning and containing blocks and view ports (and an introduction to me of the importance of transformed ancestors which was new to me!)

@jonnymuir
Copy link
Author

jonnymuir commented Feb 25, 2025

Closed pull request and created a new one to move the tablist role into the grid and onto the popover list. #1029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants