From cbd23e96541d69dacf4a1225c52b7cc1275142a6 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Wed, 23 Oct 2024 14:38:29 +0200 Subject: [PATCH 1/2] Support a 'No value' entry for the confidence filtering options --- .../CompareResults/ResultsTable.test.tsx | 32 ++++++++++++------- .../OverTimeResultsView.test.tsx.snap | 4 ++- .../__snapshots__/ResultsTable.test.tsx.snap | 4 ++- .../__snapshots__/ResultsView.test.tsx.snap | 4 ++- .../CompareResults/ResultsTable.tsx | 12 +++++-- src/components/CompareResults/RevisionRow.tsx | 2 +- src/types/state.ts | 2 +- src/types/types.ts | 2 +- 8 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index 4b919ce9b..ecf506450 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -123,7 +123,7 @@ describe('Results Table', () => { ' - OSX, Improvement, Low', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ' - Android, Improvement, Low', ]); @@ -156,7 +156,7 @@ describe('Results Table', () => { ' - OSX, Improvement, Low', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ' - Android, Improvement, Low', ]); @@ -165,7 +165,7 @@ describe('Results Table', () => { 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ' - Android, Improvement, Low', ]); @@ -174,7 +174,7 @@ describe('Results Table', () => { 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); }); @@ -188,7 +188,7 @@ describe('Results Table', () => { ' - OSX, Improvement, Low', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); const user = userEvent.setup({ delay: null }); @@ -205,13 +205,13 @@ describe('Results Table', () => { 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); await clickMenuItem(user, /Status/, /Regression/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); }); @@ -225,7 +225,7 @@ describe('Results Table', () => { ' - OSX, Improvement, Low', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); const user = userEvent.setup({ delay: null }); @@ -234,20 +234,20 @@ describe('Results Table', () => { 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', ]); await clickMenuItem(user, /Confidence/, /High/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Linux, Regression, Medium', - ' - Windows, -, ', + ' - Windows, -, -', ]); await clickMenuItem(user, /Confidence/, /Medium/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', - ' - Windows, -, ', + ' - Windows, -, -', ]); await clickMenuItem(user, /Confidence/, /Clear filters/); @@ -256,7 +256,15 @@ describe('Results Table', () => { ' - OSX, Improvement, Low', ' - Linux, Regression, Medium', ' - Windows, -, High', - ' - Windows, -, ', + ' - Windows, -, -', + ]); + + await clickMenuItem(user, /Confidence/, /No value/); + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - OSX, Improvement, Low', + ' - Linux, Regression, Medium', + ' - Windows, -, High', ]); }); }); diff --git a/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap b/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap index 29544e8a0..72482a2ce 100644 --- a/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap +++ b/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap @@ -1200,7 +1200,9 @@ exports[`Results View The table should match snapshot and other elements should
+ > + - +
+ > + - +
+ > + - +
- result.confidence_text === value, + possibleValues: ['No value', 'Low', 'Medium', 'High'], + matchesFunction: (result: CompareResultsItem, value: string) => { + switch (value) { + case 'No value': + return !result.confidence_text; + default: + return result.confidence_text === value; + } + }, }, { name: 'Total Runs', diff --git a/src/components/CompareResults/RevisionRow.tsx b/src/components/CompareResults/RevisionRow.tsx index e6e9b4f82..4bac9c167 100644 --- a/src/components/CompareResults/RevisionRow.tsx +++ b/src/components/CompareResults/RevisionRow.tsx @@ -396,7 +396,7 @@ function RevisionRow(props: RevisionRowProps) {
{confidenceText && confidenceIcons[confidenceText]} - {confidenceText} + {confidenceText || '-'}
diff --git a/src/types/state.ts b/src/types/state.ts index 9cb122893..aa1b3e73a 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -108,7 +108,7 @@ export type CompareResultsItem = { is_regression: boolean; is_meaningful: boolean; more_runs_are_needed: boolean; - /* + /* Each test has a signature and each signature may or may not have a parent_signature. If a signature has a parent_signature then we are looking at a subtest. For regular tests this field will be null. */ diff --git a/src/types/types.ts b/src/types/types.ts index f9d1b61c2..3a2d6770a 100644 --- a/src/types/types.ts +++ b/src/types/types.ts @@ -19,7 +19,7 @@ export type CompareResultsTableConfig = matchesFunction: (result: CompareResultsItem, value: string) => boolean; }; -export type ConfidenceText = 'High' | 'Medium' | 'Low'; +export type ConfidenceText = 'High' | 'Medium' | 'Low' | ''; export type MeasurementUnit = | 'W' From 1a1c4b1c488d28445dcf9a727b7c6c9613c8e53b Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Wed, 23 Oct 2024 14:47:44 +0200 Subject: [PATCH 2/2] Add more options to the filtering menus, to make it easier to select the various options --- .../CompareResults/ResultsTable.test.tsx | 25 ++++++++++++++++--- src/components/CompareResults/TableHeader.tsx | 21 +++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index ecf506450..3ad7d834e 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -150,7 +150,7 @@ describe('Results Table', () => { ' - Android, Improvement, Low', ]); - await clickMenuItem(user, /Platform/, 'Clear filters'); + await clickMenuItem(user, /Platform/, 'Select all values'); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - OSX, Improvement, Low', @@ -176,6 +176,12 @@ describe('Results Table', () => { ' - Windows, -, High', ' - Windows, -, -', ]); + + await clickMenuItem(user, /Platform/, /Select only.*Android/); + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Android, Improvement, Low', + ]); }); it('should filter on the Status column', async () => { @@ -199,7 +205,7 @@ describe('Results Table', () => { ' - Linux, Regression, Medium', ]); - await clickMenuItem(user, /Status/, /Clear filters/); + await clickMenuItem(user, /Status/, /Select all values/); await clickMenuItem(user, /Status/, /Improvement/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', @@ -207,12 +213,19 @@ describe('Results Table', () => { ' - Windows, -, High', ' - Windows, -, -', ]); + await clickMenuItem(user, /Status/, /Regression/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - Windows, -, High', ' - Windows, -, -', ]); + + await clickMenuItem(user, /Status/, /Select only.*Regression/); + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Linux, Regression, Medium', + ]); }); it('should filter on the Confidence column', async () => { @@ -250,7 +263,7 @@ describe('Results Table', () => { ' - Windows, -, -', ]); - await clickMenuItem(user, /Confidence/, /Clear filters/); + await clickMenuItem(user, /Confidence/, /Select all values/); expect(summarizeVisibleRows()).toEqual([ 'a11yr dhtml.html spam opt e10s fission stylo webrender', ' - OSX, Improvement, Low', @@ -266,5 +279,11 @@ describe('Results Table', () => { ' - Linux, Regression, Medium', ' - Windows, -, High', ]); + + await clickMenuItem(user, /Confidence/, /Select only.*High/); + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Windows, -, High', + ]); }); }); diff --git a/src/components/CompareResults/TableHeader.tsx b/src/components/CompareResults/TableHeader.tsx index 2188dffa3..8ad0fb72d 100644 --- a/src/components/CompareResults/TableHeader.tsx +++ b/src/components/CompareResults/TableHeader.tsx @@ -1,6 +1,7 @@ import CheckIcon from '@mui/icons-material/Check'; import FilterListIcon from '@mui/icons-material/FilterList'; import Button from '@mui/material/Button'; +import Divider from '@mui/material/Divider'; import Menu from '@mui/material/Menu'; import MenuItem from '@mui/material/MenuItem'; import { @@ -43,6 +44,13 @@ function FilterableColumn({ onToggle(newUncheckedValues); }; + const onClickOnlyFilter = (value: string) => { + const newUncheckedValues = new Set( + possibleValues.filter((possibleValue) => possibleValue !== value), + ); + onToggle(newUncheckedValues); + }; + const hasFilteredValues = uncheckedValues && uncheckedValues.size; const buttonAriaLabel = hasFilteredValues ? `${name} (Click to filter values. Some filters are active.)` @@ -75,8 +83,19 @@ function FilterableColumn({ - Clear filters + Select all values + + {possibleValues.map((possibleValue) => ( + onClickOnlyFilter(possibleValue)} + > + Select only “{possibleValue}” + + ))} + {possibleValues.map((possibleValue) => { const isChecked = !uncheckedValues || !uncheckedValues.has(possibleValue);