Skip to content

Commit

Permalink
Remove forceRerender from Tab component (#1846)
Browse files Browse the repository at this point in the history
* remove `forceRerender` code

This was necessary to ensure the `Panel` and the `Tab` were properly
connected with eachother because it could happen that the `Tab` renders
but the corresponding `Panel` is not the active one which means that it
didn't have a DOM node and no `id` attached.

Whenever new `Tab` became active, it rerendered but the `Panel` wasn't
available yet, after that the `Panel` rendered and an `id` was available
but the actual `Tab` was already rendered so there was no link between
them.

We then forced a re-render because now the `Panel` does have a DOM node
ref attached and the `aria-labelledby` could be filled in.

However, in #1837 we fixed an issue where the order of `Tab` elements
with their corresponding `Panel` elements weren't always correct. To fix
this we ensured that the `Panel` always rendered a `<Hidden />`
component which means that a DOM node is always available.

This now means that we can get rid of the `forceRerender`.

* update changelog
  • Loading branch information
RobinMalfait committed Sep 12, 2022
1 parent 46a7ab6 commit 1a145fd
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 18 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `<fieldset disabled>` check to radio group options in React ([#1835](https://github.com/tailwindlabs/headlessui/pull/1835))
- Ensure `Tab` order stays consistent, and the currently active `Tab` stays active ([#1837](https://github.com/tailwindlabs/headlessui/pull/1837))
- Ensure `Combobox.Label` is properly linked when rendered after `Combobox.Button` and `Combobox.Input` components ([#1838](https://github.com/tailwindlabs/headlessui/pull/1838))
- Remove `forceRerender` from `Tab` component ([#1846](https://github.com/tailwindlabs/headlessui/pull/1846))

## [1.7.0] - 2022-09-06

Expand Down
50 changes: 50 additions & 0 deletions packages/@headlessui-react/src/components/tabs/tabs.test.tsx
Expand Up @@ -168,6 +168,56 @@ describe('Rendering', () => {
)

describe('`renderProps`', () => {
it(
'should be possible to render using as={Fragment}',
suppressConsoleLogs(async () => {
render(
<Tab.Group>
<Tab.List>
<Tab as={React.Fragment}>
<button>Tab 1</button>
</Tab>
<Tab>Tab 2</Tab>
<Tab>Tab 3</Tab>
</Tab.List>

<Tab.Panels>
<Tab.Panel>Content 1</Tab.Panel>
<Tab.Panel>Content 2</Tab.Panel>
<Tab.Panel>Content 3</Tab.Panel>
</Tab.Panels>
</Tab.Group>
)

assertTabs({ active: 0, tabContents: 'Tab 1', panelContents: 'Content 1' })
})
)

it(
'should be possible to render using multiple as={Fragment}',
suppressConsoleLogs(async () => {
render(
<Tab.Group>
<Tab.List>
<Tab as={React.Fragment}>
<button>Tab 1</button>
</Tab>
<Tab as={React.Fragment}>
<button>Tab 2</button>
</Tab>
</Tab.List>

<Tab.Panels>
<Tab.Panel>Content 1</Tab.Panel>
<Tab.Panel>Content 2</Tab.Panel>
</Tab.Panels>
</Tab.Group>
)

assertTabs({ active: 0, tabContents: 'Tab 1', panelContents: 'Content 1' })
})
)

it(
'should expose the `selectedIndex` on the `Tab.Group` component',
suppressConsoleLogs(async () => {
Expand Down
20 changes: 2 additions & 18 deletions packages/@headlessui-react/src/components/tabs/tabs.tsx
Expand Up @@ -44,8 +44,6 @@ enum ActionTypes {

RegisterPanel,
UnregisterPanel,

ForceRerender,
}

type Actions =
Expand All @@ -54,7 +52,6 @@ type Actions =
| { type: ActionTypes.UnregisterTab; tab: MutableRefObject<HTMLElement | null> }
| { type: ActionTypes.RegisterPanel; panel: MutableRefObject<HTMLElement | null> }
| { type: ActionTypes.UnregisterPanel; panel: MutableRefObject<HTMLElement | null> }
| { type: ActionTypes.ForceRerender }

let reducers: {
[P in ActionTypes]: (
Expand Down Expand Up @@ -110,9 +107,6 @@ let reducers: {
[ActionTypes.UnregisterPanel](state, action) {
return { ...state, panels: state.panels.filter((panel) => panel !== action.panel) }
},
[ActionTypes.ForceRerender](state) {
return { ...state }
},
}

let TabsSSRContext = createContext<MutableRefObject<{ tabs: string[]; panels: string[] }> | null>(
Expand Down Expand Up @@ -154,7 +148,6 @@ let TabsActionsContext = createContext<{
registerTab(tab: MutableRefObject<HTMLElement | null>): () => void
registerPanel(panel: MutableRefObject<HTMLElement | null>): () => void
change(index: number): void
forceRerender(): void
} | null>(null)
TabsActionsContext.displayName = 'TabsActionsContext'

Expand Down Expand Up @@ -229,9 +222,6 @@ let Tabs = forwardRefWithAs(function Tabs<TTag extends ElementType = typeof DEFA
dispatch({ type: ActionTypes.RegisterPanel, panel })
return () => dispatch({ type: ActionTypes.UnregisterPanel, panel })
},
forceRerender() {
dispatch({ type: ActionTypes.ForceRerender })
},
change(index: number) {
if (realSelectedIndex.current !== index) {
onChangeRef.current(index)
Expand Down Expand Up @@ -335,10 +325,7 @@ let TabRoot = forwardRefWithAs(function Tab<TTag extends ElementType = typeof DE
let SSRContext = useSSRTabsCounter('Tab')

let internalTabRef = useRef<HTMLElement | null>(null)
let tabRef = useSyncRefs(internalTabRef, ref, (element) => {
if (!element) return
requestAnimationFrame(() => actions.forceRerender())
})
let tabRef = useSyncRefs(internalTabRef, ref)

useIsoMorphicEffect(() => actions.registerTab(internalTabRef), [actions, internalTabRef])

Expand Down Expand Up @@ -492,10 +479,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE

let id = `headlessui-tabs-panel-${useId()}`
let internalPanelRef = useRef<HTMLElement | null>(null)
let panelRef = useSyncRefs(internalPanelRef, ref, (element) => {
if (!element) return
requestAnimationFrame(() => actions.forceRerender())
})
let panelRef = useSyncRefs(internalPanelRef, ref)

useIsoMorphicEffect(() => actions.registerPanel(internalPanelRef), [actions, internalPanelRef])

Expand Down

0 comments on commit 1a145fd

Please sign in to comment.