From dd5530b0a26b23b6f07d075a37fd40ceb1643245 Mon Sep 17 00:00:00 2001 From: Stephanie Roy Date: Wed, 6 Dec 2023 10:48:51 -0500 Subject: [PATCH 1/2] Simplify using the smooth signal on plots --- webview/src/plots/components/App.test.tsx | 25 ++------ .../components/vegaLite/ExtendedVegaLite.tsx | 62 ++++++------------- 2 files changed, 23 insertions(+), 64 deletions(-) diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index 8b13dfb467..b65f646566 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/webview/src/plots/components/App.test.tsx @@ -2853,10 +2853,7 @@ describe('App', () => { const smoothPlot = screen.getByTestId(`plot_${smoothId}`) await waitForVega(smoothPlot) - // eslint-disable-next-line testing-library/no-node-access - const slider = smoothPlot.querySelector( - '.vega-bindings input[name="smooth"]' - ) + const slider = within(smoothPlot).getByRole('slider') expect(slider).toBeInTheDocument() fireEvent.change(slider as HTMLInputElement, { target: { value: 0.4 } }) @@ -2876,10 +2873,7 @@ describe('App', () => { const popup = screen.getByTestId('zoomed-in-plot') await waitForVega(popup) - // eslint-disable-next-line testing-library/no-node-access - const slider = popup.querySelector( - '.vega-bindings input[name="smooth"]' - ) + const slider = within(popup).getByRole('slider') expect(slider).toBeInTheDocument() fireEvent.change(slider as HTMLInputElement, { target: { value: 0.4 } }) @@ -2895,10 +2889,7 @@ describe('App', () => { const smoothPlot = screen.getByTestId(`plot_${smoothId}`) await waitForVega(smoothPlot) - // eslint-disable-next-line testing-library/no-node-access - const slider = smoothPlot.querySelector( - '.vega-bindings input[name="smooth"]' - ) + const slider = within(smoothPlot).getByRole('slider') expect(slider).toBeInTheDocument() expect(slider).toHaveValue('0.6') @@ -2918,10 +2909,7 @@ describe('App', () => { const popup = screen.getByTestId('zoomed-in-plot') await waitForVega(popup) - // eslint-disable-next-line testing-library/no-node-access - const slider = popup.querySelector( - '.vega-bindings input[name="smooth"]' - ) + const slider = within(popup).getByRole('slider') expect(slider).toBeInTheDocument() expect(slider).toHaveValue('0.6') @@ -2936,10 +2924,7 @@ describe('App', () => { await waitForVega(smoothPlot) - // eslint-disable-next-line testing-library/no-node-access - const slider = smoothPlot.querySelector( - '.vega-bindings input[name="smooth"]' - ) + const slider = within(smoothPlot).getByRole('slider') expect(slider).toBeInTheDocument() expect(slider).toHaveValue('0.2') diff --git a/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx b/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx index f49bb3c04f..d43687d25c 100644 --- a/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx +++ b/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx @@ -8,11 +8,6 @@ import { setSmoothPlotValues } from '../../util/messages' import { config } from '../constants' import { useGetPlot } from '../../hooks/useGetPlot' -interface VegaState { - signals?: { [name: string]: number | undefined } - data?: unknown -} - export const ExtendedVegaLite = ({ actions, id, @@ -44,68 +39,47 @@ export const ExtendedVegaLite = ({ } as VegaLiteProps const vegaView = useRef() - const plotWrapperEl = useRef(null) const smoothPlotValues = useSelector( (state: PlotsState) => state.template.smoothPlotValues ) const changeDebounceTimer = useRef(0) + const currentValue = smoothPlotValues[id] useEffect(() => { - const newValue = smoothPlotValues[id] - if (!newValue || !vegaView.current) { + if (!currentValue || !vegaView.current) { return } - const currentState: VegaState = vegaView.current.getState() - const currentValue: number | undefined = currentState?.signals?.smooth - if (newValue !== currentValue) { - vegaView.current.setState({ - ...currentState, - signals: { ...currentState.signals, smooth: newValue } - }) + if (vegaView.current.signal('smooth') !== currentValue) { + vegaView.current.signal('smooth', currentValue) + vegaView.current.run() } - }, [smoothPlotValues, id]) - - const addRangeEventListener = () => { - const smoothRange = plotWrapperEl.current?.querySelector( - 'input[name="smooth"]' - ) - - smoothRange?.addEventListener('change', (event: Event) => { - if (event.target) { - window.clearTimeout(changeDebounceTimer.current) - changeDebounceTimer.current = window.setTimeout(() => { - setSmoothPlotValues( - id, - Number((event.target as HTMLInputElement).value) - ) - }, 500) - } - }) - } + }, [currentValue]) return ( - + { onNewView(view) vegaView.current = view - const defaultValue = smoothPlotValues[id] - const state = view.getState() as VegaState - if (!state?.signals?.smooth) { + if (!view.signal('smooth')) { return } - if (defaultValue) { - view.setState({ - ...state, - signals: { ...state.signals, smooth: defaultValue } - }) + if (currentValue) { + view.signal('smooth', currentValue) + view.run() } - addRangeEventListener() + view.addSignalListener('smooth', (_, value) => { + window.clearTimeout(changeDebounceTimer.current) + changeDebounceTimer.current = window.setTimeout( + () => setSmoothPlotValues(id, Number(value)), + 500 + ) + }) }} /> From 0c94ade403cb6afc14e42ee2d43b4c7a0ac94dca Mon Sep 17 00:00:00 2001 From: Stephanie Roy Date: Wed, 6 Dec 2023 10:53:10 -0500 Subject: [PATCH 2/2] Add a finalize event to the vega view --- webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx b/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx index d43687d25c..16371ab7d7 100644 --- a/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx +++ b/webview/src/plots/components/vegaLite/ExtendedVegaLite.tsx @@ -45,6 +45,12 @@ export const ExtendedVegaLite = ({ const changeDebounceTimer = useRef(0) const currentValue = smoothPlotValues[id] + useEffect(() => { + return () => { + vegaView.current?.finalize() + } + }, []) + useEffect(() => { if (!currentValue || !vegaView.current) { return