From c5a064af51dc8f509f4a9dc7502554f1d3a3c5be Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 29 Oct 2026 13:55:20 -0500 Subject: [PATCH] Fix QueryParamTabs switching tab before nav commits --- .../QueryParamTabs.browser.spec.tsx | 72 +++++++++++++++++++ app/components/QueryParamTabs.tsx | 10 ++- 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 app/components/QueryParamTabs.browser.spec.tsx diff --git a/app/components/QueryParamTabs.browser.spec.tsx b/app/components/QueryParamTabs.browser.spec.tsx new file mode 100644 index 0000000000..8d1bc45526 --- /dev/null +++ b/app/components/QueryParamTabs.browser.spec.tsx @@ -0,0 +1,72 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import { createRoutesStub, useLoaderData, useNavigation } from 'react-router' +import { expect, test } from 'vitest' +import { render } from 'vitest-browser-react' + +import { Tabs } from '~/ui/lib/Tabs' + +import { QueryParamTabs } from './QueryParamTabs' + +test('waits for the loader before showing the new tab', async () => { + let finishLoad = () => {} + // tab each render of the second panel saw in loader data + const panelTwoRenders: string[] = [] + + function PanelTwo() { + const { tab } = useLoaderData<{ tab: string }>() + panelTwoRenders.push(tab) + return

Panel two

+ } + + function Page() { + // like the loading bar in RootLayout, which rerenders while the nav is pending + const navigation = useNavigation() + return ( + + + One + Two + + +

Panel one

+
+ + + +

Navigation {navigation.state}

+
+ ) + } + + const Stub = createRoutesStub([ + { + path: '/', + Component: Page, + // like a page loader that only prefetches the current tab's data + loader: async ({ request }) => { + const tab = new URL(request.url).searchParams.get('tab') ?? 'one' + if (tab === 'two') await new Promise((resolve) => (finishLoad = resolve)) + return { tab } + }, + }, + ]) + + const screen = await render() + await expect.element(screen.getByText('Panel one')).toBeVisible() + + await screen.getByRole('tab', { name: 'Two' }).click() + await expect.element(screen.getByText('Navigation loading')).toBeInTheDocument() + // still on the old tab while the new tab's loader runs + await expect.element(screen.getByText('Panel one')).toBeVisible() + expect(panelTwoRenders).toEqual([]) + + finishLoad() + await expect.element(screen.getByText('Panel two')).toBeVisible() + expect(panelTwoRenders).not.toContain('one') +}) diff --git a/app/components/QueryParamTabs.tsx b/app/components/QueryParamTabs.tsx index ba273d7ca1..593ba294ea 100644 --- a/app/components/QueryParamTabs.tsx +++ b/app/components/QueryParamTabs.tsx @@ -20,12 +20,16 @@ export function QueryParamTabs(props: TabsRootProps) { const value = searchParams.get('tab') || props.defaultValue function onValueChange(newValue: string | number | null) { + // Copy rather than mutate: the hook's object is what we read `value` from, + // so mutating it switches the tab on the next render, before the navigation + // (and the loader it triggers) has committed. + const nextParams = new URLSearchParams(searchParams) if (newValue === props.defaultValue) { - searchParams.delete('tab') + nextParams.delete('tab') } else { - searchParams.set('tab', String(newValue)) + nextParams.set('tab', String(newValue)) } - setSearchParams(searchParams, { replace: true }) + setSearchParams(nextParams, { replace: true }) } return