From b6b0949278c7ba173d957bd7ee454bea945566ce Mon Sep 17 00:00:00 2001 From: Sajjad Hashemian Date: Fri, 6 Sep 2019 23:56:53 +0430 Subject: [PATCH 1/3] hookify tab --- src/components/Tabs/components/Tab/Tab.tsx | 168 ++++++++---------- src/components/Tabs/components/Tab/index.ts | 4 +- .../Tabs/components/Tab/tests/Tab.test.tsx | 2 +- 3 files changed, 75 insertions(+), 99 deletions(-) diff --git a/src/components/Tabs/components/Tab/Tab.tsx b/src/components/Tabs/components/Tab/Tab.tsx index 0d308d23232..539e5cd9fd2 100644 --- a/src/components/Tabs/components/Tab/Tab.tsx +++ b/src/components/Tabs/components/Tab/Tab.tsx @@ -1,12 +1,8 @@ -import React from 'react'; +import React, {useEffect, useRef} from 'react'; import {focusFirstFocusableNode} from '@shopify/javascript-utilities/focus'; import {classNames} from '../../../../utilities/css'; import {UnstyledLink} from '../../../UnstyledLink'; -import { - withAppProvider, - WithAppProviderProps, -} from '../../../../utilities/with-app-provider'; import {handleMouseUpByBlurring} from '../../../../utilities/focus'; import styles from '../../Tabs.scss'; @@ -24,17 +20,24 @@ export interface TabProps { onClick?(id: string): void; } -type CombinedProps = TabProps & WithAppProviderProps; - -class Tab extends React.PureComponent { - private node: HTMLElement | null = null; - +export function Tab({ + id, + focused, + siblingTabHasFocus, + children, + onClick, + selected, + url, + panelID, + measuring, + accessibilityLabel, +}: TabProps) { + const wasSelected = useRef(selected); + const node = useRef(null); // A tab can start selected when it is moved from the disclosure dropdown // into the main list, so we need to send focus from the tab to the panel // on mount and update - componentDidMount() { - const {id, measuring, selected, panelID, focused} = this.props; - + useEffect(() => { if (measuring) { return; } @@ -50,96 +53,75 @@ class Tab extends React.PureComponent { if (itemHadFocus && selected && panelID != null) { focusPanelID(panelID); } - } - componentDidUpdate(previousProps: TabProps) { - const {selected: wasSelected} = previousProps; - const {focused, measuring, selected, panelID} = this.props; + /* eslint-disable-next-line react-hooks/exhaustive-deps */ + }, []); + useEffect(() => { if (measuring) { return; } - if (selected && !wasSelected && panelID != null) { + if (selected && !wasSelected.current && panelID != null) { focusPanelID(panelID); - } else if (focused && this.node != null) { - focusFirstFocusableNode(this.node); + } else if (focused && node.current != null) { + focusFirstFocusableNode(node.current); } - } - render() { - const { - id, - focused, - siblingTabHasFocus, - children, - onClick, - selected, - url, - panelID, - measuring, - accessibilityLabel, - } = this.props; - - const handleClick = onClick && onClick.bind(null, id); - - const className = classNames( - styles.Tab, - selected && styles['Tab-selected'], - ); - - let tabIndex: 0 | -1; - - if (selected && !siblingTabHasFocus && !measuring) { - tabIndex = 0; - } else if (focused && !measuring) { - tabIndex = 0; - } else { - tabIndex = -1; - } + wasSelected.current = selected; + }, [focused, measuring, panelID, selected]); + + const handleClick = onClick && onClick.bind(null, id); + + const className = classNames(styles.Tab, selected && styles['Tab-selected']); - const markup = url ? ( - - {children} - - ) : ( - - ); - - return ( -
  • - {markup} -
    • - ); + let tabIndex: 0 | -1; + + if (selected && !siblingTabHasFocus && !measuring) { + tabIndex = 0; + } else if (focused && !measuring) { + tabIndex = 0; + } else { + tabIndex = -1; } - private setNode = (node: HTMLElement | null) => { - this.node = node; - }; + const markup = url ? ( + + {children} + + ) : ( + + ); + + return ( +
  • + {markup} +
    • + ); } function focusPanelID(panelID: string) { @@ -148,7 +130,3 @@ function focusPanelID(panelID: string) { panel.focus(); } } - -// Use named export once withAppProvider is refactored away -// eslint-disable-next-line import/no-default-export -export default withAppProvider()(Tab); diff --git a/src/components/Tabs/components/Tab/index.ts b/src/components/Tabs/components/Tab/index.ts index 8af86c598e2..ec917d10aae 100644 --- a/src/components/Tabs/components/Tab/index.ts +++ b/src/components/Tabs/components/Tab/index.ts @@ -1,3 +1 @@ -import Tab, {TabProps} from './Tab'; - -export {Tab, TabProps}; +export {Tab, TabProps} from './Tab'; diff --git a/src/components/Tabs/components/Tab/tests/Tab.test.tsx b/src/components/Tabs/components/Tab/tests/Tab.test.tsx index 4003f7dd821..7620b2d6509 100644 --- a/src/components/Tabs/components/Tab/tests/Tab.test.tsx +++ b/src/components/Tabs/components/Tab/tests/Tab.test.tsx @@ -1,6 +1,6 @@ import React from 'react'; import {mountWithAppProvider} from 'test-utilities/legacy'; -import Tab from '../Tab'; +import {Tab} from '../Tab'; describe('', () => { it('has the tab role', () => { From 9bf7a66cc0165b4636400711689e7c904e1a2576 Mon Sep 17 00:00:00 2001 From: Sajjad Hashemian Date: Wed, 11 Sep 2019 22:43:30 +0430 Subject: [PATCH 2/3] add changelog entry --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index fcaf4bceb4f..5ed3face47e 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -27,5 +27,6 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Code quality - Added `MediaQueryProvider` to ease the use of media queries and reduce duplication ([#2117](https://github.com/Shopify/polaris-react/pull/2117)) +- Migrated `Tab` to use hooks instead of `withAppProvider` ([#2096](https://github.com/Shopify/polaris-react/pull/2096)) ### Deprecations From 5e79f5b306742fd66b0333dd2ea94d0e978dc7d8 Mon Sep 17 00:00:00 2001 From: Sajjad Hashemian Date: Thu, 26 Sep 2019 18:00:29 +0330 Subject: [PATCH 3/3] merge useEffects --- src/components/Tabs/components/Tab/Tab.tsx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/components/Tabs/components/Tab/Tab.tsx b/src/components/Tabs/components/Tab/Tab.tsx index 539e5cd9fd2..d0fad474261 100644 --- a/src/components/Tabs/components/Tab/Tab.tsx +++ b/src/components/Tabs/components/Tab/Tab.tsx @@ -33,7 +33,9 @@ export function Tab({ accessibilityLabel, }: TabProps) { const wasSelected = useRef(selected); + const panelFocused = useRef(false); const node = useRef(null); + // A tab can start selected when it is moved from the disclosure dropdown // into the main list, so we need to send focus from the tab to the panel // on mount and update @@ -50,16 +52,9 @@ export function Tab({ // If we just check for selected, the panel for the active tab will // be focused on page load, which we don’t want - if (itemHadFocus && selected && panelID != null) { + if (itemHadFocus && selected && panelID != null && !panelFocused.current) { focusPanelID(panelID); - } - - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - }, []); - - useEffect(() => { - if (measuring) { - return; + panelFocused.current = true; } if (selected && !wasSelected.current && panelID != null) { @@ -69,7 +64,7 @@ export function Tab({ } wasSelected.current = selected; - }, [focused, measuring, panelID, selected]); + }, [focused, id, measuring, panelID, selected]); const handleClick = onClick && onClick.bind(null, id);