From ac69e8bf4a4381f839fd58500462d191ecaa7ae5 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Wed, 24 Feb 2021 21:22:02 -0600 Subject: [PATCH 1/4] fix(tab): tabpanel is focusable when content has no focusable content --- .../__snapshots__/PublicAPI-test.js.snap | 6 ++++ packages/react/src/components/Tab/Tab.js | 5 +++ .../src/components/TabContent/TabContent.js | 16 ++++++++-- .../react/src/components/Tabs/Tabs-story.js | 31 ++++++++++++------- packages/react/src/components/Tabs/Tabs.js | 4 ++- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 13d6768580c9..ea98b6483a26 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -5070,6 +5070,9 @@ Map { "handleTabKeyDown": Object { "type": "func", }, + "hasNoFocusableContent": Object { + "type": "bool", + }, "href": [Function], "id": Object { "type": "string", @@ -5119,6 +5122,9 @@ Map { "className": Object { "type": "string", }, + "hasNoFocusableContent": Object { + "type": "bool", + }, "selected": Object { "type": "bool", }, diff --git a/packages/react/src/components/Tab/Tab.js b/packages/react/src/components/Tab/Tab.js index 68b3862159f8..dc2bf0d764c5 100644 --- a/packages/react/src/components/Tab/Tab.js +++ b/packages/react/src/components/Tab/Tab.js @@ -38,6 +38,11 @@ export default class Tab extends React.Component { */ handleTabKeyDown: PropTypes.func, + /** + * Specify if the tab content does not contain focusable content. The tabpanel instead will be included in the focusable dom order. + */ + hasNoFocusableContent: PropTypes.bool, + /** * Provide a string that represents the `href` of the Tab */ diff --git a/packages/react/src/components/TabContent/TabContent.js b/packages/react/src/components/TabContent/TabContent.js index abe8140d73d3..5793aec44ec6 100644 --- a/packages/react/src/components/TabContent/TabContent.js +++ b/packages/react/src/components/TabContent/TabContent.js @@ -13,7 +13,13 @@ import { settings } from 'carbon-components'; const { prefix } = settings; const TabContent = (props) => { - const { className, selected, children, ...other } = props; + const { + className, + selected, + children, + hasNoFocusableContent, + ...other + } = props; const tabContentClasses = classNames(`${prefix}--tab-content`, { [className]: className, }); @@ -23,7 +29,8 @@ const TabContent = (props) => { {...other} className={tabContentClasses} selected={selected} - hidden={!selected}> + hidden={!selected} + tabIndex={hasNoFocusableContent ? 0 : undefined}> {children} ); @@ -40,6 +47,11 @@ TabContent.propTypes = { */ className: PropTypes.string, + /** + * Specify if the tab content does not contain focusable content. The tabpanel instead will be included in the focusable dom order. + */ + hasNoFocusableContent: PropTypes.bool, + /** * Specify whether the TabContent is selected */ diff --git a/packages/react/src/components/Tabs/Tabs-story.js b/packages/react/src/components/Tabs/Tabs-story.js index 67490d3fa4f9..615cd49edd44 100644 --- a/packages/react/src/components/Tabs/Tabs-story.js +++ b/packages/react/src/components/Tabs/Tabs-story.js @@ -18,6 +18,7 @@ import { settings } from 'carbon-components'; import classNames from 'classnames'; import './Tabs-story.scss'; import CodeSnippet from '../CodeSnippet'; +import Button from '../Button'; import Tabs from '../Tabs'; import Tab from '../Tab'; import TabsSkeleton from '../Tabs/Tabs.Skeleton'; @@ -123,21 +124,23 @@ export default { export const _Default = () => ( - +

Content for first tab goes here.

Content for second tab goes here.

+
- +

Content for third tab goes here.

+ title="Tab label 4 shows truncation" + hasNoFocusableContent>

Content for fourth tab goes here.

- Custom Label}> + Custom Label} hasNoFocusableContent>

Content for fifth tab goes here.

@@ -150,17 +153,17 @@ _Default.story = { export const Playground = () => (
- +

Content for first tab goes here.

- +

Content for second tab goes here.

- +

Content for third tab goes here.

@@ -180,7 +183,10 @@ export const Playground = () => (
- }> + } + hasNoFocusableContent>

Content for fifth tab goes here.

@@ -191,18 +197,19 @@ export const Playground = () => ( export const Container = () => ( - +

Content for first tab goes here.

- +

Content for second tab goes here.

+ title="Tab label 3 shows truncation" + hasNoFocusableContent>

Content for third tab goes here.

- Custom Label}> + Custom Label} hasNoFocusableContent>

Content for fourth tab goes here.

diff --git a/packages/react/src/components/Tabs/Tabs.js b/packages/react/src/components/Tabs/Tabs.js index 4548c0cacf79..c1513861ed6c 100644 --- a/packages/react/src/components/Tabs/Tabs.js +++ b/packages/react/src/components/Tabs/Tabs.js @@ -439,6 +439,7 @@ export default class Tabs extends React.Component { children, selected, renderContent: Content = TabContent, + hasNoFocusableContent, } = tab.props; return ( @@ -447,7 +448,8 @@ export default class Tabs extends React.Component { className={tabContentClassName} hidden={!selected} selected={selected} - aria-labelledby={tabId}> + aria-labelledby={tabId} + hasNoFocusableContent={hasNoFocusableContent}> {children} ); From 227bb8d73f9e317bf19c7d1c0cbabd5eeaf31e8f Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Thu, 25 Feb 2021 15:44:45 -0600 Subject: [PATCH 2/4] fix(tabcontent): automatically determine if contents is tabbable --- .../__snapshots__/PublicAPI-test.js.snap | 6 -- packages/react/src/components/Tab/Tab.js | 5 -- .../components/TabContent/TabContent-test.js | 55 ++++++++++++++----- .../src/components/TabContent/TabContent.js | 38 ++++++++----- .../react/src/components/Tabs/Tabs-story.js | 29 ++++------ packages/react/src/components/Tabs/Tabs.js | 4 +- 6 files changed, 77 insertions(+), 60 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index ea98b6483a26..13d6768580c9 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -5070,9 +5070,6 @@ Map { "handleTabKeyDown": Object { "type": "func", }, - "hasNoFocusableContent": Object { - "type": "bool", - }, "href": [Function], "id": Object { "type": "string", @@ -5122,9 +5119,6 @@ Map { "className": Object { "type": "string", }, - "hasNoFocusableContent": Object { - "type": "bool", - }, "selected": Object { "type": "bool", }, diff --git a/packages/react/src/components/Tab/Tab.js b/packages/react/src/components/Tab/Tab.js index dc2bf0d764c5..68b3862159f8 100644 --- a/packages/react/src/components/Tab/Tab.js +++ b/packages/react/src/components/Tab/Tab.js @@ -38,11 +38,6 @@ export default class Tab extends React.Component { */ handleTabKeyDown: PropTypes.func, - /** - * Specify if the tab content does not contain focusable content. The tabpanel instead will be included in the focusable dom order. - */ - hasNoFocusableContent: PropTypes.bool, - /** * Provide a string that represents the `href` of the Tab */ diff --git a/packages/react/src/components/TabContent/TabContent-test.js b/packages/react/src/components/TabContent/TabContent-test.js index 6f43b8f46361..0ac53779850b 100644 --- a/packages/react/src/components/TabContent/TabContent-test.js +++ b/packages/react/src/components/TabContent/TabContent-test.js @@ -7,29 +7,54 @@ import React from 'react'; import TabContent from '../TabContent'; -import { shallow } from 'enzyme'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; describe('TabContent', () => { describe('renders as expected', () => { - const wrapper = shallow( - -
content
-
content
-
- ); - it('renders children as expected', () => { - expect(wrapper.props().children.length).toEqual(2); + render( + +
content
+
content
+
+ ); + expect(screen.getByRole('tabpanel').children.length).toEqual(2); }); - it('sets selected if passed in via props', () => { - wrapper.setProps({ selected: true }); - expect(wrapper.props().selected).toEqual(true); + it('sets selected and hidden props with opposite boolean values', () => { + const { rerender } = render( + +
content
+
content
+
+ ); + expect(screen.queryByRole('tabpanel')).not.toBeInTheDocument(); + rerender( + +
content
+
content
+
+ ); + expect(screen.getByRole('tabpanel')).toBeVisible(); }); - it('sets selected and hidden props with opposite boolean values', () => { - wrapper.setProps({ selected: true }); - expect(wrapper.props().hidden).toEqual(false); + it('includes the content container in the tabbable index when no tabble contents is provided', () => { + render( + +

content

+
+ ); + expect(screen.getByRole('tabpanel')).toHaveAttribute('tabindex', '0'); + }); + + it('does not include the content container in the tabbable index when tabble contents is provided', () => { + render( + + content + + ); + expect(screen.getByRole('tabpanel')).not.toHaveAttribute('tabindex', '0'); }); }); }); diff --git a/packages/react/src/components/TabContent/TabContent.js b/packages/react/src/components/TabContent/TabContent.js index 5793aec44ec6..c93d9fdccbb2 100644 --- a/packages/react/src/components/TabContent/TabContent.js +++ b/packages/react/src/components/TabContent/TabContent.js @@ -6,23 +6,37 @@ */ import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useCallback, useState } from 'react'; import classNames from 'classnames'; import { settings } from 'carbon-components'; +import { selectorTabbable } from '../../internal/keyboard/navigation'; const { prefix } = settings; +/** + * Determine if the node within the provided ref contains content that is tabbable. + */ +function useTabbableContent() { + const [hasTabbableContent, setHasTabbableContent] = useState(false); + + const ref = useCallback((node) => { + if (node !== null) { + // we don't need to know which element or how many are tabbable, just that there is _something_ tabbable + node.querySelector(selectorTabbable) + ? setHasTabbableContent(true) + : setHasTabbableContent(false); + } + }, []); + + return [hasTabbableContent, ref]; +} + const TabContent = (props) => { - const { - className, - selected, - children, - hasNoFocusableContent, - ...other - } = props; + const { className, selected, children, ...other } = props; const tabContentClasses = classNames(`${prefix}--tab-content`, { [className]: className, }); + const [hasTabbableContent, ref] = useTabbableContent(); return (
{ className={tabContentClasses} selected={selected} hidden={!selected} - tabIndex={hasNoFocusableContent ? 0 : undefined}> + ref={ref} + tabIndex={hasTabbableContent ? undefined : 0}> {children}
); @@ -47,11 +62,6 @@ TabContent.propTypes = { */ className: PropTypes.string, - /** - * Specify if the tab content does not contain focusable content. The tabpanel instead will be included in the focusable dom order. - */ - hasNoFocusableContent: PropTypes.bool, - /** * Specify whether the TabContent is selected */ diff --git a/packages/react/src/components/Tabs/Tabs-story.js b/packages/react/src/components/Tabs/Tabs-story.js index 615cd49edd44..664a38270285 100644 --- a/packages/react/src/components/Tabs/Tabs-story.js +++ b/packages/react/src/components/Tabs/Tabs-story.js @@ -124,23 +124,22 @@ export default { export const _Default = () => ( - +

Content for first tab goes here.

Content for second tab goes here.

- +

Content for third tab goes here.

+ title="Tab label 4 shows truncation">

Content for fourth tab goes here.

- Custom Label} hasNoFocusableContent> + Custom Label}>

Content for fifth tab goes here.

@@ -153,17 +152,17 @@ _Default.story = { export const Playground = () => (
- +

Content for first tab goes here.

- +

Content for second tab goes here.

- +

Content for third tab goes here.

@@ -183,10 +182,7 @@ export const Playground = () => (
- } - hasNoFocusableContent> + }>

Content for fifth tab goes here.

@@ -197,19 +193,18 @@ export const Playground = () => ( export const Container = () => ( - +

Content for first tab goes here.

- +

Content for second tab goes here.

+ title="Tab label 3 shows truncation">

Content for third tab goes here.

- Custom Label} hasNoFocusableContent> + Custom Label}>

Content for fourth tab goes here.

diff --git a/packages/react/src/components/Tabs/Tabs.js b/packages/react/src/components/Tabs/Tabs.js index c1513861ed6c..4548c0cacf79 100644 --- a/packages/react/src/components/Tabs/Tabs.js +++ b/packages/react/src/components/Tabs/Tabs.js @@ -439,7 +439,6 @@ export default class Tabs extends React.Component { children, selected, renderContent: Content = TabContent, - hasNoFocusableContent, } = tab.props; return ( @@ -448,8 +447,7 @@ export default class Tabs extends React.Component { className={tabContentClassName} hidden={!selected} selected={selected} - aria-labelledby={tabId} - hasNoFocusableContent={hasNoFocusableContent}> + aria-labelledby={tabId}> {children} ); From e9ad7e8541fc959ee13f9789c2d328dca9953984 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Thu, 25 Feb 2021 16:49:19 -0600 Subject: [PATCH 3/4] Update packages/react/src/components/TabContent/TabContent.js Co-authored-by: emyarod --- packages/react/src/components/TabContent/TabContent.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react/src/components/TabContent/TabContent.js b/packages/react/src/components/TabContent/TabContent.js index c93d9fdccbb2..6b3829b8e1e0 100644 --- a/packages/react/src/components/TabContent/TabContent.js +++ b/packages/react/src/components/TabContent/TabContent.js @@ -22,9 +22,7 @@ function useTabbableContent() { const ref = useCallback((node) => { if (node !== null) { // we don't need to know which element or how many are tabbable, just that there is _something_ tabbable - node.querySelector(selectorTabbable) - ? setHasTabbableContent(true) - : setHasTabbableContent(false); + setHasTabbableContent(node.querySelector(selectorTabbable)); } }, []); From bd00df366117334306935340860a8f8fd2040d48 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Tue, 9 Mar 2021 15:02:58 -0600 Subject: [PATCH 4/4] feat(tabs): add useIsomorphicEffect, reconfigure useTabbableContent --- .../src/components/TabContent/TabContent.js | 19 ++++++++++--------- .../react/src/internal/useIsomorphicEffect.js | 7 +++++++ 2 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 packages/react/src/internal/useIsomorphicEffect.js diff --git a/packages/react/src/components/TabContent/TabContent.js b/packages/react/src/components/TabContent/TabContent.js index 6b3829b8e1e0..5269fe427ee9 100644 --- a/packages/react/src/components/TabContent/TabContent.js +++ b/packages/react/src/components/TabContent/TabContent.js @@ -6,27 +6,27 @@ */ import PropTypes from 'prop-types'; -import React, { useCallback, useState } from 'react'; +import React, { useState, useRef } from 'react'; import classNames from 'classnames'; import { settings } from 'carbon-components'; import { selectorTabbable } from '../../internal/keyboard/navigation'; +import useIsomorphicEffect from '../../internal/useIsomorphicEffect'; const { prefix } = settings; /** * Determine if the node within the provided ref contains content that is tabbable. */ -function useTabbableContent() { +function useTabbableContent(ref) { const [hasTabbableContent, setHasTabbableContent] = useState(false); - const ref = useCallback((node) => { - if (node !== null) { - // we don't need to know which element or how many are tabbable, just that there is _something_ tabbable - setHasTabbableContent(node.querySelector(selectorTabbable)); + useIsomorphicEffect(() => { + if (ref.current) { + setHasTabbableContent(ref.current.querySelector(selectorTabbable)); } - }, []); + }); - return [hasTabbableContent, ref]; + return hasTabbableContent; } const TabContent = (props) => { @@ -34,7 +34,8 @@ const TabContent = (props) => { const tabContentClasses = classNames(`${prefix}--tab-content`, { [className]: className, }); - const [hasTabbableContent, ref] = useTabbableContent(); + const ref = useRef(null); + const hasTabbableContent = useTabbableContent(ref); return (