From 1d35b1b3af26f20c5aef8f3f00b0a59cd67ed574 Mon Sep 17 00:00:00 2001 From: Nate Kelley Date: Wed, 7 May 2025 13:03:15 -0600 Subject: [PATCH] fix broken scatter menu --- .../bar-chart-navigation.spec.ts | 4 +++- .../scatter-chart-updates.test.ts | 21 ++++++++++++------- .../hooks/useSeriesOptions/interfaces.ts | 1 - .../scatterSeriesBuilder.test.ts | 2 -- .../useSeriesOptions/scatterSeriesBuilder.ts | 8 ++++++- web/src/components/ui/slider/Slider.tsx | 20 +++++++----------- .../StylingAppStyling/EditScatterDotSize.tsx | 1 - 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/web/playwright-tests/bar-chart-navigation.spec.ts b/web/playwright-tests/bar-chart-navigation.spec.ts index 4bca73150..5e2a6b045 100644 --- a/web/playwright-tests/bar-chart-navigation.spec.ts +++ b/web/playwright-tests/bar-chart-navigation.spec.ts @@ -49,7 +49,9 @@ test('Can click start chat', async ({ page }) => { await page.waitForTimeout(500); await page.waitForLoadState('networkidle'); - await page.waitForTimeout(5000); + await page.waitForLoadState('domcontentloaded'); + await page.waitForLoadState('load'); + await page.waitForTimeout(8000); await expect(page).toHaveURL('http://localhost:3000/app/chats', { timeout: 30000 }); }); diff --git a/web/playwright-tests/scatter-chart-updates.test.ts b/web/playwright-tests/scatter-chart-updates.test.ts index 61a8c02f0..f66e34d95 100644 --- a/web/playwright-tests/scatter-chart-updates.test.ts +++ b/web/playwright-tests/scatter-chart-updates.test.ts @@ -26,24 +26,29 @@ test.describe.serial('Create a scatter plot with a question', () => { ); const url = page.url(); - await page.goto( - 'http://localhost:3000/app/chats/21cd1170-7ecf-4796-9d5e-9828285c62ec/metrics/0023f1a3-58fe-53f7-9f23-07f20868e1b4/chart?secondary_view=chart-edit' - ); + scatterURL = url; }); - scatterURL = - 'http://localhost:3000/app/chats/21cd1170-7ecf-4796-9d5e-9828285c62ec/metrics/0023f1a3-58fe-53f7-9f23-07f20868e1b4/chart'; + // scatterURL = + // 'http://localhost:3000/app/chats/84c1d148-4056-4aca-8741-29f2d11619c2/metrics/8c1e2db2-1cbb-532a-bf36-040c2431c7f3/chart?metric_version_number=1&secondary_view=chart-edit'; + test(`I can update the scatter plot`, async ({ page }) => { await page.goto(scatterURL); - + await expect(page.getByTestId('edit-chart-button').getByRole('button')).toBeVisible(); await page.getByTestId('edit-chart-button').getByRole('button').click(); - await expect(page.getByTestId('select-chart-type-scatter')).toBeVisible(); + await page.waitForTimeout(250); + await page.waitForLoadState('domcontentloaded'); + await page.waitForLoadState('load'); + await page.waitForTimeout(1500); + await expect(page.getByTestId('select-chart-type-scatter')).not.toBeVisible(); + await page.getByTestId('edit-chart-button').getByRole('button').click(); + await page.waitForTimeout(250); await expect(page.getByTestId('select-chart-type-scatter')).toHaveAttribute( 'data-state', 'selected' ); await page.getByTestId('segmented-trigger-Styling').click(); - // + await expect(page.getByText('Dot size')).toBeVisible(); }); }); diff --git a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/interfaces.ts b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/interfaces.ts index ec44556e3..88f24f698 100644 --- a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/interfaces.ts +++ b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/interfaces.ts @@ -7,7 +7,6 @@ export interface SeriesBuilderProps { colors: string[]; columnLabelFormats: NonNullable; xAxisKeys: ChartEncodes['x']; - sizeOptions: { key: string; minValue: number; diff --git a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.test.ts b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.test.ts index 52ea21aad..1383e3751 100644 --- a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.test.ts +++ b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.test.ts @@ -58,11 +58,9 @@ describe('scatterSeriesBuilder_data', () => { columnLabelFormats: baseColumnLabelFormats, xAxisKeys: mockXAxisKeys, sizeOptions: null, - categoryKeys: [], datasetOptions: baseDatasetOptions, columnSettings: {}, lineGroupType: null, - selectedChartType: ChartType.Scatter, barShowTotalAtTop: false, barGroupType: null, yAxisKeys: ['metric1'], diff --git a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.ts b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.ts index 3bdae0fc4..5bdfd865e 100644 --- a/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.ts +++ b/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/scatterSeriesBuilder.ts @@ -38,7 +38,13 @@ export const scatterSeriesBuilder_data = ({ const isXAxisDate = isDateColumnType(xAxisColumnLabelFormat.columnType); const hasSizeKeyIndex = sizeOptions !== null && !!sizeOptions.key; - const scatterElementConfig = hasSizeKeyIndex + + const useCustomScatterElementConfig = + hasSizeKeyIndex || + scatterDotSize?.[0] !== DEFAULT_CHART_CONFIG.scatterDotSize[0] || + scatterDotSize?.[1] !== DEFAULT_CHART_CONFIG.scatterDotSize[1]; + + const scatterElementConfig = useCustomScatterElementConfig ? { point: { radius: (context: ScriptableContext<'bubble'>) => diff --git a/web/src/components/ui/slider/Slider.tsx b/web/src/components/ui/slider/Slider.tsx index d9d59bc27..578f06f4c 100644 --- a/web/src/components/ui/slider/Slider.tsx +++ b/web/src/components/ui/slider/Slider.tsx @@ -11,6 +11,7 @@ import { import { cn } from '@/lib/utils'; import { ErrorBoundary } from '../error'; +import { useMemoizedFn } from '@/hooks'; export interface SliderProps extends React.ComponentPropsWithoutRef { min?: number; @@ -40,21 +41,16 @@ const Slider = React.forwardRef, S ); const currentValue: number[] = value || defaultValue || [min]; - const handleValueChange = React.useCallback( - (newValue: number[]) => { - onValueChange?.(newValue); - setInternalValues(newValue); - setUseTooltip(true); - }, - [onValueChange] - ); + const handleValueChange = useMemoizedFn((newValue: number[]) => { + onValueChange?.(newValue); + setInternalValues(newValue); + setUseTooltip(true); + }); - const handleValueCommit = React.useCallback(() => { + const handleValueCommit = useMemoizedFn(() => { setUseTooltip(false); setInternalValues(currentValue); - }, []); - - console.log(internalValues, value, defaultValue, min); + }); return ( Error}> diff --git a/web/src/controllers/MetricController/MetricViewChart/MetricEditController/MetricStylingApp/StylingAppStyling/EditScatterDotSize.tsx b/web/src/controllers/MetricController/MetricViewChart/MetricEditController/MetricStylingApp/StylingAppStyling/EditScatterDotSize.tsx index 98b6f6d58..3d03c0478 100644 --- a/web/src/controllers/MetricController/MetricViewChart/MetricEditController/MetricStylingApp/StylingAppStyling/EditScatterDotSize.tsx +++ b/web/src/controllers/MetricController/MetricViewChart/MetricEditController/MetricStylingApp/StylingAppStyling/EditScatterDotSize.tsx @@ -18,7 +18,6 @@ export const EditScatterDotSize: React.FC<{ const newLower = v[0]; const newUpper = hasSize ? v[1] : newLower + 18; const arrayFormat: [number, number] = [newLower, newUpper]; - console.log({ arrayFormat }); onUpdateChartConfig({ scatterDotSize: arrayFormat });