diff --git a/.gitignore b/.gitignore index d7b8f1bd7..56b53c457 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ yarn-debug.log* yarn-error.log* .eslintcache +CLAUDE.md diff --git a/src/__tests__/CompareResults/FilteredRowsNotice.test.tsx b/src/__tests__/CompareResults/FilteredRowsNotice.test.tsx new file mode 100644 index 000000000..9d9e2308f --- /dev/null +++ b/src/__tests__/CompareResults/FilteredRowsNotice.test.tsx @@ -0,0 +1,43 @@ +import FilteredRowsNotice from '../../components/CompareResults/FilteredRowsNotice'; +import type { ActiveColumnFilter } from '../../hooks/useTableFilters'; +import { render, screen } from '../utils/test-utils'; + +describe('FilteredRowsNotice', () => { + const filters: ActiveColumnFilter[] = [ + { name: 'Significance', excludedLabels: ['Noise'] }, + ]; + + it('renders nothing when no rows are hidden', () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + expect( + screen.queryByTestId('filtered-rows-notice'), + ).not.toBeInTheDocument(); + }); + + it('reports the count (singular) and the active filter reasons', () => { + render(); + const notice = screen.getByTestId('filtered-rows-notice'); + expect(notice).toHaveTextContent('1 row hidden by filters'); + expect(notice).toHaveTextContent('Significance: Noise'); + }); + + it('pluralizes the count and joins multiple filters and values', () => { + render( + , + ); + const notice = screen.getByTestId('filtered-rows-notice'); + expect(notice).toHaveTextContent('3 rows hidden by filters'); + expect(notice).toHaveTextContent( + 'Significance: Noise • Status: No changes, Improvement', + ); + }); +}); diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index 18605c806..96ce88ade 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -1308,7 +1308,7 @@ describe('Advanced-columns toggle for mann-whitney-u testVersion', () => { // Open the dropdown and enable Cliff's Delta only. await user.click( - screen.getByRole('combobox', { name: 'Advanced columns' }), + screen.getByRole('combobox', { name: 'Advanced options' }), ); await user.click(screen.getByRole('option', { name: "Cliff's Delta" })); expect(header().querySelector('.delta-header')).toBeTruthy(); @@ -1349,7 +1349,7 @@ describe('Advanced-columns toggle for mann-whitney-u testVersion', () => { expect(advancedParam()).toBeNull(); await user.click( - screen.getByRole('combobox', { name: 'Advanced columns' }), + screen.getByRole('combobox', { name: 'Advanced options' }), ); await user.click(screen.getByRole('option', { name: "Cliff's Delta" })); expect(advancedParam()).toBe('cliffs_delta'); @@ -1364,6 +1364,76 @@ describe('Advanced-columns toggle for mann-whitney-u testVersion', () => { await user.click(screen.getByRole('option', { name: 'CLES' })); expect(advancedParam()).toBeNull(); }); + + it('groups the dropdown into Columns and Expanded row sections', async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const { testCompareMannWhitneyData } = getTestData(); + setupAndRender(testCompareMannWhitneyData, 'test_version=mann-whitney-u'); + await screen.findByText('a11yr'); + + await user.click( + screen.getByRole('combobox', { name: 'Advanced options' }), + ); + + // Both group headers and one option from each group are present. + expect(screen.getByText('Columns')).toBeInTheDocument(); + expect(screen.getByText('Expanded row')).toBeInTheDocument(); + expect( + screen.getByRole('option', { name: "Cliff's Delta" }), + ).toBeInTheDocument(); + expect( + screen.getByRole('option', { name: 'Statistics table' }), + ).toBeInTheDocument(); + }); + + it('seeds the expanded-row selection from the advanced_expanded URL param', async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const { testCompareMannWhitneyData } = getTestData(); + setupAndRender( + testCompareMannWhitneyData, + 'test_version=mann-whitney-u&advanced_expanded=stats_table', + ); + await screen.findByText('a11yr'); + + await user.click( + screen.getByRole('combobox', { name: 'Advanced options' }), + ); + + // The seeded option is checked; an unseeded one is not. + expect( + screen.getByRole('option', { name: 'Statistics table' }), + ).toHaveAttribute('aria-selected', 'true'); + expect( + screen.getByRole('option', { name: 'Data warnings' }), + ).toHaveAttribute('aria-selected', 'false'); + }); + + it('persists the expanded-row selection to the advanced_expanded URL param', async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const { testCompareMannWhitneyData } = getTestData(); + setupAndRender(testCompareMannWhitneyData, 'test_version=mann-whitney-u'); + await screen.findByText('a11yr'); + + const expandedParam = () => + new URLSearchParams(window.location.search).get('advanced_expanded'); + expect(expandedParam()).toBeNull(); + + await user.click( + screen.getByRole('combobox', { name: 'Advanced options' }), + ); + await user.click(screen.getByRole('option', { name: 'Statistics table' })); + expect(expandedParam()).toBe('stats_table'); + + await user.click(screen.getByRole('option', { name: 'Data warnings' })); + expect(expandedParam()).toBe('stats_table,warnings'); + + // Turning the columns off leaves the expanded-row param untouched. + await user.click(screen.getByRole('option', { name: 'Statistics table' })); + expect(expandedParam()).toBe('warnings'); + + await user.click(screen.getByRole('option', { name: 'Data warnings' })); + expect(expandedParam()).toBeNull(); + }); }); describe('cookie persistence vs. shareable URLs', () => { @@ -1483,3 +1553,40 @@ describe('cookie persistence vs. shareable URLs', () => { }); }); }); + +// Placed at the end of the file on purpose: this test performs extra renders / +// menu interactions, and React's useId counter is global across renders in a +// jest run, so running it before the snapshot tests above would shift their +// generated ids. Keeping it last avoids perturbing those snapshots. +describe('Filtered-rows notice', () => { + it('shows how many rows the active filters hide, and why', async () => { + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + // No active filters → no notice. + expect( + screen.queryByTestId('filtered-rows-notice'), + ).not.toBeInTheDocument(); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + + // Hide the two "No changes" rows; the notice reports the count and reason. + await clickMenuItem(user, 'Status', /No changes/); + const notice = await screen.findByTestId('filtered-rows-notice'); + expect(notice).toHaveTextContent('2 rows hidden by filters'); + expect(notice).toHaveTextContent('Status: No changes'); + + // Narrowing to a single status hides more rows and the count updates. + await clickMenuItem(user, 'Status', /Select only.*Regression/); + expect(await screen.findByTestId('filtered-rows-notice')).toHaveTextContent( + '3 rows hidden by filters', + ); + + // Clearing the filter removes the notice again. + await clickMenuItem(user, 'Status', /Select all values/); + expect( + screen.queryByTestId('filtered-rows-notice'), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/__tests__/CompareResults/RevisionRow.test.tsx b/src/__tests__/CompareResults/RevisionRow.test.tsx index 6f542eab5..93ddd8e2a 100644 --- a/src/__tests__/CompareResults/RevisionRow.test.tsx +++ b/src/__tests__/CompareResults/RevisionRow.test.tsx @@ -15,6 +15,7 @@ import { screen, renderWithRouter, enableAdvancedColumns, + enableExpandedRowOptions, } from '../utils/test-utils'; jest.mock('../../hooks/useSubtestRegressionCount'); @@ -167,6 +168,10 @@ describe('Subtest count pills', () => { }); describe('Expanded row', () => { + // Several tests assert the (advanced) MWU expanded-row components, which are + // hidden by default; enable them. No-op for Student-T expanded rows. + beforeEach(() => enableExpandedRowOptions()); + it('should display "Show 39 more" and "Show less" for base runs when row is expanded', async () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const { testCompareDataWithReplicatesMultipleValues: rowData } = diff --git a/src/__tests__/CompareResults/RevisionRowExpandable.test.tsx b/src/__tests__/CompareResults/RevisionRowExpandable.test.tsx index 5c002d62a..c1fef150c 100644 --- a/src/__tests__/CompareResults/RevisionRowExpandable.test.tsx +++ b/src/__tests__/CompareResults/RevisionRowExpandable.test.tsx @@ -5,7 +5,11 @@ import fetchMock from '@fetch-mock/jest'; import { loader } from '../../components/CompareResults/loader'; import RevisionRowExpandable from '../../components/CompareResults/RevisionRowExpandable'; import getTestData from '../utils/fixtures'; -import { screen, renderWithRouter } from '../utils/test-utils'; +import { + screen, + renderWithRouter, + enableExpandedRowOptions, +} from '../utils/test-utils'; function renderWithRoute(component: ReactElement) { fetchMock @@ -62,6 +66,9 @@ describe('RevisionRowExpandable for student-t testVersion', () => { }); describe('RevisionRowExpandable for mann-whitney-u testVersion', () => { + // These assert the (advanced) expanded-row components, hidden by default. + beforeEach(() => enableExpandedRowOptions()); + it('should display warnings', async () => { const { mockMannWhitneyResultData } = getTestData(); @@ -147,3 +154,106 @@ describe('RevisionRowExpandable for mann-whitney-u testVersion', () => { expect(goodFit).toBeInTheDocument(); }); }); + +describe('RevisionRowExpandable simplified Mann-Whitney-U view', () => { + // Text unique to each advanced expanded-row component, used to assert whether + // that component is currently rendered. + const EFFECT_SIZE_TEXT = /Effect Size:/; + const STATS_TABLE_TEXT = /Normality Test/; + const WARNINGS_TEXT = /Shapiro-Wilk test cannot be run/; + const MODES_LABEL = 'Mode-by-mode breakdown'; + + function renderMwuRow() { + const { mockMannWhitneyResultData } = getTestData(); + renderWithRoute( + , + ); + } + + it('shows the graph blurb and hides every advanced component by default', async () => { + renderMwuRow(); + + // The how-to-read blurb is always present in the simplified view. + expect( + await screen.findByText(/how the Base and New results are distributed/i), + ).toBeInTheDocument(); + + // None of the advanced (checkbox-gated) components render by default. + expect(screen.queryByText(EFFECT_SIZE_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByText(STATS_TABLE_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByText(WARNINGS_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(MODES_LABEL)).not.toBeInTheDocument(); + }); + + it('reveals only the effect size component when that option is on', async () => { + enableExpandedRowOptions({ effectSize: true }); + renderMwuRow(); + + expect(await screen.findByText(EFFECT_SIZE_TEXT)).toBeInTheDocument(); + expect(screen.queryByText(STATS_TABLE_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByText(WARNINGS_TEXT)).not.toBeInTheDocument(); + }); + + it('reveals only the statistics table when that option is on', async () => { + enableExpandedRowOptions({ statsTable: true }); + renderMwuRow(); + + expect(await screen.findByText(STATS_TABLE_TEXT)).toBeInTheDocument(); + expect(screen.queryByText(EFFECT_SIZE_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByText(WARNINGS_TEXT)).not.toBeInTheDocument(); + }); + + it('reveals only the data warnings when that option is on', async () => { + enableExpandedRowOptions({ warnings: true }); + renderMwuRow(); + + // Base and New each contribute a Shapiro-Wilk warning. + expect((await screen.findAllByText(WARNINGS_TEXT)).length).toBeGreaterThan( + 0, + ); + expect(screen.queryByText(EFFECT_SIZE_TEXT)).not.toBeInTheDocument(); + expect(screen.queryByText(STATS_TABLE_TEXT)).not.toBeInTheDocument(); + }); + + it('hides the mode-analysis controls (valley-depth slider + Show modes) by default', async () => { + renderMwuRow(); + await screen.findByText(/how the Base and New results are distributed/i); + + expect( + screen.queryByRole('slider', { name: /valley depth threshold/i }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('checkbox', { name: /show modes/i }), + ).not.toBeInTheDocument(); + }); + + it('reveals the mode-analysis controls when Mode analysis is enabled', async () => { + enableExpandedRowOptions({ modes: true }); + renderMwuRow(); + + expect( + await screen.findByRole('slider', { name: /valley depth threshold/i }), + ).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: /show modes/i }), + ).toBeInTheDocument(); + }); + + it('shows a placeholder when Mode analysis is on but no breakdown is available', async () => { + // The mock is a sparse/unimodal comparison, so no mode-by-mode breakdown is + // produced; the cell should surface a placeholder rather than stay empty. + enableExpandedRowOptions({ modes: true }); + renderMwuRow(); + + expect( + await screen.findByText(/No mode analysis available/i), + ).toBeInTheDocument(); + expect( + screen.queryByLabelText('Mode-by-mode breakdown'), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/__tests__/CompareResults/SubtestsResultsView.test.tsx b/src/__tests__/CompareResults/SubtestsResultsView.test.tsx index 531ea6d6f..1607c4b5c 100644 --- a/src/__tests__/CompareResults/SubtestsResultsView.test.tsx +++ b/src/__tests__/CompareResults/SubtestsResultsView.test.tsx @@ -14,11 +14,18 @@ import { renderWithRouter, screen, enableAdvancedColumns, + enableExpandedRowOptions, } from '../utils/test-utils'; jest.mock('../../utils/location'); const mockedGetLocationOrigin = getLocationOrigin as jest.Mock; +// The Mann-Whitney-U expanded row hides its statistical components (stats +// table, warnings, effect size, modes) behind "Advanced options" by default. +// These tests assert on those components, so enable them everywhere here. This +// is a no-op for rows that aren't expanded and for the Student-T layout. +beforeEach(() => enableExpandedRowOptions()); + const setup = ({ element, route, diff --git a/src/__tests__/CompareResults/SubtestsRevisionRow.test.tsx b/src/__tests__/CompareResults/SubtestsRevisionRow.test.tsx index 1b328e56a..9cfef3274 100644 --- a/src/__tests__/CompareResults/SubtestsRevisionRow.test.tsx +++ b/src/__tests__/CompareResults/SubtestsRevisionRow.test.tsx @@ -11,6 +11,7 @@ import { screen, renderWithRouter, enableAdvancedColumns, + enableExpandedRowOptions, } from '../utils/test-utils'; function renderWithRoute(component: ReactElement) { @@ -124,6 +125,7 @@ describe('SubtestsRevisionRow Component', () => { }); it('renders subtests results with mann-whitney-u testVersion', async () => { + enableExpandedRowOptions(); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const { subtestsResult } = getTestData(); const mockGridTemplateColumns = '1fr 1fr 1fr 1fr 1fr 1fr 1fr 1fr'; @@ -145,6 +147,7 @@ describe('SubtestsRevisionRow Component', () => { }); it('renders subtests results defaulting to mann-whitney-u with no testVersion', async () => { + enableExpandedRowOptions(); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const { subtestsMannWhitneyResult } = getTestData(); const mockGridTemplateColumns = '1fr 1fr 1fr 1fr 1fr 1fr 1fr 1fr'; diff --git a/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap b/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap index 5a06c4746..d580ae35a 100644 --- a/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap +++ b/src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap @@ -373,23 +373,23 @@ exports[`Results View The table should match snapshot and other elements should class="MuiGrid-root MuiGrid-direction-xs-row MuiGrid-grid-xs-auto css-zh2j38-MuiGrid-root" >
@@ -5002,7 +5002,7 @@ exports[`Results Table for MannWhitneyResultsItem for mann-whitney-u testVersion class="revision-block fw0pvlu" >
-
+
+
+ Only one run (consider more runs for greater confidence). + +
+
+ + Base application + + : + firefox + +
+
+ + New application + + : + chrome + +
-

- Runs Density Distribution -

-
- - Valley depth threshold - - : - - - - - - - - - - 50 - % - - -
-
-
-
+ + Comparison result + + : + ( + lower + is better)
-
-
- Only one run (consider more runs for greater confidence). - -
-
- - Base application - - : - firefox - -
-
- - New application - - : - chrome - -
-
- - Comparison result - - : - - ( - lower - is better) -
-
- - - - - - - - - - - - - - - - - - - -
- Metric - - Value - - Interpretation -
- Cliff's Delta - - -
- Significance (p-value) - - -
- CLES - - -
-
- - - -
+ This graph shows how the Base and New results are distributed. Two curves that mostly overlap mean the builds performed about the same; curves that sit apart suggest a real difference — the further apart, the bigger the change.
-
-
-
+

+ Runs Density Distribution +

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- Metric - - Base - - New - - Interpretation -
- Mean - - N/A - - N/A - -
- Median - - N/A - - N/A - -
- Variance - - N/A - - N/A - -
- Standard Deviation - - N/A - - N/A - -
- Min - - N/A - - N/A - -
- Max - - N/A - - N/A - -
- Normality Test -
- Shapiro-Wilk - - N/A - - N/A - - N/A -
- N/A -
- Goodness of Fit Test -
- Kolmogorov-Smirnov Test - - - -
-
+ style="width: 100%; height: 364px;" + />
@@ -540,508 +90,58 @@ exports[`Results View Should display Base, New and Common graphs with tooltips 1 class="MuiDivider-root MuiDivider-fullWidth MuiDivider-flexItem css-lhgpb-MuiDivider-root" />
-
+
+
+ Only one run (consider more runs for greater confidence). + +
+
+ + Base application + + : + firefox + +
+
+ + New application + + : + chrome + +
-

- Runs Density Distribution -

-
- - Valley depth threshold - - : - - - - - - - - - - 50 - % - - -
-
-
-
+ + Comparison result + + : + ( + lower + is better)
-
-
- Only one run (consider more runs for greater confidence). - -
-
- - Base application - - : - firefox - -
-
- - New application - - : - chrome - -
-
- - Comparison result - - : - - ( - lower - is better) -
-
- - - - - - - - - - - - - - - - - - - -
- Metric - - Value - - Interpretation -
- Cliff's Delta - - -
- Significance (p-value) - - -
- CLES - - -
-
- - - -
+ This graph shows how the Base and New results are distributed. Two curves that mostly overlap mean the builds performed about the same; curves that sit apart suggest a real difference — the further apart, the bigger the change.
-
-
-
+

+ Runs Density Distribution +

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- Metric - - Base - - New - - Interpretation -
- Mean - - N/A - - N/A - -
- Median - - N/A - - N/A - -
- Variance - - N/A - - N/A - -
- Standard Deviation - - N/A - - N/A - -
- Min - - N/A - - N/A - -
- Max - - N/A - - N/A - -
- Normality Test -
- Shapiro-Wilk - - N/A - - N/A - - N/A -
- N/A -
- Goodness of Fit Test -
- Kolmogorov-Smirnov Test - - - -
-
+ style="width: 100%; height: 364px;" + />
@@ -1486,23 +586,23 @@ exports[`Results View The table should match snapshot and other elements should class="MuiGrid-root MuiGrid-direction-xs-row MuiGrid-grid-xs-auto css-zh2j38-MuiGrid-root" >
- - -
- ); + renderExpandedBottom() { + // The Mann-Whitney-U statistics table and data warnings are now rendered + // (and individually gated) by RevisionRowExpandable's two-column expanded + // grid, so there is nothing to render from the strategy's bottom slot. + return null; }, renderColumns( diff --git a/src/components/CompareResults/AdvancedColumnsMenu.tsx b/src/components/CompareResults/AdvancedColumnsMenu.tsx deleted file mode 100644 index 44b91bc2f..000000000 --- a/src/components/CompareResults/AdvancedColumnsMenu.tsx +++ /dev/null @@ -1,126 +0,0 @@ -import { useState } from 'react'; - -import Checkbox from '@mui/material/Checkbox'; -import FormControl from '@mui/material/FormControl'; -import ListItemText from '@mui/material/ListItemText'; -import MenuItem from '@mui/material/MenuItem'; -import Select, { SelectChangeEvent } from '@mui/material/Select'; -import Tooltip from '@mui/material/Tooltip'; - -import { useAppDispatch, useAppSelector } from '../../hooks/app'; -import useAdvancedColumns from '../../hooks/useAdvancedColumns'; -import useRawSearchParams from '../../hooks/useRawSearchParams'; -import { - updateShowCliffsDelta, - updateShowCles, - updateShowSignificance, -} from '../../reducers/ColumnPrefsSlice'; -import type { AdvancedColumns } from '../../types/types'; -import { - ADVANCED_COLUMNS_PARAM, - CLIFFS_DELTA, - CLES, - 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 -// and the serializer share one source of truth. Shared by the main and -// subtests controls. -const COLUMN_OPTIONS = [ - { key: CLIFFS_DELTA, label: "Cliff's Delta" }, - { key: CLES, label: 'CLES' }, - { key: SIGNIFICANCE, label: 'Significance' }, -] as const; - -function AdvancedColumnsMenu() { - const dispatch = useAppDispatch(); - const mode = useAppSelector((state) => state.theme.mode); - const advancedColumns = useAdvancedColumns(); - 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); - const [tooltipOpen, setTooltipOpen] = useState(false); - - // Derive the selected option values from the same serializer used for the - // URL, so the Select's value can't drift from the URL encoding. - const selectedKeys = - serializeAdvancedColumns(advancedColumns)?.split(',') ?? []; - - const applyAdvancedColumns = (next: AdvancedColumns) => { - dispatch(updateShowCliffsDelta(next.cliffsDelta)); - dispatch(updateShowCles(next.cles)); - dispatch(updateShowSignificance(next.significance)); - - // 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); - } else { - params.delete(ADVANCED_COLUMNS_PARAM); - } - updateRawSearchParams(params); - }; - - const onChange = (event: SelectChangeEvent) => { - const { value } = event.target; - const keys = typeof value === 'string' ? value.split(',') : value; - applyAdvancedColumns({ - cliffsDelta: keys.includes(CLIFFS_DELTA), - cles: keys.includes(CLES), - significance: keys.includes(SIGNIFICANCE), - }); - }; - - return ( - setTooltipOpen(true)} - onClose={() => setTooltipOpen(false)} - > - - - - - ); -} - -export default AdvancedColumnsMenu; diff --git a/src/components/CompareResults/AdvancedOptionsMenu.tsx b/src/components/CompareResults/AdvancedOptionsMenu.tsx new file mode 100644 index 000000000..7a8ed1855 --- /dev/null +++ b/src/components/CompareResults/AdvancedOptionsMenu.tsx @@ -0,0 +1,158 @@ +import { useState } from 'react'; + +import Checkbox from '@mui/material/Checkbox'; +import FormControl from '@mui/material/FormControl'; +import ListItemText from '@mui/material/ListItemText'; +import ListSubheader from '@mui/material/ListSubheader'; +import MenuItem from '@mui/material/MenuItem'; +import Select, { SelectChangeEvent } from '@mui/material/Select'; +import Tooltip from '@mui/material/Tooltip'; + +import { useAppDispatch, useAppSelector } from '../../hooks/app'; +import useAdvancedColumns from '../../hooks/useAdvancedColumns'; +import useExpandedRowOptions from '../../hooks/useExpandedRowOptions'; +import useRawSearchParams from '../../hooks/useRawSearchParams'; +import { + updateShowCliffsDelta, + updateShowCles, + updateShowSignificance, + updateExpandedRow, +} from '../../reducers/ColumnPrefsSlice'; +import type { AdvancedColumns, ExpandedRowOptions } from '../../types/types'; +import { + ADVANCED_COLUMNS_PARAM, + CLIFFS_DELTA, + CLES, + SIGNIFICANCE, + serializeAdvancedColumns, +} from '../../utils/advancedColumnsUrl'; +import { + EXPANDED_ROW_PARAM, + EFFECT_SIZE, + MODES, + STATS_TABLE, + WARNINGS, + serializeExpandedRow, +} from '../../utils/expandedRowUrl'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; + +// Power-user options for the Mann-Whitney-U view, in two independent groups: +// extra table columns and extra expanded-row components. Any combination can be +// shown. Option values reuse the URL keys so the dropdown, the URL and the +// serializers share one source of truth. Shared by the main and subtests +// controls. +const COLUMN_OPTIONS = [ + { key: CLIFFS_DELTA, label: "Cliff's Delta" }, + { key: CLES, label: 'CLES' }, + { key: SIGNIFICANCE, label: 'Significance' }, +] as const; + +const EXPANDED_OPTIONS = [ + { key: EFFECT_SIZE, label: 'Effect size & confidence intervals' }, + { key: MODES, label: 'Mode analysis' }, + { key: STATS_TABLE, label: 'Statistics table' }, + { key: WARNINGS, label: 'Data warnings' }, +] as const; + +function AdvancedOptionsMenu() { + const dispatch = useAppDispatch(); + const mode = useAppSelector((state) => state.theme.mode); + const advancedColumns = useAdvancedColumns(); + const expandedRow = useExpandedRowOptions(); + 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); + const [tooltipOpen, setTooltipOpen] = useState(false); + + // Derive the selected option values from the same serializers used for the + // URL, so the Select's value can't drift from the URL encoding. + const selectedKeys = [ + ...(serializeAdvancedColumns(advancedColumns)?.split(',') ?? []), + ...(serializeExpandedRow(expandedRow)?.split(',') ?? []), + ]; + + const applyAdvancedOptions = (keys: string[]) => { + const columns: AdvancedColumns = { + cliffsDelta: keys.includes(CLIFFS_DELTA), + cles: keys.includes(CLES), + significance: keys.includes(SIGNIFICANCE), + }; + const expanded: ExpandedRowOptions = { + effectSize: keys.includes(EFFECT_SIZE), + modes: keys.includes(MODES), + statsTable: keys.includes(STATS_TABLE), + warnings: keys.includes(WARNINGS), + }; + + dispatch(updateShowCliffsDelta(columns.cliffsDelta)); + dispatch(updateShowCles(columns.cles)); + dispatch(updateShowSignificance(columns.significance)); + dispatch(updateExpandedRow(expanded)); + + // Write both params onto the live URL so neither group clobbers the other. + const params = currentUrlParams(); + const setOrDelete = (param: string, value: string | null) => + value ? params.set(param, value) : params.delete(param); + setOrDelete(ADVANCED_COLUMNS_PARAM, serializeAdvancedColumns(columns)); + setOrDelete(EXPANDED_ROW_PARAM, serializeExpandedRow(expanded)); + updateRawSearchParams(params); + }; + + const onChange = (event: SelectChangeEvent) => { + const { value } = event.target; + const keys = typeof value === 'string' ? value.split(',') : value; + applyAdvancedOptions(keys); + }; + + const renderOption = ({ key, label }: { key: string; label: string }) => ( + + + + + ); + + return ( + setTooltipOpen(true)} + onClose={() => setTooltipOpen(false)} + > + + + + + ); +} + +export default AdvancedOptionsMenu; diff --git a/src/components/CompareResults/CommonGraph.tsx b/src/components/CompareResults/CommonGraph.tsx index 0bbb2a027..9a6346346 100644 --- a/src/components/CompareResults/CommonGraph.tsx +++ b/src/components/CompareResults/CommonGraph.tsx @@ -47,12 +47,12 @@ function computeMax(a?: number, b?: number) { return Math.max(a, b); } -const LABEL_ROW_PX = 16; // vertical space per stagger level -const KDE_TOP_BASE = 28; +const LABEL_ROW_PX = 16; +const KDE_TOP_BASE = 44; const KDE_HEIGHT = 155; -const SCATTER_TOP_BASE = 250; +const SCATTER_TOP_BASE = 274; const SCATTER_HEIGHT = 50; -const CHART_HEIGHT_BASE = 340; +const CHART_HEIGHT_BASE = 364; // Axis/grid line greys, shared across both grids and the tooltip crosshair. const AXIS_LINE_COLOR = '#999'; @@ -130,6 +130,7 @@ function CommonGraph({ onVtChange, showModes, onShowModesChange, + showModeControls = true, }: CommonGraphProps) { const chartContainerRef = useRef(null); const chartInstanceRef = useRef(null); @@ -472,7 +473,7 @@ function CommonGraph({ }, legend: { data: ['Base', 'New'], - top: 240, + top: 0, left: 'center', itemHeight: 10, itemWidth: 30, @@ -592,81 +593,89 @@ function CommonGraph({ Runs Density Distribution - - - {/* - MUI Slider exposes two events: `onChange` fires continuously during - drag (we send it to local state for a smooth thumb), and - `onChangeCommitted` fires once when the user releases (we push the - final value up to the parent then). This is the moral equivalent of - a debounce — the expensive consumer (`computeModeInfo`) runs once - per drag instead of on every pixel of movement. - */} - setLocalVt(value)} - onChangeCommitted={(_, value) => onVtChange(value)} - aria-label='Valley depth threshold' - sx={{ maxWidth: 240 }} - /> - - {Math.round(localVt * 100)}% - - onShowModesChange(checked)} - /> - } - label='Show modes' - sx={{ ml: 1, '& .MuiFormControlLabel-label': { fontSize: 14 } }} - /> - + Valley depth threshold + + + + : + + {/* + MUI Slider exposes two events: `onChange` fires continuously during + drag (we send it to local state for a smooth thumb), and + `onChangeCommitted` fires once when the user releases (we push the + final value up to the parent then). This is the moral equivalent of + a debounce — the expensive consumer (`computeModeInfo`) runs once + per drag instead of on every pixel of movement. + */} + setLocalVt(value)} + onChangeCommitted={(_, value) => onVtChange(value)} + aria-label='Valley depth threshold' + sx={{ maxWidth: 240 }} + /> + + {Math.round(localVt * 100)}% + + onShowModesChange(checked)} + /> + } + label='Show modes' + sx={{ ml: 1, '& .MuiFormControlLabel-label': { fontSize: 14 } }} + /> + + )}
void; showModes: boolean; onShowModesChange: (value: boolean) => void; + // Whether to render the mode-analysis controls (valley-depth slider + "Show + // modes" checkbox). Defaults to true; the Mann-Whitney-U simplified view sets + // it to false until the "Mode analysis" expanded-row option is turned on. + showModeControls?: boolean; } export default CommonGraph; diff --git a/src/components/CompareResults/FilteredRowsNotice.tsx b/src/components/CompareResults/FilteredRowsNotice.tsx new file mode 100644 index 000000000..aa743adca --- /dev/null +++ b/src/components/CompareResults/FilteredRowsNotice.tsx @@ -0,0 +1,40 @@ +import Alert from '@mui/material/Alert'; + +import type { ActiveColumnFilter } from '../../hooks/useTableFilters'; +import { Strings } from '../../resources/Strings'; + +interface FilteredRowsNoticeProps { + // How many rows the column filters are hiding (search-hidden rows excluded). + hiddenCount: number; + // The columns doing the hiding, with the value labels they exclude. + activeFilters: ActiveColumnFilter[]; +} + +// Tells the user that column filters are hiding rows, and which ones — so a +// comparison that shows fewer rows than expected doesn't read as missing data. +// Renders nothing when no rows are hidden. +function FilteredRowsNotice({ + hiddenCount, + activeFilters, +}: FilteredRowsNoticeProps) { + if (hiddenCount <= 0) { + return null; + } + + const reasons = activeFilters + .map((filter) => `${filter.name}: ${filter.excludedLabels.join(', ')}`) + .join(' • '); + + return ( + + {Strings.components.filteredRowsNotice.summary(hiddenCount)} + {reasons ? ` — ${reasons}` : ''} + + ); +} + +export default FilteredRowsNotice; diff --git a/src/components/CompareResults/KdeModesPanel.tsx b/src/components/CompareResults/KdeModesPanel.tsx index 1ac151467..b6fb47b59 100644 --- a/src/components/CompareResults/KdeModesPanel.tsx +++ b/src/components/CompareResults/KdeModesPanel.tsx @@ -311,6 +311,10 @@ type KdeModesPanelProps = { // throughput/score-style metrics. Drives improvement/regression wording so // the blurb doesn't presuppose timing. lowerIsBetter: boolean; + // When true, render a placeholder box instead of nothing if no mode-by-mode + // breakdown could be produced (e.g. a unimodal comparison). Used by the Mann- + // Whitney-U expanded grid so the "Mode analysis" cell isn't left empty. + showEmptyState?: boolean; }; function KdeModesPanel({ @@ -321,6 +325,7 @@ function KdeModesPanel({ vt, showModes, lowerIsBetter, + showEmptyState = false, }: KdeModesPanelProps) { // ECharts-equivalent reasoning: MUI's ThemeProvider sets the Box background // for us, but the inline text colors for the success/regression signals @@ -358,7 +363,26 @@ function KdeModesPanel({ }; }, [blurb, unit, palette, lowerIsBetter]); - if (!blurb || !derived) return null; + if (!blurb || !derived) { + // Mode analysis was requested (showModes) but this comparison didn't yield + // a multi-mode breakdown — surface that instead of leaving an empty cell. + if (showEmptyState && showModes) { + return ( + + No mode analysis available + + ); + } + return null; + } const { pairs, unmatchedBase, unmatchedNew, baseModes, newModes } = blurb; const { v, verdictColor, unitLabel, baseCount, newCount, modeStr } = derived; diff --git a/src/components/CompareResults/MannWhitneyCompareMetrics.tsx b/src/components/CompareResults/MannWhitneyCompareMetrics.tsx index 9c0e14850..f4d8555f3 100644 --- a/src/components/CompareResults/MannWhitneyCompareMetrics.tsx +++ b/src/components/CompareResults/MannWhitneyCompareMetrics.tsx @@ -57,7 +57,6 @@ export const MannWhitneyCompareMetrics = ({ sx={{ backgroundColor: 'manWhitneyComps.compareMetricsBg', marginBottom: 2, - maxWidth: '85%', width: '100%', borderRadius: '5px', padding: 2, diff --git a/src/components/CompareResults/ResultsControls.tsx b/src/components/CompareResults/ResultsControls.tsx index 35c3b8ea4..c89e05b81 100644 --- a/src/components/CompareResults/ResultsControls.tsx +++ b/src/components/CompareResults/ResultsControls.tsx @@ -6,7 +6,7 @@ import Grid from '@mui/material/Grid'; import Tooltip from '@mui/material/Tooltip'; import { style } from 'typestyle'; -import AdvancedColumnsMenu from './AdvancedColumnsMenu'; +import AdvancedOptionsMenu from './AdvancedOptionsMenu'; import { DownloadButton } from './DownloadButton'; import RevisionSelect from './RevisionSelect'; import SearchInput from './SearchInput'; @@ -151,7 +151,7 @@ export default function ResultsControls({ - + diff --git a/src/components/CompareResults/ResultsTable.tsx b/src/components/CompareResults/ResultsTable.tsx index bacc612c4..8f25fb4e6 100644 --- a/src/components/CompareResults/ResultsTable.tsx +++ b/src/components/CompareResults/ResultsTable.tsx @@ -13,7 +13,7 @@ import { MANN_WHITNEY_U } from '../../common/constants'; import useAdvancedColumns from '../../hooks/useAdvancedColumns'; import useInitializeTableStateFromCookies from '../../hooks/useInitializeTableStateFromCookies'; import useRawSearchParams from '../../hooks/useRawSearchParams'; -import useSeedAdvancedColumnsFromUrl from '../../hooks/useSeedAdvancedColumnsFromUrl'; +import useSeedAdvancedOptionsFromUrl from '../../hooks/useSeedAdvancedOptionsFromUrl'; import useTableFilters from '../../hooks/useTableFilters'; import useTableSort from '../../hooks/useTableSort'; import { Framework, TestVersion } from '../../types/types'; @@ -39,7 +39,7 @@ export default function ResultsTable() { // This is our custom hook that updates the search params without a rerender. const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); - useSeedAdvancedColumnsFromUrl(); + useSeedAdvancedOptionsFromUrl(); const advancedColumns = useAdvancedColumns(); const columnsConfig = useMemo( diff --git a/src/components/CompareResults/RevisionRowExpandable.tsx b/src/components/CompareResults/RevisionRowExpandable.tsx index efaf2afa4..3b399a1bf 100644 --- a/src/components/CompareResults/RevisionRowExpandable.tsx +++ b/src/components/CompareResults/RevisionRowExpandable.tsx @@ -7,10 +7,17 @@ import Stack from '@mui/material/Stack'; import CommonGraph from './CommonGraph'; import KdeModesPanel from './KdeModesPanel'; +import { MannWhitneyCompareMetrics } from './MannWhitneyCompareMetrics'; +import { StatisticsWarnings } from './StatisticsWarnings'; +import { MANN_WHITNEY_U } from '../../common/constants'; import { getStrategy } from '../../common/testVersions'; +import useExpandedRowOptions from '../../hooks/useExpandedRowOptions'; import { Strings } from '../../resources/Strings'; import { Spacing } from '../../styles'; -import type { CombinedResultsItemType } from '../../types/state'; +import type { + CombinedResultsItemType, + MannWhitneyResultsItem, +} from '../../types/state'; import { TestVersion } from '../../types/types'; import { bandwidthFor } from '../../utils/kdeAnalysis'; @@ -21,6 +28,14 @@ const LARGE_BW_RATIO = 0.5; const { singleRun } = Strings.components.expandableRow; +// Plain-language help shown above the density graph in the simplified +// Mann-Whitney-U expanded view. +const GRAPH_BLURB = + 'This graph shows how the Base and New results are distributed. Two curves ' + + 'that mostly overlap mean the builds performed about the same; curves that ' + + 'sit apart suggest a real difference — the further apart, the bigger the ' + + 'change.'; + function RevisionRowExpandable(props: RevisionRowExpandableProps) { const { result, id, testVersion } = props; @@ -45,6 +60,23 @@ function RevisionRowExpandable(props: RevisionRowExpandableProps) { } = result; const strategy = getStrategy(testVersion); + const isMannWhitney = testVersion === MANN_WHITNEY_U; + const expandedRow = useExpandedRowOptions(); + + // In the Mann-Whitney-U simplified view the mode-analysis controls (valley- + // depth slider + "Show modes" checkbox) and the on-chart mode overlays stay + // hidden until the "Mode analysis" expanded-row option is enabled. Student-T + // always shows them. + const modeAnalysisEnabled = isMannWhitney ? expandedRow.modes : true; + const showModesForChart = modeAnalysisEnabled && showModes; + + // The statistics table and data warnings are Mann-Whitney-U-only components. + const mwResult = result as MannWhitneyResultsItem; + const anyExpandedCell = + expandedRow.effectSize || + expandedRow.modes || + expandedRow.statsTable || + expandedRow.warnings; const baseValues = baseRunsReplicates && baseRunsReplicates.length @@ -79,6 +111,61 @@ function RevisionRowExpandable(props: RevisionRowExpandableProps) { return { sharedBw: sharedBwOut, isLargeBw: isLargeBwOut }; }, [baseValues, newValues, isSubtest, bwMultiplier]); + // Rendered identically in both layouts; declared once so the two branches + // don't duplicate the (long) prop lists. + const graph = + baseValues.length > 0 || newValues.length > 0 ? ( + + ) : null; + + const modesPanel = ( + + ); + + const comparisonSummary = ( + <> + {moreRunsAreNeeded &&
{singleRun}
} + {baseApplication && ( +
+ Base application: {baseApplication}{' '} +
+ )} + {newApplication && ( +
+ New application: {newApplication}{' '} +
+ )} + + Comparison result: {strategy.getComparisonResult(result)} ( + {lowerIsBetter ? 'lower' : 'higher'} is better) + + + ); + return ( {platform} - - - - {(baseValues.length > 0 || newValues.length > 0) && ( - - )} - {strategy.renderExpandedLeft(result)} - - - -
- {moreRunsAreNeeded &&
{singleRun}
} - {baseApplication && ( -
- Base application: {baseApplication}{' '} -
- )} - {newApplication && ( + {isMannWhitney ? ( + // Simplified Mann-Whitney-U view: full-width graph with a how-to-read + // blurb. The heavier statistical components are shown only when their + // "Advanced options → Expanded row" checkbox is on, laid out below the + // graph in a two-column grid so they pair up (effect size + mode + // analysis on one row, statistics table + data warnings on the next) + // instead of each spanning the full width. + +
{comparisonSummary}
+ {GRAPH_BLURB} + {graph} + {anyExpandedCell && ( + + {expandedRow.effectSize && ( + + {strategy.renderExpandedRight(result)} + + )} + {expandedRow.modes && ( + {modesPanel} + )} + {expandedRow.statsTable && ( + + + + )} + {expandedRow.warnings && ( + + + + )} + + )} + + ) : ( + <> + + + + {graph} + {strategy.renderExpandedLeft(result)} + + +
- New application: {newApplication}{' '} + {comparisonSummary} + {strategy.renderExpandedRight(result)} + {modesPanel}
- )} - - Comparison result: {strategy.getComparisonResult(result)}{' '} - ({lowerIsBetter ? 'lower' : 'higher'} is better) - - {strategy.renderExpandedRight(result)} - -
- - - {strategy.renderExpandedBottom(result)} + + + {strategy.renderExpandedBottom(result)} + + )} ); diff --git a/src/components/CompareResults/StatisticsWarnings.tsx b/src/components/CompareResults/StatisticsWarnings.tsx index ca3ad4065..88a99005e 100644 --- a/src/components/CompareResults/StatisticsWarnings.tsx +++ b/src/components/CompareResults/StatisticsWarnings.tsx @@ -20,7 +20,7 @@ export const StatisticsWarnings = ({ flexWrap: 'wrap', borderRadius: 1, padding: 1, - width: '55%', + width: '100%', '& .warning-row': { verticalAlign: 'bottom', display: 'flex', diff --git a/src/components/CompareResults/SubtestsResults/SubtestsResultsMain.tsx b/src/components/CompareResults/SubtestsResults/SubtestsResultsMain.tsx index d00c4f945..4c5909d7e 100644 --- a/src/components/CompareResults/SubtestsResults/SubtestsResultsMain.tsx +++ b/src/components/CompareResults/SubtestsResults/SubtestsResultsMain.tsx @@ -10,7 +10,7 @@ import { style } from 'typestyle'; import SubtestsBreadcrumbs from './SubtestsBreadcrumbs'; import SubtestsResultsTable from './SubtestsResultsTable'; import SubtestsRevisionHeader from './SubtestsRevisionHeader'; -import AdvancedColumnsMenu from '.././AdvancedColumnsMenu'; +import AdvancedOptionsMenu from '.././AdvancedOptionsMenu'; import { DownloadButton, DisabledDownloadButton } from '.././DownloadButton'; import SearchInput from '.././SearchInput'; import { @@ -93,7 +93,7 @@ function SubtestsResultsHeader({ - + diff --git a/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx b/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx index b2c9e303c..f14330324 100644 --- a/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx +++ b/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx @@ -5,13 +5,17 @@ import CircularProgress from '@mui/material/CircularProgress'; import { Await } from 'react-router'; import SubtestsTableContent from './SubtestsTableContent'; +import FilteredRowsNotice from '.././FilteredRowsNotice'; import NoResultsFound from '.././NoResultsFound'; import TableHeader from '.././TableHeader'; import { STUDENT_T } from '../../../common/constants'; import useAdvancedColumns from '../../../hooks/useAdvancedColumns'; import useInitializeTableStateFromCookies from '../../../hooks/useInitializeTableStateFromCookies'; -import useSeedAdvancedColumnsFromUrl from '../../../hooks/useSeedAdvancedColumnsFromUrl'; -import useTableFilters, { filterResults } from '../../../hooks/useTableFilters'; +import useSeedAdvancedOptionsFromUrl from '../../../hooks/useSeedAdvancedOptionsFromUrl'; +import useTableFilters, { + filterResults, + getFilterHiddenSummary, +} from '../../../hooks/useTableFilters'; import useTableSort, { sortResults } from '../../../hooks/useTableSort'; import type { CombinedResultsItemType } from '../../../types/state'; import type { TestVersion } from '../../../types/types'; @@ -82,7 +86,7 @@ function SubtestsResultsTable({ replicates, testVersion, }: ResultsTableProps) { - useSeedAdvancedColumnsFromUrl(); + useSeedAdvancedOptionsFromUrl(); const advancedColumns = useAdvancedColumns(); const columnsConfiguration = useMemo( () => @@ -160,12 +164,42 @@ function SubtestsResultsTable({ return processResults(filteredAndSortedResults); }, [filteredAndSortedResults]); + // Which column filters are narrowing the view, and how many rows + // they hide (excluding rows the search term already removed). + const { activeFilters, hiddenCount: hiddenByFilters } = + getFilterHiddenSummary( + columnsConfiguration, + results, + filteringSearchTerm, + tableFilters, + resultMatchesSearchTerm, + filteredResults.length, + ); + + // Gate on the count here (rather than only inside + // FilteredRowsNotice) so that when nothing is hidden no element + // enters the tree — otherwise its slot would shift the React- + // generated ids of the rows below it. + const notice = + hiddenByFilters > 0 ? ( + + ) : null; + if (processedResults.length === 0) { - return ; + return ( + <> + {notice} + + + ); } return ( <> + {notice} {processedResults.map((res) => ( state.comparison.activeComparison, ); - const filteredResults = useMemo(() => { - const resultsForCurrentComparison = + const resultsForCurrentComparison = useMemo( + () => activeComparison === allRevisionsOption ? results.flat() : (results?.find((result) => result[0]?.new_rev === activeComparison) ?? - []); + []), + [ + results, + activeComparison, + testVersion, // trigger refetch for new testVersion selection + ], + ); - const filteredResults = filterResults( + const filteredResults = useMemo(() => { + return filterResults( columnsConfiguration, resultsForCurrentComparison, filteringSearchTerm, tableFilters, resultMatchesSearchTerm, ); - return filteredResults; }, [ - results, - activeComparison, + resultsForCurrentComparison, filteringSearchTerm, tableFilters, columnsConfiguration, - testVersion, // trigger refetch for new testVersion selection ]); + // Which column filters are narrowing the view, and how many rows they hide + // (excluding rows the search term already removed). Powers FilteredRowsNotice. + const { activeFilters, hiddenCount: hiddenByFilters } = useMemo( + () => + getFilterHiddenSummary( + columnsConfiguration, + resultsForCurrentComparison, + filteringSearchTerm, + tableFilters, + resultMatchesSearchTerm, + filteredResults.length, + ), + [ + columnsConfiguration, + resultsForCurrentComparison, + filteringSearchTerm, + tableFilters, + filteredResults, + ], + ); + const sortedResults = useMemo(() => { return sortResults( columnsConfiguration, @@ -220,34 +249,53 @@ function TableContent({ return processResults(sortedResults); }, [sortedResults]); + // Gate on the count here (rather than only inside FilteredRowsNotice) so that + // when nothing is hidden no element enters the tree — otherwise its slot would + // shift the React-generated ids of the rows below it. + const notice = + hiddenByFilters > 0 ? ( + + ) : null; + if (!filteredResults.length) { - return ; + return ( + <> + {notice} + + + ); } return ( - header} - itemContent={(_, [, resultsForHeader]) => ( - - )} - /> + <> + {notice} + header} + itemContent={(_, [, resultsForHeader]) => ( + + )} + /> + ); } diff --git a/src/hooks/useExpandedRowOptions.ts b/src/hooks/useExpandedRowOptions.ts new file mode 100644 index 000000000..c593d8bad --- /dev/null +++ b/src/hooks/useExpandedRowOptions.ts @@ -0,0 +1,12 @@ +import { useAppSelector } from './app'; +import type { ExpandedRowOptions } from '../types/types'; + +// Visibility of the (power-user) components in the Mann-Whitney-U expanded row +// (effect size, mode analysis, statistics table, warnings) from the columnPrefs +// slice. The slice stores them as one object, so the reference is stable +// between updates and callers can use it directly in `useMemo` deps. +function useExpandedRowOptions(): ExpandedRowOptions { + return useAppSelector((state) => state.columnPrefs.expandedRow); +} + +export default useExpandedRowOptions; diff --git a/src/hooks/useSeedAdvancedColumnsFromUrl.ts b/src/hooks/useSeedAdvancedColumnsFromUrl.ts deleted file mode 100644 index 53d2b1634..000000000 --- a/src/hooks/useSeedAdvancedColumnsFromUrl.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { useEffect } from 'react'; - -import { useAppDispatch } from './app'; -import useRawSearchParams from './useRawSearchParams'; -import { - updateShowCliffsDelta, - updateShowCles, - updateShowSignificance, -} from '../reducers/ColumnPrefsSlice'; -import { - ADVANCED_COLUMNS_PARAM, - parseAdvancedColumns, -} from '../utils/advancedColumnsUrl'; - -// On mount, seed the advanced-column visibility from the URL so a shared link -// reproduces the selected columns. Toggling updates both the URL (for sharing) -// and Redux (for reactive rendering); this only handles the initial -// URL → Redux direction. Call once per results view. -function useSeedAdvancedColumnsFromUrl() { - const [rawSearchParams] = useRawSearchParams(); - const dispatch = useAppDispatch(); - useEffect(() => { - const params = new URLSearchParams(rawSearchParams); - if (!params.has(ADVANCED_COLUMNS_PARAM)) { - return; - } - const { cliffsDelta, cles, significance } = parseAdvancedColumns(params); - dispatch(updateShowCliffsDelta(cliffsDelta)); - dispatch(updateShowCles(cles)); - dispatch(updateShowSignificance(significance)); - }, []); -} - -export default useSeedAdvancedColumnsFromUrl; diff --git a/src/hooks/useSeedAdvancedOptionsFromUrl.ts b/src/hooks/useSeedAdvancedOptionsFromUrl.ts new file mode 100644 index 000000000..5d399d2a0 --- /dev/null +++ b/src/hooks/useSeedAdvancedOptionsFromUrl.ts @@ -0,0 +1,38 @@ +import { useEffect } from 'react'; + +import { useAppDispatch } from './app'; +import useRawSearchParams from './useRawSearchParams'; +import { + updateShowCliffsDelta, + updateShowCles, + updateShowSignificance, + updateExpandedRow, +} from '../reducers/ColumnPrefsSlice'; +import { + ADVANCED_COLUMNS_PARAM, + parseAdvancedColumns, +} from '../utils/advancedColumnsUrl'; +import { EXPANDED_ROW_PARAM, parseExpandedRow } from '../utils/expandedRowUrl'; + +// On mount, seed the advanced-options visibility (columns + expanded row) from +// the URL so a shared link reproduces the selection. Toggling updates both the +// URL (for sharing) and Redux (for reactive rendering); this only handles the +// initial URL → Redux direction. Call once per results view. +function useSeedAdvancedOptionsFromUrl() { + const [rawSearchParams] = useRawSearchParams(); + const dispatch = useAppDispatch(); + useEffect(() => { + const params = new URLSearchParams(rawSearchParams); + if (params.has(ADVANCED_COLUMNS_PARAM)) { + const { cliffsDelta, cles, significance } = parseAdvancedColumns(params); + dispatch(updateShowCliffsDelta(cliffsDelta)); + dispatch(updateShowCles(cles)); + dispatch(updateShowSignificance(significance)); + } + if (params.has(EXPANDED_ROW_PARAM)) { + dispatch(updateExpandedRow(parseExpandedRow(params))); + } + }, []); +} + +export default useSeedAdvancedOptionsFromUrl; diff --git a/src/hooks/useTableFilters.ts b/src/hooks/useTableFilters.ts index 07c3c18e5..15ec97073 100644 --- a/src/hooks/useTableFilters.ts +++ b/src/hooks/useTableFilters.ts @@ -157,6 +157,83 @@ const useTableFilters = ( export default useTableFilters; /* --- Functions used to implement the filtering --- */ + +// A column whose filter is actively hiding rows, with the human-readable labels +// of the values it excludes. Used to explain to the user which filters are +// hiding results (see FilteredRowsNotice). +export type ActiveColumnFilter = { + name: string; + excludedLabels: string[]; +}; + +// Describe every column whose filter is narrowing the results — i.e. some of +// its possible values are unchecked. A column with all values checked (or no +// entry in the map) isn't narrowing anything and is skipped. Labels come from +// the column configuration so they always match what the header shows. +function getActiveColumnFilters( + columnsConfiguration: CompareResultsTableConfig, + tableFilters: Map>, +): ActiveColumnFilter[] { + const active: ActiveColumnFilter[] = []; + for (const column of columnsConfiguration) { + if (!('filter' in column)) { + continue; + } + const checkedValues = tableFilters.get(column.key); + if (!checkedValues || checkedValues.size >= column.possibleValues.length) { + continue; + } + const excludedLabels = column.possibleValues + .filter(({ key }) => !checkedValues.has(key)) + .map(({ label }) => label); + if (excludedLabels.length) { + active.push({ name: column.name, excludedLabels }); + } + } + return active; +} + +// Stable empty filter map used to compute the "search only" result set, so we +// can tell how many rows the column filters (as opposed to the search term) are +// hiding. +const NO_FILTERS: Map> = new Map(); + +// Summarize what the active column filters are hiding, for FilteredRowsNotice: +// the active filters (with their excluded value labels) and how many rows they +// remove — counting only rows the column filters drop, not ones the search term +// already removed. `filteredCount` is the length of the already-computed +// search+filter result, passed in so we don't filter that set a second time. +export function getFilterHiddenSummary( + columnsConfiguration: CompareResultsTableConfig, + results: CombinedResultsItemType[], + searchTerm: string, + tableFilters: Map>, + resultMatchesSearchTerm: ( + result: CombinedResultsItemType, + searchTerm: string, + ) => boolean, + filteredCount: number, +): { activeFilters: ActiveColumnFilter[]; hiddenCount: number } { + const activeFilters = getActiveColumnFilters( + columnsConfiguration, + tableFilters, + ); + if (!activeFilters.length) { + return { activeFilters, hiddenCount: 0 }; + } + const searchOnlyResults = filterResults( + columnsConfiguration, + results, + searchTerm, + NO_FILTERS, + resultMatchesSearchTerm, + ); + return { + activeFilters, + hiddenCount: searchOnlyResults.length - filteredCount, + }; +} + function resultMatchesColumnFilter( columnsConfiguration: CompareResultsTableConfig, result: CombinedResultsItemType, diff --git a/src/reducers/ColumnPrefsSlice.ts b/src/reducers/ColumnPrefsSlice.ts index f0e0eec19..cf98f1d22 100644 --- a/src/reducers/ColumnPrefsSlice.ts +++ b/src/reducers/ColumnPrefsSlice.ts @@ -1,5 +1,7 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; +import type { ExpandedRowOptions } from '../types/types'; + export const HOW_TO_READ_STORAGE_KEY = 'showHowToRead'; export const MANN_WHITNEY_WARNING_STORAGE_KEY = 'showMannWhitneyWarning'; @@ -13,12 +15,16 @@ export const MANN_WHITNEY_WARNING_STORAGE_KEY = 'showMannWhitneyWarning'; // - showMannWhitneyWarning: when true the experimental Mann-Whitney-U warning // banner is shown (on both the Results and Subtests pages). Persisted to // localStorage so dismissing it sticks across reloads and both pages. +// - expandedRow: visibility of the (power-user) components in the MWU +// expanded row, toggled from the "Advanced options" dropdown. Persisted in +// the URL (see utils/expandedRowUrl). All off by default (simplified view). const initialState: { showCliffsDelta: boolean; showCles: boolean; showSignificance: boolean; showHowToRead: boolean; showMannWhitneyWarning: boolean; + expandedRow: ExpandedRowOptions; } = { showCliffsDelta: false, showCles: false, @@ -26,6 +32,12 @@ const initialState: { showHowToRead: localStorage.getItem(HOW_TO_READ_STORAGE_KEY) !== 'false', showMannWhitneyWarning: localStorage.getItem(MANN_WHITNEY_WARNING_STORAGE_KEY) !== 'false', + expandedRow: { + effectSize: false, + modes: false, + statsTable: false, + warnings: false, + }, }; const columnPrefs = createSlice({ @@ -52,6 +64,9 @@ const columnPrefs = createSlice({ String(action.payload), ); }, + updateExpandedRow(state, action: PayloadAction) { + state.expandedRow = action.payload; + }, }, }); @@ -61,5 +76,6 @@ export const { updateShowSignificance, updateShowHowToRead, updateShowMannWhitneyWarning, + updateExpandedRow, } = columnPrefs.actions; export default columnPrefs.reducer; diff --git a/src/resources/Strings.tsx b/src/resources/Strings.tsx index 171e628af..d68bee397 100644 --- a/src/resources/Strings.tsx +++ b/src/resources/Strings.tsx @@ -145,6 +145,11 @@ export const Strings = { mainMessage: 'No results found', note: 'For the selected revision(s), no results when compared to the base revision.', }, + filteredRowsNotice: { + // e.g. "1 row hidden by filters" / "3 rows hidden by filters" + summary: (count: number) => + `${count} ${count === 1 ? 'row' : 'rows'} hidden by filters`, + }, comparisonRevisionDropdown: { allRevisions: { key: 'all-revisions', diff --git a/src/types/types.ts b/src/types/types.ts index 76e2a60d3..acb179932 100644 --- a/src/types/types.ts +++ b/src/types/types.ts @@ -470,6 +470,16 @@ export interface AdvancedColumns { significance: boolean; } +// Visibility of the (power-user) components in the Mann-Whitney-U expanded row. +// Hidden by default (the simplified expanded view); toggled from the "Advanced +// options" dropdown. Persisted in the URL like AdvancedColumns. +export interface ExpandedRowOptions { + effectSize: boolean; + modes: boolean; + statsTable: boolean; + warnings: boolean; +} + export type SortFunc = ( resultA: CombinedResultsItemType, resultB: CombinedResultsItemType, diff --git a/src/utils/expandedRowUrl.ts b/src/utils/expandedRowUrl.ts new file mode 100644 index 000000000..619ca81b4 --- /dev/null +++ b/src/utils/expandedRowUrl.ts @@ -0,0 +1,47 @@ +import type { ExpandedRowOptions } from '../types/types'; + +// The advanced (power-user) components of the Mann-Whitney-U expanded row are +// persisted in the URL so a shared link reproduces the expanded view. Encoded +// as a comma-separated list of the enabled keys, e.g. +// `?advanced_expanded=modes,warnings`. Kept in a separate param from the +// advanced columns so the two groups stay independent. An absent param means +// none are shown (the simplified expanded view). +export const EXPANDED_ROW_PARAM = 'advanced_expanded'; + +// The URL key for each expanded-row option. Exported so the dropdown's option +// values reuse them and can't drift from the URL encoding. +export const EFFECT_SIZE = 'effect_size'; +export const MODES = 'modes'; +export const STATS_TABLE = 'stats_table'; +export const WARNINGS = 'warnings'; + +// Parse expanded-row visibility from a URL search string or params. +export function parseExpandedRow( + search: string | URLSearchParams, +): ExpandedRowOptions { + const params = + typeof search === 'string' ? new URLSearchParams(search) : search; + const enabled = (params.get(EXPANDED_ROW_PARAM) ?? '') + .split(',') + .filter(Boolean); + return { + effectSize: enabled.includes(EFFECT_SIZE), + modes: enabled.includes(MODES), + statsTable: enabled.includes(STATS_TABLE), + warnings: enabled.includes(WARNINGS), + }; +} + +// Serialize to the comma-list value, or null when nothing is on so the caller +// can delete the param and keep shared URLs clean. +export function serializeExpandedRow( + options: ExpandedRowOptions, +): string | null { + const enabled = [ + options.effectSize ? EFFECT_SIZE : null, + options.modes ? MODES : null, + options.statsTable ? STATS_TABLE : null, + options.warnings ? WARNINGS : null, + ].filter(Boolean); + return enabled.length ? enabled.join(',') : null; +}