refactor: impact metrics modal - label filtering and UX (#10377)

Modal for editing a chart now follows design of other parts of the app
more closely.

frontend/src/component/impact-metrics/ChartConfigModal/ChartConfigModal.tsx

@@ -8,12 +8,16 @@ import {
     TextField,
     Box,
     styled,
+    useTheme,
+    useMediaQuery,
+    Divider,
 } from '@mui/material';
 import { ImpactMetricsControls } from './ImpactMetricsControls/ImpactMetricsControls.tsx';
-import { ImpactMetricsChartPreview } from './ImpactMetricsChartPreview.tsx';
+import { useChartFormState } from '../hooks/useChartFormState.ts';
-import { useChartFormState } from './hooks/useChartFormState.ts';
+import type { ChartConfig } from '../types.ts';
-import type { ChartConfig } from './types.ts';
 import type { ImpactMetricsSeries } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata';
+import { LabelsFilter } from './LabelFilter/LabelsFilter.tsx';
+import { ImpactMetricsChart } from '../ImpactMetricsChart.tsx';
 
 export const StyledConfigPanel = styled(Box)(({ theme }) => ({
     display: 'flex',
@@ -62,6 +66,8 @@ export const ChartConfigModal: FC<ChartConfigModalProps> = ({
             open,
             initialConfig,
         });
+    const theme = useTheme();
+    const screenBreakpoint = useMediaQuery(theme.breakpoints.down('lg'));
 
     const handleSave = () => {
         if (!isValid) return;
@@ -111,21 +117,33 @@ export const ChartConfigModal: FC<ChartConfigModalProps> = ({
                             actions={actions}
                             metricSeries={metricSeries}
                             loading={loading}
-                            availableLabels={currentAvailableLabels}
                         />
                     </StyledConfigPanel>
                     <StyledPreviewPanel>
-                        <ImpactMetricsChartPreview
+                        <Box sx={(theme) => ({ padding: theme.spacing(1) })}>
-                            selectedSeries={formData.selectedSeries}
+                            <ImpactMetricsChart
-                            selectedRange={formData.selectedRange}
+                                key={screenBreakpoint ? 'small' : 'large'}
-                            selectedLabels={formData.selectedLabels}
+                                selectedSeries={formData.selectedSeries}
-                            beginAtZero={formData.beginAtZero}
+                                selectedRange={formData.selectedRange}
-                            aggregationMode={formData.aggregationMode}
+                                selectedLabels={formData.selectedLabels}
-                        />
+                                beginAtZero={formData.beginAtZero}
+                                aggregationMode={formData.aggregationMode}
+                                isPreview
+                            />
+                        </Box>
                     </StyledPreviewPanel>
                 </Box>
+
+                {currentAvailableLabels ? (
+                    <LabelsFilter
+                        selectedLabels={formData.selectedLabels}
+                        onChange={actions.setSelectedLabels}
+                        availableLabels={currentAvailableLabels}
+                    />
+                ) : null}
             </DialogContent>
-            <DialogActions>
+            <Divider />
+            <DialogActions sx={(theme) => ({ margin: theme.spacing(2, 3, 3) })}>
                 <Button onClick={onClose}>Cancel</Button>
                 <Button
                     onClick={handleSave}

frontend/src/component/impact-metrics/ChartConfigModal/ImpactMetricsControls/ImpactMetricsControls.tsx

@@ -0,0 +1,81 @@
+import type { FC } from 'react';
+import { Box, Typography, FormControlLabel, Checkbox } from '@mui/material';
+import type { ImpactMetricsSeries } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata';
+import { SeriesSelector } from './SeriesSelector/SeriesSelector.tsx';
+import { RangeSelector } from './RangeSelector/RangeSelector.tsx';
+import { ModeSelector } from './ModeSelector/ModeSelector.tsx';
+import type { ChartFormState } from '../../hooks/useChartFormState.ts';
+import { getMetricType } from '../../utils.ts';
+
+export type ImpactMetricsControlsProps = {
+    formData: ChartFormState['formData'];
+    actions: Pick<
+        ChartFormState['actions'],
+        | 'handleSeriesChange'
+        | 'setSelectedRange'
+        | 'setBeginAtZero'
+        | 'setSelectedLabels'
+        | 'setAggregationMode'
+    >;
+    metricSeries: (ImpactMetricsSeries & { name: string })[];
+    loading?: boolean;
+};
+
+export const ImpactMetricsControls: FC<ImpactMetricsControlsProps> = ({
+    formData,
+    actions,
+    metricSeries,
+    loading,
+}) => (
+    <Box>
+        <Box
+            sx={(theme) => ({
+                display: 'flex',
+                flexDirection: 'column',
+                gap: theme.spacing(3),
+            })}
+        >
+            <Typography variant='body2' color='text.secondary'>
+                Select a custom metric to see its value over time. This can help
+                you understand the impact of your feature rollout on key
+                outcomes, such as system performance, usage patterns or error
+                rates.
+            </Typography>
+
+            <SeriesSelector
+                value={formData.selectedSeries}
+                onChange={actions.handleSeriesChange}
+                options={metricSeries}
+                loading={loading}
+            />
+
+            {formData.selectedSeries ? (
+                <>
+                    <RangeSelector
+                        value={formData.selectedRange}
+                        onChange={actions.setSelectedRange}
+                    />
+                    <ModeSelector
+                        value={formData.aggregationMode}
+                        onChange={actions.setAggregationMode}
+                        seriesType={getMetricType(formData.selectedSeries)!}
+                    />
+                </>
+            ) : null}
+        </Box>
+        {formData.selectedSeries ? (
+            <FormControlLabel
+                sx={(theme) => ({ margin: theme.spacing(1.5, 0) })}
+                control={
+                    <Checkbox
+                        checked={formData.beginAtZero}
+                        onChange={(e) =>
+                            actions.setBeginAtZero(e.target.checked)
+                        }
+                    />
+                }
+                label='Begin at zero'
+            />
+        ) : null}
+    </Box>
+);

frontend/src/component/impact-metrics/ChartConfigModal/ImpactMetricsControls/ModeSelector/ModeSelector.tsx

@@ -1,6 +1,6 @@
 import type { FC } from 'react';
 import { FormControl, InputLabel, Select, MenuItem } from '@mui/material';
-import type { AggregationMode } from '../../types.ts';
+import type { AggregationMode } from '../../../types.ts';
 
 export type ModeSelectorProps = {
     value: AggregationMode;
@@ -15,11 +15,7 @@ export const ModeSelector: FC<ModeSelectorProps> = ({
 }) => {
     if (seriesType === 'unknown') return null;
     return (
-        <FormControl
+        <FormControl variant='outlined' size='small' sx={{ minWidth: 200 }}>
-            variant='outlined'
-            size='small'
-            sx={{ minWidth: 200, mt: 1 }}
-        >
             <InputLabel id='mode-select-label'>Mode</InputLabel>
             <Select
                 labelId='mode-select-label'

frontend/src/component/impact-metrics/ChartConfigModal/ImpactMetricsControls/SeriesSelector/SeriesSelector.tsx

@@ -47,6 +47,7 @@ export const SeriesSelector: FC<SeriesSelectorProps> = ({
                 placeholder='Search for a metric…'
                 variant='outlined'
                 size='small'
+                required
             />
         )}
         noOptionsText='No metrics available'

frontend/src/component/impact-metrics/ChartConfigModal/LabelFilter/LabelFilterItem/LabelFilterItem.tsx

@@ -0,0 +1,104 @@
+import {
+    Autocomplete,
+    Box,
+    Checkbox,
+    Chip,
+    FormControlLabel,
+    TextField,
+} from '@mui/material';
+import type { FC } from 'react';
+
+type LabelFilterItemProps = {
+    labelKey: string;
+    options: string[];
+    value: string[];
+    onChange: (values: string[]) => void;
+    handleAllToggle: (labelKey: string, checked: boolean) => void;
+};
+
+export const LabelFilterItem: FC<LabelFilterItemProps> = ({
+    labelKey,
+    options,
+    value,
+    onChange,
+}) => {
+    const isAllSelected = value.includes('*');
+    const autocompleteId = `autocomplete-${labelKey}`;
+    const selectAllId = `select-all-${labelKey}`;
+
+    return (
+        <>
+            <FormControlLabel
+                sx={(theme) => ({
+                    marginLeft: theme.spacing(0),
+                })}
+                control={
+                    <Checkbox
+                        id={selectAllId}
+                        size='small'
+                        checked={isAllSelected}
+                        onChange={(e) =>
+                            onChange(e.target.checked ? ['*'] : [])
+                        }
+                        inputProps={{
+                            'aria-describedby': autocompleteId,
+                            'aria-label': `Select all ${labelKey} options`,
+                        }}
+                    />
+                }
+                label={
+                    <Box
+                        component='span'
+                        sx={(theme) => ({
+                            fontSize: theme.fontSizes.smallBody,
+                        })}
+                    >
+                        Select all
+                    </Box>
+                }
+            />
+            <Autocomplete
+                multiple
+                id={autocompleteId}
+                options={options}
+                value={isAllSelected ? options : value}
+                onChange={(_, newValues) => {
+                    onChange(newValues);
+                }}
+                disabled={isAllSelected}
+                renderTags={(value, getTagProps) =>
+                    value.map((option, index) => {
+                        const { key, ...chipProps } = getTagProps({
+                            index,
+                        });
+                        return (
+                            <Chip
+                                {...chipProps}
+                                key={key}
+                                label={option}
+                                size='small'
+                            />
+                        );
+                    })
+                }
+                renderInput={(params) => (
+                    <TextField
+                        {...params}
+                        label={labelKey}
+                        placeholder={
+                            isAllSelected ? undefined : 'Select values…'
+                        }
+                        variant='outlined'
+                        size='small'
+                        inputProps={{
+                            ...params.inputProps,
+                            'aria-describedby': isAllSelected
+                                ? `${selectAllId}-description`
+                                : undefined,
+                        }}
+                    />
+                )}
+            />
+        </>
+    );
+};

frontend/src/component/impact-metrics/ChartConfigModal/LabelFilter/LabelsFilter.tsx

@@ -0,0 +1,102 @@
+import type { FC } from 'react';
+import { Box, Typography, Chip } from '@mui/material';
+import type { ImpactMetricsLabels } from 'hooks/api/getters/useImpactMetricsData/useImpactMetricsData';
+import { LabelFilterItem } from './LabelFilterItem/LabelFilterItem.tsx';
+
+export type LabelsFilterProps = {
+    selectedLabels: Record<string, string[]>;
+    onChange: (labels: Record<string, string[]>) => void;
+    availableLabels: ImpactMetricsLabels;
+};
+
+export const LabelsFilter: FC<LabelsFilterProps> = ({
+    selectedLabels,
+    onChange,
+    availableLabels,
+}) => {
+    const handleLabelChange = (labelKey: string, values: string[]) => {
+        const newLabels = { ...selectedLabels };
+        if (values.length === 0) {
+            delete newLabels[labelKey];
+        } else {
+            newLabels[labelKey] = values;
+        }
+        onChange(newLabels);
+    };
+
+    const handleAllToggle = (labelKey: string, checked: boolean) => {
+        const newLabels = { ...selectedLabels };
+        if (checked) {
+            newLabels[labelKey] = ['*'];
+        } else {
+            delete newLabels[labelKey];
+        }
+        onChange(newLabels);
+    };
+
+    const clearAllLabels = () => {
+        onChange({});
+    };
+
+    if (!availableLabels || Object.keys(availableLabels).length === 0) {
+        return null;
+    }
+
+    return (
+        <Box sx={{ display: 'flex', gap: 2, flexWrap: 'wrap' }}>
+            <Box
+                sx={{
+                    display: 'flex',
+                    alignItems: 'center',
+                    gap: 1,
+                    width: '100%',
+                }}
+            >
+                <Typography variant='subtitle2'>Filter by labels</Typography>
+                {Object.keys(selectedLabels).length > 0 && (
+                    <Chip
+                        label='Clear all'
+                        size='small'
+                        variant='outlined'
+                        onClick={clearAllLabels}
+                    />
+                )}
+            </Box>
+
+            <Box
+                sx={{
+                    display: 'grid',
+                    gridTemplateColumns:
+                        'repeat(auto-fill, minmax(300px, 1fr))',
+                    gap: 2,
+                    flexGrow: 1,
+                }}
+            >
+                {Object.entries(availableLabels).map(([labelKey, values]) => {
+                    const currentSelection = selectedLabels[labelKey] || [];
+
+                    return (
+                        <Box
+                            key={labelKey}
+                            sx={{
+                                display: 'flex',
+                                flexDirection: 'column',
+                                flexGrow: 1,
+                            }}
+                        >
+                            <LabelFilterItem
+                                labelKey={labelKey}
+                                options={values}
+                                value={currentSelection}
+                                onChange={(newValues) =>
+                                    handleLabelChange(labelKey, newValues)
+                                }
+                                handleAllToggle={handleAllToggle}
+                            />
+                        </Box>
+                    );
+                })}
+            </Box>
+        </Box>
+    );
+};

frontend/src/component/impact-metrics/ImpactMetrics.tsx

@@ -4,7 +4,7 @@ import { Typography, Button, Paper, styled } from '@mui/material';
 import Add from '@mui/icons-material/Add';
 import { PageHeader } from 'component/common/PageHeader/PageHeader.tsx';
 import { useImpactMetricsMetadata } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata';
-import { ChartConfigModal } from './ChartConfigModal.tsx';
+import { ChartConfigModal } from './ChartConfigModal/ChartConfigModal.tsx';
 import { ChartItem } from './ChartItem.tsx';
 import { GridLayoutWrapper, type GridItem } from './GridLayoutWrapper.tsx';
 import { useImpactMetricsState } from './hooks/useImpactMetricsState.ts';

frontend/src/component/impact-metrics/ImpactMetricsChartPreview.tsx

@@ -1,50 +0,0 @@
-import type { FC } from 'react';
-import { Box, Typography, useMediaQuery, useTheme } from '@mui/material';
-import { ImpactMetricsChart } from './ImpactMetricsChart.tsx';
-import type { AggregationMode } from './types.ts';
-
-type ImpactMetricsChartPreviewProps = {
-    selectedSeries: string;
-    selectedRange: 'hour' | 'day' | 'week' | 'month';
-    selectedLabels: Record<string, string[]>;
-    beginAtZero: boolean;
-    aggregationMode?: AggregationMode;
-};
-
-export const ImpactMetricsChartPreview: FC<ImpactMetricsChartPreviewProps> = ({
-    selectedSeries,
-    selectedRange,
-    selectedLabels,
-    beginAtZero,
-    aggregationMode,
-}) => {
-    const theme = useTheme();
-    const screenBreakpoint = useMediaQuery(theme.breakpoints.down('lg'));
-    const key = screenBreakpoint ? 'small' : 'large';
-
-    return (
-        <>
-            <Typography variant='h6' color='text.secondary'>
-                Preview
-            </Typography>
-
-            {!selectedSeries ? (
-                <Typography variant='body2' color='text.secondary'>
-                    Select a metric series to view the preview
-                </Typography>
-            ) : null}
-
-            <Box sx={(theme) => ({ padding: theme.spacing(1) })}>
-                <ImpactMetricsChart
-                    key={key}
-                    selectedSeries={selectedSeries}
-                    selectedRange={selectedRange}
-                    selectedLabels={selectedLabels}
-                    beginAtZero={beginAtZero}
-                    aggregationMode={aggregationMode}
-                    isPreview
-                />
-            </Box>
-        </>
-    );
-};

frontend/src/component/impact-metrics/ImpactMetricsControls/ImpactMetricsControls.tsx

@@ -1,88 +0,0 @@
-import type { FC } from 'react';
-import { Box, Typography, FormControlLabel, Checkbox } from '@mui/material';
-import type { ImpactMetricsSeries } from 'hooks/api/getters/useImpactMetricsMetadata/useImpactMetricsMetadata';
-import type { ImpactMetricsLabels } from 'hooks/api/getters/useImpactMetricsData/useImpactMetricsData';
-import { SeriesSelector } from './components/SeriesSelector.tsx';
-import { RangeSelector } from './components/RangeSelector.tsx';
-import { LabelsFilter } from './components/LabelsFilter.tsx';
-import { ModeSelector } from './components/ModeSelector.tsx';
-import type { ChartFormState } from '../hooks/useChartFormState.ts';
-import { getMetricType } from '../utils.ts';
-
-export type ImpactMetricsControlsProps = {
-    formData: ChartFormState['formData'];
-    actions: Pick<
-        ChartFormState['actions'],
-        | 'handleSeriesChange'
-        | 'setSelectedRange'
-        | 'setBeginAtZero'
-        | 'setSelectedLabels'
-        | 'setAggregationMode'
-    >;
-    metricSeries: (ImpactMetricsSeries & { name: string })[];
-    loading?: boolean;
-    availableLabels?: ImpactMetricsLabels;
-};
-
-export const ImpactMetricsControls: FC<ImpactMetricsControlsProps> = ({
-    formData,
-    actions,
-    metricSeries,
-    loading,
-    availableLabels,
-}) => (
-    <Box
-        sx={(theme) => ({
-            display: 'flex',
-            flexDirection: 'column',
-            gap: theme.spacing(3),
-        })}
-    >
-        <Typography variant='body2' color='text.secondary'>
-            Select a custom metric to see its value over time. This can help you
-            understand the impact of your feature rollout on key outcomes, such
-            as system performance, usage patterns or error rates.
-        </Typography>
-
-        <SeriesSelector
-            value={formData.selectedSeries}
-            onChange={actions.handleSeriesChange}
-            options={metricSeries}
-            loading={loading}
-        />
-
-        <RangeSelector
-            value={formData.selectedRange}
-            onChange={actions.setSelectedRange}
-        />
-
-        <Box sx={{ display: 'flex', flexDirection: 'column' }}>
-            <FormControlLabel
-                control={
-                    <Checkbox
-                        checked={formData.beginAtZero}
-                        onChange={(e) =>
-                            actions.setBeginAtZero(e.target.checked)
-                        }
-                    />
-                }
-                label='Begin at zero'
-            />
-
-            {formData.selectedSeries ? (
-                <ModeSelector
-                    value={formData.aggregationMode}
-                    onChange={actions.setAggregationMode}
-                    seriesType={getMetricType(formData.selectedSeries)!}
-                />
-            ) : null}
-        </Box>
-        {availableLabels && (
-            <LabelsFilter
-                selectedLabels={formData.selectedLabels}
-                onChange={actions.setSelectedLabels}
-                availableLabels={availableLabels}
-            />
-        )}
-    </Box>
-);

frontend/src/component/impact-metrics/ImpactMetricsControls/components/LabelsFilter.tsx

@@ -1,136 +0,0 @@
-import type { FC } from 'react';
-import {
-    Box,
-    Autocomplete,
-    TextField,
-    Typography,
-    Chip,
-    Checkbox,
-    FormControlLabel,
-} from '@mui/material';
-import type { ImpactMetricsLabels } from 'hooks/api/getters/useImpactMetricsData/useImpactMetricsData';
-
-export type LabelsFilterProps = {
-    selectedLabels: Record<string, string[]>;
-    onChange: (labels: Record<string, string[]>) => void;
-    availableLabels: ImpactMetricsLabels;
-};
-
-export const LabelsFilter: FC<LabelsFilterProps> = ({
-    selectedLabels,
-    onChange,
-    availableLabels,
-}) => {
-    const handleLabelChange = (labelKey: string, values: string[]) => {
-        const newLabels = { ...selectedLabels };
-        if (values.length === 0) {
-            delete newLabels[labelKey];
-        } else {
-            newLabels[labelKey] = values;
-        }
-        onChange(newLabels);
-    };
-
-    const handleAllToggle = (labelKey: string, checked: boolean) => {
-        const newLabels = { ...selectedLabels };
-        if (checked) {
-            newLabels[labelKey] = ['*'];
-        } else {
-            delete newLabels[labelKey];
-        }
-        onChange(newLabels);
-    };
-
-    const clearAllLabels = () => {
-        onChange({});
-    };
-
-    if (!availableLabels || Object.keys(availableLabels).length === 0) {
-        return null;
-    }
-
-    return (
-        <Box sx={{ display: 'flex', flexDirection: 'column', gap: 2 }}>
-            <Box sx={{ display: 'flex', alignItems: 'center', gap: 1 }}>
-                <Typography variant='subtitle2'>Filter by labels</Typography>
-                {Object.keys(selectedLabels).length > 0 && (
-                    <Chip
-                        label='Clear all'
-                        size='small'
-                        variant='outlined'
-                        onClick={clearAllLabels}
-                    />
-                )}
-            </Box>
-
-            {Object.entries(availableLabels).map(([labelKey, values]) => {
-                const currentSelection = selectedLabels[labelKey] || [];
-                const isAllSelected = currentSelection.includes('*');
-
-                return (
-                    <Box
-                        key={labelKey}
-                        sx={(theme) => ({
-                            display: 'flex',
-                            alignItems: 'center',
-                            gap: theme.spacing(3),
-                        })}
-                    >
-                        <Autocomplete
-                            multiple
-                            options={values}
-                            value={isAllSelected ? [] : currentSelection}
-                            onChange={(_, newValues) => {
-                                handleLabelChange(labelKey, newValues);
-                            }}
-                            disabled={isAllSelected}
-                            renderTags={(value, getTagProps) =>
-                                value.map((option, index) => {
-                                    const { key, ...chipProps } = getTagProps({
-                                        index,
-                                    });
-                                    return (
-                                        <Chip
-                                            {...chipProps}
-                                            key={key}
-                                            label={option}
-                                            size='small'
-                                        />
-                                    );
-                                })
-                            }
-                            renderInput={(params) => (
-                                <TextField
-                                    {...params}
-                                    label={labelKey}
-                                    placeholder={
-                                        isAllSelected
-                                            ? 'All values selected'
-                                            : 'Select values…'
-                                    }
-                                    variant='outlined'
-                                    size='small'
-                                />
-                            )}
-                            sx={{ minWidth: 300, flexGrow: 1 }}
-                        />
-                        <FormControlLabel
-                            control={
-                                <Checkbox
-                                    checked={isAllSelected}
-                                    onChange={(e) =>
-                                        handleAllToggle(
-                                            labelKey,
-                                            e.target.checked,
-                                        )
-                                    }
-                                />
-                            }
-                            label='All'
-                        />
-                    </Box>
-                );
-            })}
-        </Box>
-    );
-};