Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update useLayoutEffect dependencies to ensure TabLine resizes #3688

Merged
merged 9 commits into from Jan 19, 2023
Merged

Update useLayoutEffect dependencies to ensure TabLine resizes #3688

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9032a0e
Update useLayoutEffect dependency array to ensure TabLine resizes pro…
tywayne Oct 27, 2022
2cb2ffe
useResizeObserver to catch any changes to selectedTab size
tywayne Oct 27, 2022
b8059f6
Merge branch 'main' into issue-2004
reidbarber Nov 21, 2022
856d73c
Merge branch 'main' into issue-2004
LFDanLu Nov 22, 2022
1b84c88
Merge branch 'main' into issue-2004
snowystinger Jan 18, 2023
70007d6
handle other tabs pushing the selected tab
snowystinger Jan 18, 2023
6b4715a
fix lint
snowystinger Jan 18, 2023
8bb2141
Merge branch 'main' into issue-2004
LFDanLu Jan 19, 2023
aaecc81
Merge branch 'main' into issue-2004
snowystinger Jan 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@react-spectrum/tabs/src/Tabs.tsx
View file
Open in desktop
Expand Up @@ -227,7 +227,7 @@ function TabLine(props: TabLineProps) {
setStyle(styleObj);
}

}, [direction, setStyle, selectedTab, orientation, scale, selectedKey]);
}, [direction, setStyle, selectedTab, selectedTab?.offsetWidth, selectedTab?.offsetLeft, orientation, scale, selectedKey]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may not update in all cases, only if we get lucky that the size update is tied to a state update. I think what we really want to do is attach a resizeObserver

An example of this changing outside of a state update is a font change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, yep. I do remember seeing a comment on the issue about it not updating after a font change or FOUT type of thing, I just forgot to solve for it.

I'll take a look at the useResizeObserver util and se what I can do 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snowystinger Okay, this should be more resilient now. Moved the main functionality into a callback, and pass that to an observer as well as keeping a useLayoutEffect to catch the regular state changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will look again soon. We're in the midst of release planning and some other things at work, so apologies if it takes a few days


return <div className={classNames(styles, 'spectrum-Tabs-selectionIndicator')} role="presentation" style={style} />;
}
Expand Down
22 changes: 22 additions & 0 deletions packages/@react-spectrum/tabs/stories/Tabs.stories.tsx
View file
Open in desktop
Expand Up @@ -250,6 +250,28 @@ storiesOf('Tabs', module)
);
}
)
.add(
'Tab 1 controlled TabList item',
() => {
let [tab1Text, setTab1Text] = useState('Tab 1');

return (
<Flex minHeight={400} minWidth={400} direction="column">
<TextField label="Tab Title" value={tab1Text} onChange={setTab1Text} />
<Tabs maxWidth={500}>
<TabList>
<Item>{tab1Text}</Item>
<Item>Tab 2</Item>
</TabList>
<TabPanels>
<Item>Tab 1 Content</Item>
<Item>Tab 2 Content</Item>
</TabPanels>
</Tabs>
</Flex>
);
}
)
.add(
'changing selection programatically',
() => (
Expand Down