Skip to content

Commit 033ca27

Browse files
authored
replaced setSearchParams with updateRawSearchParams for framework change (#983)
This PR is a fix for [Bug 1999795](https://bugzilla.mozilla.org/show_bug.cgi?id=1999795). The reporter noticed that after clearing a search term, changing the framework creates an url with the old search term intact. This is not the expected behavior. This PR replaces `setSearchParams` with `updateRawSearchParams` for onFrameworkChange and onTestVersionChange, using the custom hook for updating the search params. To test with production version bug: [Production Link](https://perf.compare/compare-results?baseRev=e5e0964a228d62e9f77b29e93e8b0d8e6ef15a5a&baseRepo=try&newRev=89ebb0972d42440367c8420ed0fbd10721177c2b&newRepo=try&framework=13&search=speedo) 1. Clear the search term speedo 2. It should be clear in the url. 3. Change the framework from browsertime to talos 4. The url still has the search=speedo ### To test with deployed version: [DeployLink](https://deploy-preview-983--mozilla-perfcompare.netlify.app/compare-results?baseRev=e5e0964a228d62e9f77b29e93e8b0d8e6ef15a5a&baseRepo=try&newRev=89ebb0972d42440367c8420ed0fbd10721177c2b&newRepo=try&framework=13&test_version=mann-whitney-u&search=speedo) 1. Clear the search term speedo 2. It should be clear in the url. 3. Change the framework from browsertime to talos 4. The url should not have the search term speedo
2 parents bf4db14 + 1766c47 commit 033ca27

File tree

2 files changed

+25
-6
lines changed

2 files changed

+25
-6
lines changed

src/__tests__/CompareResults/ResultsTable.test.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,4 +1171,24 @@ describe('Results Table for MannWhitneyResultsItem for mann-whitney-u testVersio
11711171
// It should be persisted in the URL
11721172
expectParameterToHaveValue('sort', 'effects|asc');
11731173
});
1174+
1175+
it('should switch between Student-T and Mann-Whitney-U test versions', async () => {
1176+
const { testCompareData } = getTestData();
1177+
setupAndRender(testCompareData, 'test_version=student-t');
1178+
await screen.findByText('a11yr');
1179+
expectParameterToHaveValue('test_version', 'student-t');
1180+
const testVersionDropdown = screen.getByRole('combobox', {
1181+
name: 'Stats Test Version',
1182+
});
1183+
expect(testVersionDropdown).toBeInTheDocument();
1184+
const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
1185+
await user.click(testVersionDropdown);
1186+
const mannWhitneyOption = await screen.findByRole('option', {
1187+
name: 'Mann-Whitney-U',
1188+
});
1189+
await user.click(mannWhitneyOption);
1190+
1191+
// The URL should be updated with the new test version
1192+
expectParameterToHaveValue('test_version', 'mann-whitney-u');
1193+
});
11741194
});

src/components/CompareResults/ResultsTable.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Suspense, useState } from 'react';
22

33
import Box from '@mui/material/Box';
44
import CircularProgress from '@mui/material/CircularProgress';
5-
import { useSearchParams, useLoaderData, Await } from 'react-router';
5+
import { useLoaderData, Await } from 'react-router';
66

77
import type { LoaderReturnValue } from './loader';
88
import type { LoaderReturnValue as OverTimeLoaderReturnValue } from './overTimeLoader';
@@ -26,7 +26,6 @@ export default function ResultsTable() {
2626
replicates,
2727
testVersion,
2828
} = useLoaderData<CombinedLoaderReturnValue>();
29-
const [searchParams, setSearchParams] = useSearchParams();
3029

3130
// This is our custom hook that updates the search params without a rerender.
3231
const [rawSearchParams, updateRawSearchParams] = useRawSearchParams();
@@ -52,8 +51,8 @@ export default function ResultsTable() {
5251

5352
const onFrameworkChange = (newFrameworkId: Framework['id']) => {
5453
setFrameworkIdVal(newFrameworkId);
55-
searchParams.set('framework', newFrameworkId.toString());
56-
setSearchParams(searchParams);
54+
rawSearchParams.set('framework', newFrameworkId.toString());
55+
updateRawSearchParams(rawSearchParams);
5756
};
5857

5958
const onSearchTermChange = (newSearchTerm: string) => {
@@ -68,8 +67,8 @@ export default function ResultsTable() {
6867

6968
const onTestVersionChange = (testVersion: TestVersion): void => {
7069
setTestVersionVal(testVersion);
71-
searchParams.set('test_version', testVersion);
72-
setSearchParams(searchParams);
70+
rawSearchParams.set('test_version', testVersion);
71+
updateRawSearchParams(rawSearchParams);
7372
};
7473

7574
const rowGridTemplateColumns = columnsConfig

0 commit comments

Comments
 (0)