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

Tabs doesn't pass accessibility with "more" button #987

Open
bjarnef opened this issue Jan 2, 2025 · 7 comments
Open

Tabs doesn't pass accessibility with "more" button #987

bjarnef opened this issue Jan 2, 2025 · 7 comments
Labels
bug Something isn't working needs triage This issue has not been tested by a team member yet

Comments

@bjarnef
Copy link
Contributor

bjarnef commented Jan 2, 2025

Which exact UI Library version are you using? For example: 1.0.1 - don't just write v1.

1.12.2

Bug summary

When tabs doesn't fit within viewport it shows "more" button.
https://uui.umbraco.com/?path=/story/uui-tab-group--default

However this doesn't pass accessibility as button is not a valid child element.

image

Specifics

No response

Steps to reproduce

Expected result / actual result

No response

@bjarnef bjarnef added bug Something isn't working needs triage This issue has not been tested by a team member yet labels Jan 2, 2025
@jonnymuir
Copy link

Hi Bjarne - this one looks really interesting from an accessibility point of view, I would quite like to look at this. I am going to spend a little time trying to work out how it works this out with the shadow DOM and how this interacts with assistive technologies.

Did you have any thoughts on what a good outcome of this would be (apart from passing wcag / axe tooling)? I feel if may be better if resizing the screen didn't change the DOM, but instead the styles on how the tab headers are presented, but I haven't quite got my head round if this is feasible. I'll do a bit of research what other frameworks do...

@bjarnef
Copy link
Contributor Author

bjarnef commented Jan 19, 2025

In Shoelace library the add buttons to scroll the tablist, but the buttons are not children of the tablist role as in this case <uui-tab-group style="" role="tablist">.
The button should probably be moved somewhere to parent wrapper.

https://shoelace.style/components/tab-group

Image

@jonnymuir
Copy link

jonnymuir commented Jan 20, 2025

Agree - the way shoelace is approach is much better from an accessibility point of view as it doesn't affect the tablist when the view is restricted. I.e. they are all there still there and visible (in terms of style - just out of view port), where-as the the Umbraco UI one is marked display: none and only accessible via the ... (more) button. As you point out the more button is a) not allowed in the tablist anyway, and b) difficult to get to. Also I'm not sure the keyboard navigation is working on the remaining ones either , you can't arrow key through them, or tab into the tablist (apologies - there is confusing overloading of the word tab when it comes to accessibility, tablists and tab).

I suspect a shoelace approach (scrolling the tabs and having buttons outside the tablist for non assistive users to use) would be a better solution? I'd like to take a look and see how much work this would be.

@jonnymuir
Copy link

@bjarnef - I have a proposed solution as above. I'll get it onto a pull request if you think it is acceptable? I just need to tidy up some testing and explore a few more scenarios to check it is ok.

The solution removes the more button and uses left and right scroll buttons to indicate more tabs than fit into the view. I've also made the tabs tab-able to support the keyboard. The good news is it simplifies the code somewhat and I think makes it easier to use. Give me a day or two to get it into shape, I am still finding my way round playwright and storybook.

I'll also pass it by my fellow accessibility guild members for comment and direction. @MMasey

Image

@bjarnef
Copy link
Contributor Author

bjarnef commented Jan 26, 2025

I think we can keep the "More" as it is, it just shouldn't be a child of the tablist.

@jonnymuir
Copy link

Ok, let me take a look. The immediate problem with the more button (which is why the accessibility checker threw it up) is it doesn't fit with what a tablist is. I.e. a tablist and a more button for more elements isn't a tablist. It is a tablist and a dropdown menu from a button (or maybe two tablists, one accessible by a button)

However it may be acceptable to have a tablist and a button drop down and make them completely separate (as you say - don't make it a child) and to make both of them accessible (and tab-able / keyboard navigatable). Not sure how this would translate to a useable accessible experience, as opposed to one which passes wcag but is confusing when using assistive tech, but we can have a play around with it to see what it is like, it may be fine.

It might also get a bit fiddly with the dynamic nature of the two tabs lists. The more button solution moves the elements around as and when it feels it needs to (on resize or new items added) , where-as a scroll solution simply provides a single tablist of elements, and uses css to hide and show the scroll buttons (the buttons aren't the only way to scroll - they are just visual clues, there is the natural browser scrolling too). Because the "more" solution affects the structure of the DOM, I don't know how this will affect aria live regions, or whether it matters as it is will mainly be on resizing. It'll be fun to explore it!

As a side note tablists are notoriously fiddly to get right because they come from a desktop concept and don't always translate so well to the web. I can see why they would be really handy in the back office interface though.

https://inclusive-components.design/tabbed-interfaces/
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tablist_role

As a second side note - using scrolling tablist removes a load of code, takes the tab-group-elements down from 391 lines of code to 240 lines (and 30 lines of that is css to provide a slight visual transparency hint optionally when the buttons are present). That's not necessarily an argument for making the change - also making as few changes as possible is valuable, but gives an interesting insight into simplicity. Just thought it interesting :)

jonnymuir pushed a commit to jonnymuir/Umbraco.UI that referenced this issue Jan 26, 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.
@jonnymuir
Copy link

I've done a pull request - see what you think.

It is absolutely possible to do what you say (move the more button outside the tablist) - in fact it is very easy to do - because it just means move the role of tablist from the main div to the grid div. The rest of the accessibility changes as above become very tricky (but they don't flag up on the automated accessibility audit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage This issue has not been tested by a team member yet
Projects
None yet
Development

No branches or pull requests

2 participants