fix(tab): tabpanel is tabbable when content has no tabbable content

[tay1orjones]

Closes #7078

Add hasNoFocusableContent prop to Tab to inform if the tabpanel container should be focusable. This preserves existing functionality and allows users to opt-in to the new focus behavior when their tab content is text only or has no focusable elements/content.

Add useTabbableContent to TabContent to automatically determine if the contents contains a tabbable element.

Changelog

New

• Add useTabbableContent hook to TabContent

Changed

• Update Tabs story to have a tabbable button in the contents of one tab
• Refactor TabContent tests to use testing-library and test for tabindex/hook functionality

Testing / Reviewing

1. Open the default Tabs story
2. Type the Tab key - focus should move to the tabpanel/content container.
3. Type Shift + Tab - focus moves back to "Tab Label 1"
4. Type → - focus should move to "Tab Label 2"
5. Type the Tab key again - focus should move to the button inside the content (the tabpanel should not be focused as the content contains a focusable element)

[netlify]

Deploy preview for carbon-elements ready!

Built with commit 3c7538d

https://deploy-preview-7894--carbon-elements.netlify.app

[netlify]

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 3c7538d

https://deploy-preview-7894--carbon-components-react.netlify.app

[joshblack]

Would we want to do this automatically for users, potentially? Like in an effect. Something like:

function TabContent(props) {
  // ...
  const ref = useRef(null);
  const hasFocusableContent = useFocusableContent(ref);

  // ...
  return <div role="tabpanel" tabIndex={hasFocusableContent ? 0 : undefined}>...</div>
}

function useFocusableContent(ref) {
  const [hasFocusableContent, setFocusableContent] = useState(false);

  useEffect(() => {
    const elements = getFocusableElements(ref.current);
    if (elements.length > 0) {
      setFocusableContent(true);
    }
  }, [ref]);

  return hasFocusableContent;
}

[tay1orjones]

I dig the magic there for sure. It would also work if content is passed in via renderContent.

My only concern would be performance of getFocusableElements(ref.current);. Iterating over every element recursively against a known list of focusable elements on every render feels ... expensive 😬

[tay1orjones]

You know what... I forgot about querySelectorAll 😅 Yeah this sounds good, I'll update the PR.

[emyarod]

this might be helpful for your querySelectorAll https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/internal/keyboard/navigation.js#L64-L72

[tay1orjones]

Thanks @emyarod, I hadn't seen that. I think it's selectorTabbable that I really wanted, "focusable elements" was a misnomer.

I modified the PR body to reflect the new approach. Also added some tests and refactored the tests off enzyme.

[jnm2377]

LGTM! Tab content is now tabbable if there's no interactive elements. Thanks for adding the comments to the code too! Super helpful! 🔥

[joshblack]

@tay1orjones definitely get what you mean, it might be that this abstraction doesn't make see if the tabpanel also needs to be explicitly labeled so would totally understand moving forward with the prop approach if this wouldn't be a good fit to do automatically.

[tay1orjones]

@joshblack It works quite nicely. Explicitly labelling the tabpanel feels like a very unlikely use case, but we could allow it by placing the automatic tabindex above {...others} so it's overridable?

[andreancardona]

@tay1orjones this looks awesome!

I'm so so sorry for the review delay... 👀