Skip to content

Commit

Permalink
remove forceRerender code
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
RobinMalfait committed Sep 12, 2022
1 parent 46a7ab6 commit 74fe786
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 18 deletions.
50 changes: 50 additions & 0 deletions packages/@headlessui-react/src/components/tabs/tabs.test.tsx
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 74fe786

Please sign in to comment.