Skip to content

Commit

Permalink
3199 resize window tabs flicker (#3202)
Browse files Browse the repository at this point in the history 
* fix flickering line in collapsed mode

instead of modifying the DOM all nodes are kept in
the DOM and are hiden/shown upon request

* fix tests to pass an aria label

* fix remarks and rename property

* Update packages/@react-spectrum/tabs/src/Tabs.tsx

Co-authored-by: Robert Snow <snowystinger@gmail.com>

* fixed overflow behavior

Co-authored-by: Robert Snow <snowystinger@gmail.com>
Co-authored-by: Robert Snow <rsnow@adobe.com>
Co-authored-by: Daniel Lu <dl1644@gmail.com>
  • Loading branch information
@tomsontom @snowystinger @LFDanLu
4 people committed Jul 23, 2022
1 parent fe9aeac commit 583b6fb
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 45 deletions.
76 changes: 38 additions & 38 deletions packages/@react-spectrum/tabs/src/Tabs.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -36,7 +36,7 @@ interface TabsContext<T> {
tabListState: TabListState<T>,
setTabListState: (state: TabListState<T>) => void,
selectedTab: HTMLElement,
collapse: boolean
collapsed: boolean
},
refs: {
wrapperRef: MutableRefObject<HTMLDivElement>,
Expand Down Expand Up @@ -64,7 +64,7 @@ function Tabs<T extends object>(props: SpectrumTabsProps<T>, ref: DOMRef<HTMLDiv

let {direction} = useLocale();
let {styleProps} = useStyleProps(otherProps);
let [collapse, setCollapse] = useValueEffect(false);
let [collapsed, setCollapsed] = useState(false);
let [selectedTab, setSelectedTab] = useState<HTMLElement>();
const [tabListState, setTabListState] = useState<TabListState<T>>(null);

Expand All @@ -77,34 +77,21 @@ function Tabs<T extends object>(props: SpectrumTabsProps<T>, ref: DOMRef<HTMLDiv
}
}
// collapse is in the dep array so selectedTab can be updated for TabLine positioning
}, [children, tabListState?.selectedKey, collapse, tablistRef]);
}, [children, tabListState?.selectedKey, collapsed, tablistRef]);

let checkShouldCollapse = useCallback(() => {
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;
setCollapsed(shouldCollapse);
}
}, [tablistRef, wrapperRef, direction, orientation, setCollapse]);
}, [tablistRef, wrapperRef, direction, orientation, setCollapsed]);

useEffect(() => {
checkShouldCollapse();
Expand All @@ -118,14 +105,14 @@ function Tabs<T extends object>(props: SpectrumTabsProps<T>, ref: DOMRef<HTMLDiv

// When the tabs are collapsed, the tabPanel should be labelled by the Picker button element.
let collapsibleTabListId = useId();
if (collapse && orientation !== 'vertical') {
if (collapsed && orientation !== 'vertical') {
tabPanelProps['aria-labelledby'] = collapsibleTabListId;
}
return (
<TabContext.Provider
value={{
tabProps: {...props, orientation, density},
tabState: {tabListState, setTabListState, selectedTab, collapse},
tabState: {tabListState, setTabListState, selectedTab, collapsed},
refs: {tablistRef, wrapperRef},
tabPanelProps
}}>
Expand Down Expand Up @@ -255,7 +242,7 @@ export function TabList<T>(props: SpectrumTabListProps<T>) {
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});
Expand All @@ -268,12 +255,18 @@ export function TabList<T>(props: SpectrumTabListProps<T>) {
setTabListState(state);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [state.disabledKeys, state.selectedItem, state.selectedKey, props.children]);
let stylePropsForVertical = orientation === 'vertical' ? styleProps : {};

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') {
tabListProps['aria-hidden'] = true;
}

let tabListclassName = classNames(styles, 'spectrum-TabsPanel-tabs');
const tabContent = (
<div
{...stylePropsForVertical}
{...stylePropsFinal}
{...tabListProps}
ref={tablistRef}
className={classNames(
Expand Down Expand Up @@ -309,7 +302,8 @@ export function TabList<T>(props: SpectrumTabListProps<T>) {
'spectrum-TabsPanel-collapseWrapper',
styleProps.className
)}>
{collapse ? <TabPicker {...props} {...tabProps} id={tabPanelProps['aria-labelledby']} state={state} className={tabListclassName} /> : tabContent}
<TabPicker {...props} {...tabProps} visible={collapsed} id={tabPanelProps['aria-labelledby']} state={state} className={tabListclassName} />
{tabContent}
</div>
);
}
Expand Down Expand Up @@ -359,7 +353,8 @@ interface TabPickerProps<T> extends Omit<SpectrumPickerProps<T>, 'children'> {
density?: 'compact' | 'regular',
isEmphasized?: boolean,
state: SingleSelectListState<T>,
className?: string
className?: string,
visible: boolean
}

function TabPicker<T>(props: TabPickerProps<T>) {
Expand All @@ -372,7 +367,8 @@ function TabPicker<T>(props: TabPickerProps<T>) {
'aria-label': ariaLabel,
density,
className,
id
id,
visible
} = props;

let ref = useRef();
Expand All @@ -394,6 +390,8 @@ function TabPicker<T>(props: TabPickerProps<T>) {
'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 (
<div
Expand All @@ -408,7 +406,9 @@ function TabPicker<T>(props: TabPickerProps<T>) {
'spectrum-Tabs--emphasized': isEmphasized
},
className
)}>
)}
style={style}
aria-hidden={visible ? undefined : true}>
<SlotProvider
slots={{
icon: {
Expand All @@ -425,7 +425,7 @@ function TabPicker<T>(props: TabPickerProps<T>) {
items={items}
ref={ref}
isQuiet
isDisabled={isDisabled}
isDisabled={!visible || isDisabled}
selectedKey={state.selectedKey}
disabledKeys={state.disabledKeys}
onSelectionChange={state.setSelectedKey}>
Expand Down
14 changes: 7 additions & 7 deletions packages/@react-spectrum/tabs/test/Tabs.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function renderComponent(props = {}, itemProps) {
let {items = defaultItems} = props;
return render(
<Provider theme={theme}>
<Tabs {...props} items={items}>
<Tabs aria-label="Tab Sample" {...props} items={items}>
<TabList>
{item => (
<Item {...itemProps} key={item.name} title={item.name || item.children} />
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('Tabs', function () {
it('does not generate conflicting ids between multiple tabs instances', function () {
let tree = render(
<Provider theme={theme}>
<Tabs>
<Tabs aria-label="Tab Sample">
<TabList>
{defaultItems.map(item => (
<Item key={item.name} title={item.name || item.children} />
Expand All @@ -236,7 +236,7 @@ describe('Tabs', function () {
))}
</TabPanels>
</Tabs>
<Tabs>
<Tabs aria-label="Tab Sample 2">
<TabList>
{defaultItems.map(item => (
<Item key={item.name} title={item.name || item.children} />
Expand Down Expand Up @@ -315,7 +315,7 @@ describe('Tabs', function () {

tree.rerender(
<Provider theme={theme}>
<Tabs disabledKeys={[defaultItems[0].name]} onSelectionChange={onSelectionChange} items={defaultItems.slice(0, 1)}>
<Tabs aria-label="Tab Example" disabledKeys={[defaultItems[0].name]} onSelectionChange={onSelectionChange} items={defaultItems.slice(0, 1)}>
<TabList>
{item => (
<Item key={item.name} title={item.name || item.children} />
Expand Down Expand Up @@ -498,7 +498,7 @@ describe('Tabs', function () {

rerender(
<Provider theme={theme}>
<Tabs items={newItems} orientation="vertical">
<Tabs aria-label="Tab Example" items={newItems} orientation="vertical">
<TabList>
{item => (
<Item key={item.name} title={item.name || item.children} />
Expand Down Expand Up @@ -617,7 +617,7 @@ describe('Tabs', function () {
it('tabpanel should have tabIndex=0 only when there are no focusable elements', async function () {
let {getByRole, getAllByRole} = render(
<Provider theme={theme}>
<Tabs maxWidth={500}>
<Tabs aria-label="Tab Example" maxWidth={500}>
<TabList>
<Item>Tab 1</Item>
<Item>Tab 2</Item>
Expand Down Expand Up @@ -652,7 +652,7 @@ describe('Tabs', function () {
it('TabPanel children do not share values between panels', () => {
let {getByDisplayValue, getAllByRole, getByTestId} = render(
<Provider theme={theme}>
<Tabs maxWidth={500}>
<Tabs aria-label="Tab Example" maxWidth={500}>
<TabList>
<Item>Tab 1</Item>
<Item>Tab 2</Item>
Expand Down

0 comments on commit 583b6fb

Please sign in to comment.