From 5a6dd9b9fed58ffd5efc9dd07090073fcc5d0efe Mon Sep 17 00:00:00 2001 From: Tom Schindl Date: Wed, 8 Jun 2022 13:43:14 +0200 Subject: [PATCH 1/5] fix flickering line in collapsed mode instead of modifying the DOM all nodes are kept in the DOM and are hiden/shown upon request --- packages/@react-spectrum/tabs/src/Tabs.tsx | 60 +++++++++++----------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/packages/@react-spectrum/tabs/src/Tabs.tsx b/packages/@react-spectrum/tabs/src/Tabs.tsx index 9f43229e04f..f5e96db489d 100644 --- a/packages/@react-spectrum/tabs/src/Tabs.tsx +++ b/packages/@react-spectrum/tabs/src/Tabs.tsx @@ -12,7 +12,7 @@ import {classNames, SlotProvider, unwrapDOMRef, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {DOMProps, DOMRef, Node, Orientation} from '@react-types/shared'; -import {filterDOMProps, useValueEffect} from '@react-aria/utils'; +import {filterDOMProps} from '@react-aria/utils'; import {FocusRing} from '@react-aria/focus'; import {Item, Picker} from '@react-spectrum/picker'; import {ListCollection, SingleSelectListState} from '@react-stately/list'; @@ -64,7 +64,7 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef(); const [tabListState, setTabListState] = useState>(null); @@ -80,29 +80,16 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef { - let computeShouldCollapse = () => { - if (wrapperRef.current) { - let tabsComponent = wrapperRef.current; - let tabs = tablistRef.current.querySelectorAll('[role="tab"]'); - let lastTab = tabs[tabs.length - 1]; - - let end = direction === 'rtl' ? 'left' : 'right'; - let farEdgeTabList = tabsComponent.getBoundingClientRect()[end]; - let farEdgeLastTab = lastTab?.getBoundingClientRect()[end]; - let shouldCollapse = direction === 'rtl' ? farEdgeLastTab < farEdgeTabList : farEdgeTabList < farEdgeLastTab; - - return shouldCollapse; - } - }; - - if (orientation !== 'vertical') { - setCollapse(function* () { - // Make Tabs render in non-collapsed state - yield false; - - // Compute if Tabs should collapse and update - yield computeShouldCollapse(); - }); + if (wrapperRef.current && orientation !== 'vertical') { + let tabsComponent = wrapperRef.current; + let tabs = tablistRef.current.querySelectorAll('[role="tab"]'); + let lastTab = tabs[tabs.length - 1]; + + let end = direction === 'rtl' ? 'left' : 'right'; + let farEdgeTabList = tabsComponent.getBoundingClientRect()[end]; + let farEdgeLastTab = lastTab?.getBoundingClientRect()[end]; + let shouldCollapse = direction === 'rtl' ? farEdgeLastTab < farEdgeTabList : farEdgeTabList < farEdgeLastTab; + setCollapse(shouldCollapse); } }, [tablistRef, wrapperRef, direction, orientation, setCollapse]); @@ -270,6 +257,8 @@ export function TabList(props: SpectrumTabListProps) { }, [state.disabledKeys, state.selectedItem, state.selectedKey, props.children]); let stylePropsForVertical = orientation === 'vertical' ? styleProps : {}; + let style : React.CSSProperties = collapse ? {visibility: 'hidden', position: 'absolute'} : {}; + let tabListclassName = classNames(styles, 'spectrum-TabsPanel-tabs'); const tabContent = (
(props: SpectrumTabListProps) { }, orientation === 'vertical' && styleProps.className ) - }> + } + aria-hidden={collapse} + style={style}> {[...state.collection].map((item) => ( ))} @@ -309,7 +300,8 @@ export function TabList(props: SpectrumTabListProps) { 'spectrum-TabsPanel-collapseWrapper', styleProps.className )}> - {collapse ? : tabContent} + + {tabContent}
); } @@ -359,7 +351,8 @@ interface TabPickerProps extends Omit, 'children'> { density?: 'compact' | 'regular', isEmphasized?: boolean, state: SingleSelectListState, - className?: string + className?: string, + visible: boolean } function TabPicker(props: TabPickerProps) { @@ -372,7 +365,8 @@ function TabPicker(props: TabPickerProps) { 'aria-label': ariaLabel, density, className, - id + id, + visible } = props; let ref = useRef(); @@ -394,6 +388,8 @@ function TabPicker(props: TabPickerProps) { 'aria-label': ariaLabel }; + const style : React.CSSProperties = visible ? {} : {visibility: 'hidden', position: 'absolute'}; + // TODO: Figure out if tabListProps should go onto the div here, v2 doesn't do it return (
(props: TabPickerProps) { 'spectrum-Tabs--emphasized': isEmphasized }, className - )}> + )} + style={style} + aria-hidden={!visible}> (props: TabPickerProps) { items={items} ref={ref} isQuiet - isDisabled={isDisabled} + isDisabled={!visible || isDisabled} selectedKey={state.selectedKey} disabledKeys={state.disabledKeys} onSelectionChange={state.setSelectedKey}> From 6c6829a9f4241db04dc8ea20ece329e7511cbf26 Mon Sep 17 00:00:00 2001 From: Tom Schindl Date: Wed, 8 Jun 2022 13:43:46 +0200 Subject: [PATCH 2/5] fix tests to pass an aria label --- packages/@react-spectrum/tabs/test/Tabs.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/tabs/test/Tabs.test.js b/packages/@react-spectrum/tabs/test/Tabs.test.js index 944e0052fd6..8e3828a9ddd 100644 --- a/packages/@react-spectrum/tabs/test/Tabs.test.js +++ b/packages/@react-spectrum/tabs/test/Tabs.test.js @@ -28,7 +28,7 @@ function renderComponent(props = {}, itemProps) { let {items = defaultItems} = props; return render( - + {item => ( @@ -223,7 +223,7 @@ describe('Tabs', function () { it('does not generate conflicting ids between multiple tabs instances', function () { let tree = render( - + {defaultItems.map(item => ( @@ -237,7 +237,7 @@ describe('Tabs', function () { ))} - + {defaultItems.map(item => ( @@ -316,7 +316,7 @@ describe('Tabs', function () { tree.rerender( - + {item => ( @@ -499,7 +499,7 @@ describe('Tabs', function () { rerender( - + {item => ( @@ -618,7 +618,7 @@ describe('Tabs', function () { it('tabpanel should have tabIndex=0 only when there are no focusable elements', async function () { let {getByRole, getAllByRole} = render( - + Tab 1 Tab 2 @@ -653,7 +653,7 @@ describe('Tabs', function () { it('TabPanel children do not share values between panels', () => { let {getByDisplayValue, getAllByRole, getByTestId} = render( - + Tab 1 Tab 2 From a7c137ec156b9243b9d0a4e9f52ae4ce91e55ee2 Mon Sep 17 00:00:00 2001 From: Tom Schindl Date: Mon, 13 Jun 2022 22:15:31 +0200 Subject: [PATCH 3/5] fix remarks and rename property --- packages/@react-spectrum/tabs/src/Tabs.tsx | 32 ++++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/@react-spectrum/tabs/src/Tabs.tsx b/packages/@react-spectrum/tabs/src/Tabs.tsx index f5e96db489d..5c3993855b9 100644 --- a/packages/@react-spectrum/tabs/src/Tabs.tsx +++ b/packages/@react-spectrum/tabs/src/Tabs.tsx @@ -36,7 +36,7 @@ interface TabsContext { tabListState: TabListState, setTabListState: (state: TabListState) => void, selectedTab: HTMLElement, - collapse: boolean + collapsed: boolean }, refs: { wrapperRef: MutableRefObject, @@ -64,7 +64,7 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef(); const [tabListState, setTabListState] = useState>(null); @@ -77,7 +77,7 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef { if (wrapperRef.current && orientation !== 'vertical') { @@ -89,9 +89,9 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef { checkShouldCollapse(); @@ -105,14 +105,14 @@ function Tabs(props: SpectrumTabsProps, ref: DOMRef @@ -242,7 +242,7 @@ export function TabList(props: SpectrumTabListProps) { const tabContext = useContext(TabContext); const {refs, tabState, tabProps, tabPanelProps} = tabContext; const {isQuiet, density, isDisabled, isEmphasized, orientation} = tabProps; - const {selectedTab, collapse, setTabListState} = tabState; + const {selectedTab, collapsed, setTabListState} = tabState; const {tablistRef, wrapperRef} = refs; // Pass original Tab props but override children to create the collection. const state = useTabListState({...tabProps, children: props.children}); @@ -255,14 +255,18 @@ export function TabList(props: SpectrumTabListProps) { setTabListState(state); // eslint-disable-next-line react-hooks/exhaustive-deps }, [state.disabledKeys, state.selectedItem, state.selectedKey, props.children]); - let stylePropsForVertical = orientation === 'vertical' ? styleProps : {}; - let style : React.CSSProperties = collapse ? {visibility: 'hidden', position: 'absolute'} : {}; + let collapseStyle : React.CSSProperties = collapsed && orientation !== 'vertical' ? {visibility: 'hidden', position: 'absolute'} : {}; + let stylePropsFinal = orientation === 'vertical' ? styleProps : {style: collapseStyle}; + + if (collapsed && orientation !== 'vertical') { + tabListProps['aria-hidden'] = true; + } let tabListclassName = classNames(styles, 'spectrum-TabsPanel-tabs'); const tabContent = (
(props: SpectrumTabListProps) { }, orientation === 'vertical' && styleProps.className ) - } - aria-hidden={collapse} - style={style}> + }> {[...state.collection].map((item) => ( ))} @@ -300,7 +302,7 @@ export function TabList(props: SpectrumTabListProps) { 'spectrum-TabsPanel-collapseWrapper', styleProps.className )}> - + {tabContent}
); From cfd64caa1946865222def065f9fb957255d96913 Mon Sep 17 00:00:00 2001 From: Tom Schindl Date: Tue, 28 Jun 2022 16:04:17 +0200 Subject: [PATCH 4/5] Update packages/@react-spectrum/tabs/src/Tabs.tsx Co-authored-by: Robert Snow --- packages/@react-spectrum/tabs/src/Tabs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tabs/src/Tabs.tsx b/packages/@react-spectrum/tabs/src/Tabs.tsx index 5c3993855b9..64e045d1d2e 100644 --- a/packages/@react-spectrum/tabs/src/Tabs.tsx +++ b/packages/@react-spectrum/tabs/src/Tabs.tsx @@ -408,7 +408,7 @@ function TabPicker(props: TabPickerProps) { className )} style={style} - aria-hidden={!visible}> + aria-hidden={visible ? undefined : true}> Date: Wed, 29 Jun 2022 20:42:56 +0200 Subject: [PATCH 5/5] fixed overflow behavior --- packages/@react-spectrum/tabs/src/Tabs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tabs/src/Tabs.tsx b/packages/@react-spectrum/tabs/src/Tabs.tsx index 64e045d1d2e..9f015e8f913 100644 --- a/packages/@react-spectrum/tabs/src/Tabs.tsx +++ b/packages/@react-spectrum/tabs/src/Tabs.tsx @@ -256,7 +256,7 @@ export function TabList(props: SpectrumTabListProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [state.disabledKeys, state.selectedItem, state.selectedKey, props.children]); - let collapseStyle : React.CSSProperties = collapsed && orientation !== 'vertical' ? {visibility: 'hidden', position: 'absolute'} : {}; + let collapseStyle : React.CSSProperties = collapsed && orientation !== 'vertical' ? {maxWidth: 'calc(100% + 1px)', overflow: 'hidden', visibility: 'hidden', position: 'absolute'} : {maxWidth: 'calc(100% + 1px)'}; let stylePropsFinal = orientation === 'vertical' ? styleProps : {style: collapseStyle}; if (collapsed && orientation !== 'vertical') {