From 8b36acdf48715c8180ad787abc793d709ade914d Mon Sep 17 00:00:00 2001 From: Carla Severe Date: Mon, 10 Aug 2026 10:52:31 -0700 Subject: [PATCH 1/3] Make shared results-table URLs reproduce the same view regardless of cookies --- .../CompareResults/ResultsTable.test.tsx | 62 ++++++++++++++++ .../SubtestsResultsView.test.tsx.snap | 16 ++-- .../CompareResults/ResultsTable.tsx | 5 ++ .../SubtestsResults/SubtestsResultsTable.tsx | 6 ++ .../useInitializeTableStateFromCookies.ts | 74 +++++++++++++++++++ src/hooks/useTableFilters.ts | 32 +++++--- src/hooks/useTableSort.ts | 24 ++++-- src/utils/tableStatePersistence.ts | 39 ++++++++++ 8 files changed, 234 insertions(+), 24 deletions(-) create mode 100644 src/hooks/useInitializeTableStateFromCookies.ts create mode 100644 src/utils/tableStatePersistence.ts diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index bfd8e35bd..d1e11f805 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -1365,3 +1365,65 @@ describe('Advanced-columns toggle for mann-whitney-u testVersion', () => { expect(advancedParam()).toBeNull(); }); }); + +describe('cookie persistence vs. shareable URLs', () => { + it('seeds filters from cookies and marks the URL initialized on a fresh URL', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + // The remembered cookie is applied to the view... + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Linux 18.04, Regression, 1.85 %, Medium', + ]); + // ...and materialised into the URL, which is now marked initialized so the + // link reproduces this exact view for anyone. + expect(summarizeTableFiltersFromUrl()).toEqual({ status: ['regression'] }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + }); + + it('ignores cookies when the URL is already initialized', async () => { + // A different viewer's cookie must not change what an initialized (shared) + // URL displays. + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t&initialized=1'); + + await screen.findByText('a11yr'); + + // Cookie is ignored: every status stays visible. + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Linux 18.04, Regression, 1.85 %, Medium', + ' - macOS 10.15, Improvement, 1.08 %, Low', + ' - Windows 10, -, -24 %, -', + ' - Windows 10, -, -2.4 %, High', + ]); + // ...and the cookie is not written into the URL. + expect(summarizeTableFiltersFromUrl()).toEqual({}); + }); + + it('keeps the initialized marker after toggling a filter', async () => { + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + const user = userEvent.setup({ + advanceTimers: jest.advanceTimersByTime, + }); + await clickMenuItem(user, 'Status', /No changes/); + + expect(summarizeTableFiltersFromUrl()).toEqual({ + status: ['improvement', 'regression'], + }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + }); +}); diff --git a/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap b/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap index 7ba930a5c..302421820 100644 --- a/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap +++ b/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap @@ -55,7 +55,7 @@ exports[`SubtestsResultsView Component Tests for mann-whitney-u testVersion tabl style="width: 2.8284271247461903px; height: 2.8284271247461903px; top: -1.4142135623730951px; left: -1.4142135623730951px;" > { + const [, updateRawSearchParams] = useRawSearchParams(); + + useEffect(() => { + if (isTableStateInitialized(window.location.search)) { + return; + } + + const params = currentUrlParams(); + + // Only seed a value from a cookie when the URL doesn't already specify it, + // so an explicit URL param always wins over the cookie. + for (const column of columnsConfiguration) { + if (!('filter' in column)) { + continue; + } + const param = filterParam(column.key); + if (!params.has(param)) { + const cookieValue = getCookie(filterCookie(column.key)); + if (cookieValue) { + params.set(param, cookieValue); + } + } + } + + if (!params.has(SORT_PARAM)) { + const sortCookie = getCookie(SORT_COOKIE); + if (sortCookie) { + params.set(SORT_PARAM, sortCookie); + } + } + + // Stamp the marker even when there were no cookies, so a filter-free view + // is still "initialized" and can't pick up cookies on this or another + // browser later. + params.set(INITIALIZED_PARAM, '1'); + updateRawSearchParams(params); + // Mount-only: the URL is materialised once and the data hooks have already + // seeded their state from the same cookies during the first render. + }, []); +}; + +export default useInitializeTableStateFromCookies; diff --git a/src/hooks/useTableFilters.ts b/src/hooks/useTableFilters.ts index eb105ccf8..07c3c18e5 100644 --- a/src/hooks/useTableFilters.ts +++ b/src/hooks/useTableFilters.ts @@ -12,6 +12,12 @@ import type { CompareMannWhitneyResultsTableColumn, } from '../types/types'; import { getCookie, setCookie, deleteCookie } from '../utils/cookies'; +import { + filterParam, + filterCookie, + isTableStateInitialized, + currentUrlParams, +} from '../utils/tableStatePersistence'; // This hook handles the state that handles table filtering, and also takes care // of handling the URL parameters that mirror this state. @@ -65,6 +71,10 @@ const useTableFilters = ( // This function collects the table filters from the search params. It will // only be called once at mount time. + // Cookies are only consulted for an uninitialized URL; an initialized URL is + // the single source of truth so a shared link reproduces the same view. + const initialized = isTableStateInitialized(window.location.search); + const getInitialTableFilters = () => { const result: Map> = new Map(); for (const columnConfiguration of columnsConfiguration) { @@ -75,8 +85,8 @@ const useTableFilters = ( const { key: columnKey, possibleValues } = columnConfiguration; const paramValue = - rawSearchParams.get('filter_' + columnKey) ?? - getCookie('perfcompare_filter_' + columnKey); + rawSearchParams.get(filterParam(columnKey)) ?? + (initialized ? null : getCookie(filterCookie(columnKey))); if (paramValue) { const configuredValuesSet = new Set( paramValue.split(',').map((item) => item.trim()), @@ -101,9 +111,10 @@ const useTableFilters = ( const [tableFilters, setTableFilters] = useState(getInitialTableFilters); const onClearFilter = (columnId: string) => { - rawSearchParams.delete(`filter_${columnId}`); - updateRawSearchParams(rawSearchParams); - deleteCookie(`perfcompare_filter_${columnId}`); + const params = currentUrlParams(); + params.delete(filterParam(columnId)); + updateRawSearchParams(params); + deleteCookie(filterCookie(columnId)); setTableFilters((oldFilters) => { const newFilters = new Map(oldFilters); @@ -123,14 +134,15 @@ const useTableFilters = ( return; } + const params = currentUrlParams(); if (filters.size < columnConfiguration.possibleValues.length) { - rawSearchParams.set(`filter_${columnId}`, [...filters].join(',')); - setCookie(`perfcompare_filter_${columnId}`, [...filters].join(',')); + params.set(filterParam(columnId), [...filters].join(',')); + setCookie(filterCookie(columnId), [...filters].join(',')); } else { - rawSearchParams.delete(`filter_${columnId}`); - deleteCookie(`perfcompare_filter_${columnId}`); + params.delete(filterParam(columnId)); + deleteCookie(filterCookie(columnId)); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); setTableFilters((oldFilters) => { const newFilters = new Map(oldFilters); diff --git a/src/hooks/useTableSort.ts b/src/hooks/useTableSort.ts index 7c8b21362..060515ec0 100644 --- a/src/hooks/useTableSort.ts +++ b/src/hooks/useTableSort.ts @@ -8,6 +8,12 @@ import type { SortFunc, } from '../types/types'; import { getCookie, setCookie, deleteCookie } from '../utils/cookies'; +import { + SORT_PARAM, + SORT_COOKIE, + isTableStateInitialized, + currentUrlParams, +} from '../utils/tableStatePersistence'; // This hook handles the state that handles table sorting, and also takes care // of handling the URL parameters that mirror this state. @@ -25,8 +31,13 @@ const useTableSort = ( // This is our custom hook that updates the search params without a rerender. const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); + // Cookies are only consulted for an uninitialized URL; an initialized URL is + // the single source of truth so a shared link reproduces the same view. + const initialized = isTableStateInitialized(window.location.search); const sortFromUrl = - rawSearchParams.get('sort') ?? getCookie('perfcompare_sort') ?? ''; + rawSearchParams.get(SORT_PARAM) ?? + (initialized ? null : getCookie(SORT_COOKIE)) ?? + ''; const [columnId, direction] = useMemo(() => { const [columnId, direction] = sortFromUrl.split('|'); if (!columnId) { @@ -57,18 +68,19 @@ const useTableSort = ( columnId: string, newSortDirection: 'asc' | 'desc' | null, ) => { + const params = currentUrlParams(); if (newSortDirection === null) { setSortColumn(null); setSortDirection(null); - rawSearchParams.delete('sort'); - deleteCookie('perfcompare_sort'); + params.delete(SORT_PARAM); + deleteCookie(SORT_COOKIE); } else { setSortColumn(columnId); setSortDirection(newSortDirection); - rawSearchParams.set('sort', columnId + '|' + newSortDirection); - setCookie('perfcompare_sort', columnId + '|' + newSortDirection); + params.set(SORT_PARAM, columnId + '|' + newSortDirection); + setCookie(SORT_COOKIE, columnId + '|' + newSortDirection); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); }; return { sortDirection, sortColumn, onToggleSort }; diff --git a/src/utils/tableStatePersistence.ts b/src/utils/tableStatePersistence.ts new file mode 100644 index 000000000..f577bd5e5 --- /dev/null +++ b/src/utils/tableStatePersistence.ts @@ -0,0 +1,39 @@ +// Central definitions for how the results table's filter/sort state is +// persisted, so the query-param and cookie keys live in exactly one place. +// +// The state lives in two layers: +// * the URL (`filter_`, `sort`) — the shareable source of truth; +// * cookies (`perfcompare_filter_`, `perfcompare_sort`) — a per-browser +// memory of the last-used values. +// +// A URL is considered "initialized" once the app has materialised the table +// state into it (marker below). Cookies are only ever *read* for an +// *uninitialized* URL; an initialized URL is the single source of truth, so a +// shared link reproduces the same view for everyone regardless of their +// cookies. Cookies are still *written* on every change, so the memory survives +// for the next fresh visit. + +// Marker that flags a URL as "initialized" (see above). +export const INITIALIZED_PARAM = 'initialized'; + +// URL query-parameter keys. +export const SORT_PARAM = 'sort'; +export const filterParam = (columnKey: string) => `filter_${columnKey}`; + +// Cookie keys. +export const SORT_COOKIE = 'perfcompare_sort'; +export const filterCookie = (columnKey: string) => + `perfcompare_filter_${columnKey}`; + +// Whether the URL already carries the initialized marker. +export function isTableStateInitialized(search: string): boolean { + return new URLSearchParams(search).has(INITIALIZED_PARAM); +} + +// Read the *live* URL params. Writes must start from this rather than a +// memoized snapshot, otherwise params added out-of-band (e.g. the initialized +// marker, seeded once on mount) would be clobbered by a later filter/sort +// change. +export function currentUrlParams(): URLSearchParams { + return new URLSearchParams(window.location.search); +} From a74d4114ffb20fae7e70f1d0ce6e28b238eaf6f9 Mon Sep 17 00:00:00 2001 From: Carla Severe Date: Wed, 19 Aug 2026 21:17:05 -0700 Subject: [PATCH 2/3] Preserve initialized marker and seeded params on all URL writes --- .../CompareResults/ResultsTable.test.tsx | 53 +++++++++++++++++++ .../CompareResults/AdvancedColumnsMenu.tsx | 7 ++- src/components/CompareResults/ResultsMain.tsx | 8 ++- .../CompareResults/ResultsTable.tsx | 26 +++++---- 4 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index d1e11f805..fe9b766c1 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -1426,4 +1426,57 @@ describe('cookie persistence vs. shareable URLs', () => { '1', ); }); + + it('keeps the initialized marker and seeded filters after a search-term change', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + // Seeded from the cookie and marked initialized. + expect(summarizeTableFiltersFromUrl()).toEqual({ status: ['regression'] }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + // Submit with Enter so the write happens immediately (bypasses the input's + // debounce, which fake timers don't flush after typing). + await user.type(screen.getByPlaceholderText('Filter results'), 'linux{Enter}'); + + // The search term is written, and the out-of-band params survive. + const params = new URLSearchParams(window.location.search); + expect(params.get('search')).toBe('linux'); + expect(params.get('initialized')).toBe('1'); + expect(params.get('filter_status')).toBe('regression'); + }); + + it('keeps the initialized marker and seeded filters after a test-version change', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + await user.click( + screen.getByRole('combobox', { name: 'Stats Test Version' }), + ); + await user.click( + await screen.findByRole('option', { name: 'Mann-Whitney-U' }), + ); + + // The test version changes (a router navigation), and the marker + seeded + // filter ride along instead of being dropped. + await waitFor(() => { + const params = new URLSearchParams(window.location.search); + expect(params.get('test_version')).toBe('mann-whitney-u'); + expect(params.get('initialized')).toBe('1'); + expect(params.get('filter_status')).toBe('regression'); + }); + }); }); diff --git a/src/components/CompareResults/AdvancedColumnsMenu.tsx b/src/components/CompareResults/AdvancedColumnsMenu.tsx index 1f6515332..44b91bc2f 100644 --- a/src/components/CompareResults/AdvancedColumnsMenu.tsx +++ b/src/components/CompareResults/AdvancedColumnsMenu.tsx @@ -23,6 +23,7 @@ import { SIGNIFICANCE, serializeAdvancedColumns, } from '../../utils/advancedColumnsUrl'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; // The advanced statistics columns are toggled independently — any combination // can be shown. The option values reuse the URL keys so the dropdown, the URL @@ -38,7 +39,7 @@ function AdvancedColumnsMenu() { const dispatch = useAppDispatch(); const mode = useAppSelector((state) => state.theme.mode); const advancedColumns = useAdvancedColumns(); - const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); + const [, updateRawSearchParams] = useRawSearchParams(); // Track the Select's open state so the tooltip can be suppressed while the // dropdown is open — otherwise it renders over (and hides) the checkboxes. const [menuOpen, setMenuOpen] = useState(false); @@ -54,7 +55,9 @@ function AdvancedColumnsMenu() { dispatch(updateShowCles(next.cles)); dispatch(updateShowSignificance(next.significance)); - const params = new URLSearchParams(rawSearchParams); + // Build from the live URL so toggling columns doesn't drop params written + // out-of-band (e.g. the `initialized` marker and cookie-seeded filter/sort). + const params = currentUrlParams(); const value = serializeAdvancedColumns(next); if (value) { params.set(ADVANCED_COLUMNS_PARAM, value); diff --git a/src/components/CompareResults/ResultsMain.tsx b/src/components/CompareResults/ResultsMain.tsx index 95a179b63..b58d584fc 100644 --- a/src/components/CompareResults/ResultsMain.tsx +++ b/src/components/CompareResults/ResultsMain.tsx @@ -23,6 +23,7 @@ import { Colors, FontsRaw, FontSizeRaw, Spacing } from '../../styles'; import pencilDark from '../../theme/img/pencil-dark.svg'; import pencil from '../../theme/img/pencil.svg'; import type { TestVersion } from '../../types/types'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; import EditTitleInput from '../CompareResults/EditTitleInput'; import ToggleReplicatesButton from '../Shared/ToggleReplicatesButton'; @@ -116,8 +117,11 @@ function ResultsMain() { }; const onSaveButtonClick = () => { - rawSearchParams.set('title', comparisonTitleName); - updateRawSearchParams(rawSearchParams); + // Build from the live URL so we don't drop params written out-of-band + // (e.g. the `initialized` marker and cookie-seeded filter/sort). + const params = currentUrlParams(); + params.set('title', comparisonTitleName); + updateRawSearchParams(params); showEditComparisonTitleInput(false); }; diff --git a/src/components/CompareResults/ResultsTable.tsx b/src/components/CompareResults/ResultsTable.tsx index 5f9b00ed5..bacc612c4 100644 --- a/src/components/CompareResults/ResultsTable.tsx +++ b/src/components/CompareResults/ResultsTable.tsx @@ -21,6 +21,7 @@ import { getColumnsConfiguration, toGridTemplateColumns, } from '../../utils/rowTemplateColumns'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; type CombinedLoaderReturnValue = LoaderReturnValue | OverTimeLoaderReturnValue; export default function ResultsTable() { @@ -33,7 +34,7 @@ export default function ResultsTable() { testVersion, } = useLoaderData(); - const [searchParams, setSearchParams] = useSearchParams(); + const [, setSearchParams] = useSearchParams(); // This is our custom hook that updates the search params without a rerender. const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); @@ -70,29 +71,36 @@ export default function ResultsTable() { ); const [expandAll, setExpandAll] = useState(false); + // These writers build from the *live* URL (currentUrlParams) rather than a + // render-time snapshot, so they preserve params written out-of-band — most + // importantly the `initialized` marker and cookie-seeded filter/sort — that a + // stale snapshot would drop (see useRawSearchParams / tableStatePersistence). const onFrameworkChange = (newFrameworkId: Framework['id']) => { setFrameworkIdVal(newFrameworkId); - searchParams.set('framework', newFrameworkId.toString()); - setSearchParams(searchParams); + const params = currentUrlParams(); + params.set('framework', newFrameworkId.toString()); + setSearchParams(params); }; const onSearchTermChange = (newSearchTerm: string) => { setSearchTerm(newSearchTerm); + const params = currentUrlParams(); if (newSearchTerm) { - rawSearchParams.set('search', newSearchTerm); + params.set('search', newSearchTerm); } else { - rawSearchParams.delete('search'); + params.delete('search'); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); }; const onTestVersionChange = (testVersion: TestVersion): void => { setTestVersionVal(testVersion); - searchParams.set('test_version', testVersion); + const params = currentUrlParams(); + params.set('test_version', testVersion); if (testVersion !== MANN_WHITNEY_U) { - searchParams.delete('replicates'); + params.delete('replicates'); } - setSearchParams(searchParams); + setSearchParams(params); }; const rowGridTemplateColumns = toGridTemplateColumns(columnsConfig); From edaf9be35178aa4b739668aa9231ccde9104845a Mon Sep 17 00:00:00 2001 From: Carla Severe Date: Wed, 19 Aug 2026 21:24:04 -0700 Subject: [PATCH 3/3] lint fix --- src/__tests__/CompareResults/ResultsTable.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index fe9b766c1..18605c806 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -1443,7 +1443,10 @@ describe('cookie persistence vs. shareable URLs', () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); // Submit with Enter so the write happens immediately (bypasses the input's // debounce, which fake timers don't flush after typing). - await user.type(screen.getByPlaceholderText('Filter results'), 'linux{Enter}'); + await user.type( + screen.getByPlaceholderText('Filter results'), + 'linux{Enter}', + ); // The search term is written, and the out-of-band params survive. const params = new URLSearchParams(window.location.search);