adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.58k stars 1.09k forks source link

[useTablist] tabpanel elements are not focusable when layout differs as per docs #6703

Closed Casneil closed 1 month ago

Casneil commented 2 months ago

Provide a general summary of the issue here

Tabpanel/Tabpanel elements looses focus if change to Tabs and Tabpanel layout.

πŸ€” Expected Behavior?

Focusable elements inside the TabPanel recieves correct focus.

😯 Current Behavior

Focusable elements within the Tabpanel does not recieve focus. This is visible via the red colored link.

πŸ’ Possible Solution

No response

πŸ”¦ Context

We are using the Tabs component with a slight change to the layout found here.

Our requirement requires us to have a seemless switch between a tab like (Tablist) component on desktop and a more tabcordion type component on mobile.

To achieve this, we chose to have both Tablist and Tabpanel components inside one wrapping element. Although this gave us the desired outcome, we realised that the tab indecies are noo longer working.

We are no longer able to focus any focusable element within the tabpanel.

πŸ–₯️ Steps to Reproduce

Issue can be reproduced here

Within this example, you can notice that the link withing the Tabpanel never recieves focus.

Version

3.32.1

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

If other, please specify.

No response

What operating system are you using?

Macbok Pro 14 inch 2021

🧒 Your Company/Team

Statista GmbH

πŸ•· Tracking Issue

No response

LFDanLu commented 1 month ago

I believe this is because the TabList is now wrapping the TabPanel itself which causes this code to skip over the focusable content within the TabPanel. I did a quick change to move the TabPanel so it is a sibling to the TabList much like how it is set up in https://react-spectrum.adobe.com/react-aria/useTabList.html#example and it seems to work, albiet with some styling issues since I had to get rid of the div wrapping the TabPanel: https://codesandbox.io/p/devbox/aria-tabcordion-grid-forked-3p9plj?file=%2Fsrc%2Ftabcordion%2Ftabs.tsx%3A39%2C11

Does a change like this work for you?

Casneil commented 1 month ago

I believe this is because the TabList is now wrapping the TabPanel itself which causes this code to skip over the focusable content within the TabPanel. I did a quick change to move the TabPanel so it is a sibling to the TabList much like how it is set up in https://react-spectrum.adobe.com/react-aria/useTabList.html#example and it seems to work, albiet with some styling issues since I had to get rid of the div wrapping the TabPanel: https://codesandbox.io/p/devbox/aria-tabcordion-grid-forked-3p9plj?file=%2Fsrc%2Ftabcordion%2Ftabs.tsx%3A39%2C11

Does a change like this work for you?

Thanks for your reply! the example link to codesandbox does not work I cannot confirm this. Also to avoid possible confusion, the change in the set up is due to having the tabs also like an "accordion" element when on mobile devices.

snowystinger commented 1 month ago

He will need to set his codesandbox to public.

In the meantime, can you explain more about what you mean that it needs to be like an accordion element?

I ask because it sounds like it should be a completely different component as those are two different aria patterns. If you need to persist state then you should raise that state up to a parent. If you need to persist focus, then you will need to manually handle that yourself.

Casneil commented 1 month ago

He will need to set his codesandbox to public.

In the meantime, can you explain more about what you mean that it needs to be like an accordion element?

I ask because it sounds like it should be a completely different component as those are two different aria patterns. If you need to persist state then you should raise that state up to a parent. If you need to persist focus, then you will need to manually handle that yourself.

Seems like the latter to me. For a more detailed explanation - we wanted a solution for having the tabs being displayed as is shown in this setup on desktop but would switch to an accordion on mobile.

We also need to achieve this by not conditionally rendering seperate components based on viewport, but rather with a pure CSS solution. With this being said, we needed to change the setup slightly which then resulted in this issue.

Given all this, if I understood you correctly, for this scenario the solution we would need to handle the tabbing from our side?

snowystinger commented 1 month ago

I don't think you should use the same component for both is what I'm saying. We don't have support for aria-multiselectable in Tabs at this time.

It sounds like you would change from single selection on desktop to multi selection on mobile? So you would need more than a CSS solution anyways.

You could make use of https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia in order to have your JS on the same breakpoints as your CSS if that's the issue.

I think the source of confusion is that tabs have this structure https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role#example ^ impossible to interleave tabpanels between tabs because they are in a tablist and a tablist may only have tab children https://w3c.github.io/aria/#tablist

while accordion has this structure https://www.w3.org/WAI/ARIA/apg/patterns/accordion/examples/accordion/ where the each section's button is immediately followed by the content which it shows/hides

Casneil commented 1 month ago

I don't think you should use the same component for both is what I'm saying. We don't have support for aria-multiselectable in Tabs at this time.

It sounds like you would change from single selection on desktop to multi selection on mobile? So you would need more than a CSS solution anyways.

You could make use of https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia in order to have your JS on the same breakpoints as your CSS if that's the issue.

I think the source of confusion is that tabs have this structure https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role#example ^ impossible to interleave tabpanels between tabs because they are in a tablist and a tablist may only have tab children https://w3c.github.io/aria/#tablist

while accordion has this structure https://www.w3.org/WAI/ARIA/apg/patterns/accordion/examples/accordion/ where the each section's button is immediately followed by the content which it shows/hides

Absolutely! from a structural standpoint they are different. We will discuss this internally. Thanks for the feedback! πŸ™‚

LFDanLu commented 1 month ago

@Casneil Ah, apologies, I've updated the sandbox to be public