From e21c2b420674aa9176d5279ad47a39d21834106b Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Fri, 9 Dec 2022 10:10:17 +0100 Subject: [PATCH 1/5] Fix conflicting interactions in selection stories --- .../src/AxialSelectToZoom.stories.tsx | 10 ++++- .../src/DefaultInteractions.stories.tsx | 38 ++++++++++++++++--- apps/storybook/src/SelectToZoom.stories.tsx | 6 ++- apps/storybook/src/Selection.stories.tsx | 6 +-- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/apps/storybook/src/AxialSelectToZoom.stories.tsx b/apps/storybook/src/AxialSelectToZoom.stories.tsx index e3b9da4aa..8568e844c 100644 --- a/apps/storybook/src/AxialSelectToZoom.stories.tsx +++ b/apps/storybook/src/AxialSelectToZoom.stories.tsx @@ -20,6 +20,7 @@ import { useMockData } from './hooks'; const { oneD, twoD } = mockValues; const Template: Story = (args) => { + const { modifierKey } = args; const domain = useDomain(oneD); assertDefined(domain); @@ -28,7 +29,7 @@ const Template: Story = (args) => { abscissaConfig={{ visDomain: [0, oneD.length], showGrid: true }} ordinateConfig={{ visDomain: domain, showGrid: true }} > - + @@ -78,7 +79,7 @@ export const DisabledInsideEqualAspectCanvas: Story = ( ordinateConfig={{ visDomain: [0, 20], showGrid: true }} aspect="equal" > - + @@ -93,6 +94,11 @@ export const DisabledInsideEqualAspectCanvas: Story = ( ); }; +DisabledInsideEqualAspectCanvas.argTypes = { + modifierKey: { control: false }, + disabled: { control: false }, +}; + export default { title: 'Building Blocks/Interactions/AxialSelectToZoom', component: AxialSelectToZoom, diff --git a/apps/storybook/src/DefaultInteractions.stories.tsx b/apps/storybook/src/DefaultInteractions.stories.tsx index 14b65b0b0..81ad297fd 100644 --- a/apps/storybook/src/DefaultInteractions.stories.tsx +++ b/apps/storybook/src/DefaultInteractions.stories.tsx @@ -1,20 +1,39 @@ -import { DefaultInteractions, ResetZoomButton, VisCanvas } from '@h5web/lib'; +import { + DefaultInteractions, + HeatmapMesh, + mockValues, + ResetZoomButton, + ScaleType, + VisCanvas, +} from '@h5web/lib'; import type { DefaultInteractionsConfig } from '@h5web/lib/src/interactions/DefaultInteractions'; import type { Meta, Story } from '@storybook/react'; import FillHeight from './decorators/FillHeight'; +import { useMockData } from './hooks'; + +const { twoD } = mockValues; export const InsideAutoAspectCanvas: Story = ( args ) => { + const { values, domain } = useMockData(twoD, [20, 41]); + return ( + + ); }; @@ -22,14 +41,23 @@ export const InsideAutoAspectCanvas: Story = ( export const InsideEqualAspectCanvas: Story = ( args ) => { + const { values, domain } = useMockData(twoD, [20, 41]); + return ( + + ); }; diff --git a/apps/storybook/src/SelectToZoom.stories.tsx b/apps/storybook/src/SelectToZoom.stories.tsx index 5554875cd..1556fcc18 100644 --- a/apps/storybook/src/SelectToZoom.stories.tsx +++ b/apps/storybook/src/SelectToZoom.stories.tsx @@ -17,13 +17,14 @@ const { twoD } = mockValues; const Template: Story = (args) => { const { values, domain } = useMockData(twoD, [20, 41]); + const { modifierKey } = args; return ( - + @@ -42,6 +43,7 @@ export const InsideAutoAspectCanvas = Template.bind({}); export const InsideEqualAspectCanvas: Story = (args) => { const { values, domain } = useMockData(twoD, [20, 41]); + const { modifierKey } = args; return ( = (args) => { ordinateConfig={{ visDomain: [0, values.shape[0]], showGrid: true }} aspect="equal" > - + diff --git a/apps/storybook/src/Selection.stories.tsx b/apps/storybook/src/Selection.stories.tsx index ab71404ce..fcceba38d 100644 --- a/apps/storybook/src/Selection.stories.tsx +++ b/apps/storybook/src/Selection.stories.tsx @@ -170,9 +170,9 @@ export const Persisted: Story = (args) => { setPersistedSelection(undefined)} onSelectionEnd={setPersistedSelection} - modifierKey={selectionModifierKey} > {({ data: [dataStart, dataEnd] }) => !persistedSelection && ( @@ -192,8 +192,8 @@ export const Persisted: Story = (args) => { }; Persisted.args = { selectionType: 'rectangle', - selectionModifierKey: 'Control', - panModifierKey: undefined, + selectionModifierKey: undefined, + panModifierKey: 'Control', }; Persisted.argTypes = { selectionModifierKey: { From 824a583aaeea26c138bc6f439e1c41947c6ae26c Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Thu, 8 Dec 2022 15:50:15 +0100 Subject: [PATCH 2/5] Fix subtle race conditions in `SelectionTool` --- .../lib/src/interactions/SelectionTool.tsx | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/lib/src/interactions/SelectionTool.tsx b/packages/lib/src/interactions/SelectionTool.tsx index 58f0b8d0c..2b0be151e 100644 --- a/packages/lib/src/interactions/SelectionTool.tsx +++ b/packages/lib/src/interactions/SelectionTool.tsx @@ -2,6 +2,7 @@ import { assertDefined } from '@h5web/shared'; import { useKeyboardEvent, useRafState } from '@react-hookz/web'; import { useThree } from '@react-three/fiber'; import type { ReactNode } from 'react'; +import { useRef } from 'react'; import { useCallback, useEffect, useState } from 'react'; import { useVisCanvasContext } from '../vis/shared/VisCanvasProvider'; @@ -38,7 +39,7 @@ function SelectionTool(props: Props) { const camera = useThree((state) => state.camera); const { worldToData } = useVisCanvasContext(); - const [startEvt, setStartEvt] = useState>(); + const startEvtRef = useRef>(); const [selection, setSelection] = useRafState(); const modifierKeys = getModifierKeyArray(modifierKey); @@ -52,8 +53,8 @@ function SelectionTool(props: Props) { const computeSelection = useCallback( (evt: CanvasEvent): Selection => { - assertDefined(startEvt); - const { dataPt: dataStart, worldPt: worldStart } = startEvt; + assertDefined(startEvtRef.current); + const { dataPt: dataStart, worldPt: worldStart } = startEvtRef.current; const { worldPt: worldEnd } = evt; const boundWorldEnd = boundWorldPointToFOV(worldEnd, camera); @@ -63,7 +64,7 @@ function SelectionTool(props: Props) { data: [dataStart, worldToData(boundWorldEnd)], }; }, - [camera, startEvt, worldToData] + [camera, worldToData] ); const transformSelection = useCallback( @@ -82,8 +83,7 @@ function SelectionTool(props: Props) { const { target, pointerId } = sourceEvent; (target as Element).setPointerCapture(pointerId); - - setStartEvt(evt); + startEvtRef.current = evt; if (onSelectionStart) { onSelectionStart(evt); @@ -94,16 +94,16 @@ function SelectionTool(props: Props) { const onPointerMove = useCallback( (evt: CanvasEvent) => { - if (startEvt) { + if (startEvtRef.current) { setSelection(computeSelection(evt)); } }, - [startEvt, setSelection, computeSelection] + [setSelection, computeSelection] ); const onPointerUp = useCallback( (evt: CanvasEvent) => { - if (!startEvt) { + if (!startEvtRef.current) { return; } @@ -111,15 +111,14 @@ function SelectionTool(props: Props) { const { target, pointerId } = sourceEvent; (target as Element).releasePointerCapture(pointerId); - setStartEvt(undefined); - setSelection(undefined); - if (onSelectionEnd && shouldInteract(sourceEvent)) { onSelectionEnd(transformSelection(computeSelection(evt))); } + + startEvtRef.current = undefined; + setSelection(undefined); }, [ - startEvt, setSelection, onSelectionEnd, shouldInteract, @@ -133,7 +132,7 @@ function SelectionTool(props: Props) { useKeyboardEvent( 'Escape', () => { - setStartEvt(undefined); + startEvtRef.current = undefined; setSelection(undefined); }, [], From 4e56ad8032506f7d1d4ce697aef64f1c248cc3ba Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Fri, 9 Dec 2022 10:08:13 +0100 Subject: [PATCH 3/5] Avoid calling `transformSelection` twice --- .../lib/src/interactions/SelectionTool.tsx | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/packages/lib/src/interactions/SelectionTool.tsx b/packages/lib/src/interactions/SelectionTool.tsx index 2b0be151e..2ae8504af 100644 --- a/packages/lib/src/interactions/SelectionTool.tsx +++ b/packages/lib/src/interactions/SelectionTool.tsx @@ -3,7 +3,7 @@ import { useKeyboardEvent, useRafState } from '@react-hookz/web'; import { useThree } from '@react-three/fiber'; import type { ReactNode } from 'react'; import { useRef } from 'react'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect } from 'react'; import { useVisCanvasContext } from '../vis/shared/VisCanvasProvider'; import { @@ -29,7 +29,7 @@ function SelectionTool(props: Props) { id = 'Selection', modifierKey, disabled, - transformSelection: customTransformSelection, + transformSelection, onSelectionStart, onSelectionChange, onSelectionEnd, @@ -59,19 +59,16 @@ function SelectionTool(props: Props) { const { worldPt: worldEnd } = evt; const boundWorldEnd = boundWorldPointToFOV(worldEnd, camera); - return { + const newSelection: Selection = { world: [worldStart, boundWorldEnd], data: [dataStart, worldToData(boundWorldEnd)], }; - }, - [camera, worldToData] - ); - const transformSelection = useCallback( - (selec: Selection): Selection => { - return customTransformSelection ? customTransformSelection(selec) : selec; + return transformSelection + ? transformSelection(newSelection) + : newSelection; }, - [customTransformSelection] + [camera, transformSelection, worldToData] ); const onPointerDown = useCallback( @@ -85,9 +82,7 @@ function SelectionTool(props: Props) { (target as Element).setPointerCapture(pointerId); startEvtRef.current = evt; - if (onSelectionStart) { - onSelectionStart(evt); - } + onSelectionStart?.(evt); }, [onSelectionStart, shouldInteract] ); @@ -112,19 +107,13 @@ function SelectionTool(props: Props) { (target as Element).releasePointerCapture(pointerId); if (onSelectionEnd && shouldInteract(sourceEvent)) { - onSelectionEnd(transformSelection(computeSelection(evt))); + onSelectionEnd(computeSelection(evt)); } startEvtRef.current = undefined; setSelection(undefined); }, - [ - setSelection, - onSelectionEnd, - shouldInteract, - transformSelection, - computeSelection, - ] + [setSelection, onSelectionEnd, shouldInteract, computeSelection] ); useCanvasEvents({ onPointerDown, onPointerMove, onPointerUp }); @@ -141,15 +130,15 @@ function SelectionTool(props: Props) { useEffect(() => { if (onSelectionChange && selection && isModifierKeyPressed) { - onSelectionChange(transformSelection(selection)); + onSelectionChange(selection); } - }, [selection, isModifierKeyPressed, onSelectionChange, transformSelection]); + }, [selection, isModifierKeyPressed, onSelectionChange]); if (!selection || !isModifierKeyPressed) { return null; } - return <>{children(transformSelection(selection))}; + return <>{children(selection)}; } export type { Props as SelectionProps }; From 12357975d2c3b3f480d06ec0cc08066af625c4c5 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Fri, 9 Dec 2022 10:08:27 +0100 Subject: [PATCH 4/5] Throttle active selection states in selection stories --- apps/storybook/src/Selection.stories.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/storybook/src/Selection.stories.tsx b/apps/storybook/src/Selection.stories.tsx index fcceba38d..405cbc2a9 100644 --- a/apps/storybook/src/Selection.stories.tsx +++ b/apps/storybook/src/Selection.stories.tsx @@ -9,6 +9,7 @@ import { Zoom, ResetZoomButton, } from '@h5web/lib'; +import { useThrottledState } from '@react-hookz/web'; import type { Meta, Story } from '@storybook/react'; import { format } from 'd3-format'; import { useState } from 'react'; @@ -45,7 +46,10 @@ const Template: Story = (args) => { const { selectionType, selectionModifierKey, panModifierKey, ...svgProps } = args; - const [activeSelection, setActiveSelection] = useState(); + const [activeSelection, setActiveSelection] = useThrottledState< + Selection | undefined + >(undefined, 50); + const SelectionComponent = SELECTION_COMPONENTS[selectionType]; if (selectionModifierKey === panModifierKey) { @@ -215,7 +219,10 @@ export const AxialSelection: Story = (args) => { ...svgProps } = args; - const [activeSelection, setActiveSelection] = useState(); + const [activeSelection, setActiveSelection] = useThrottledState< + Selection | undefined + >(undefined, 50); + const SelectionComponent = SELECTION_COMPONENTS[selectionType]; if (selectionModifierKey === panModifierKey) { From d913b7bb4727b28beb3994d0ab3142d61fd77539 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Fri, 9 Dec 2022 10:32:13 +0100 Subject: [PATCH 5/5] Let consumers not worry about memoising `SelectionTool` callbacks --- .../lib/src/interactions/SelectionTool.tsx | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/lib/src/interactions/SelectionTool.tsx b/packages/lib/src/interactions/SelectionTool.tsx index 2ae8504af..ca4b73450 100644 --- a/packages/lib/src/interactions/SelectionTool.tsx +++ b/packages/lib/src/interactions/SelectionTool.tsx @@ -1,5 +1,5 @@ import { assertDefined } from '@h5web/shared'; -import { useKeyboardEvent, useRafState } from '@react-hookz/web'; +import { useKeyboardEvent, useRafState, useSyncedRef } from '@react-hookz/web'; import { useThree } from '@react-three/fiber'; import type { ReactNode } from 'react'; import { useRef } from 'react'; @@ -29,13 +29,19 @@ function SelectionTool(props: Props) { id = 'Selection', modifierKey, disabled, - transformSelection, + transformSelection = (selection) => selection, onSelectionStart, onSelectionChange, onSelectionEnd, children, } = props; + // Wrap callbacks in up-to-date but stable refs so consumers don't have to memoise them + const transformSelectionRef = useSyncedRef(transformSelection); + const onSelectionStartRef = useSyncedRef(onSelectionStart); + const onSelectionChangeRef = useSyncedRef(onSelectionChange); + const onSelectionEndRef = useSyncedRef(onSelectionEnd); + const camera = useThree((state) => state.camera); const { worldToData } = useVisCanvasContext(); @@ -59,16 +65,12 @@ function SelectionTool(props: Props) { const { worldPt: worldEnd } = evt; const boundWorldEnd = boundWorldPointToFOV(worldEnd, camera); - const newSelection: Selection = { + return transformSelectionRef.current({ world: [worldStart, boundWorldEnd], data: [dataStart, worldToData(boundWorldEnd)], - }; - - return transformSelection - ? transformSelection(newSelection) - : newSelection; + }); }, - [camera, transformSelection, worldToData] + [camera, transformSelectionRef, worldToData] ); const onPointerDown = useCallback( @@ -82,9 +84,9 @@ function SelectionTool(props: Props) { (target as Element).setPointerCapture(pointerId); startEvtRef.current = evt; - onSelectionStart?.(evt); + onSelectionStartRef.current?.(evt); }, - [onSelectionStart, shouldInteract] + [onSelectionStartRef, shouldInteract] ); const onPointerMove = useCallback( @@ -106,14 +108,14 @@ function SelectionTool(props: Props) { const { target, pointerId } = sourceEvent; (target as Element).releasePointerCapture(pointerId); - if (onSelectionEnd && shouldInteract(sourceEvent)) { - onSelectionEnd(computeSelection(evt)); + if (onSelectionEndRef.current && shouldInteract(sourceEvent)) { + onSelectionEndRef.current(computeSelection(evt)); } startEvtRef.current = undefined; setSelection(undefined); }, - [setSelection, onSelectionEnd, shouldInteract, computeSelection] + [onSelectionEndRef, setSelection, shouldInteract, computeSelection] ); useCanvasEvents({ onPointerDown, onPointerMove, onPointerUp }); @@ -129,10 +131,10 @@ function SelectionTool(props: Props) { ); useEffect(() => { - if (onSelectionChange && selection && isModifierKeyPressed) { - onSelectionChange(selection); + if (selection && isModifierKeyPressed) { + onSelectionChangeRef.current?.(selection); } - }, [selection, isModifierKeyPressed, onSelectionChange]); + }, [selection, isModifierKeyPressed, onSelectionChangeRef]); if (!selection || !isModifierKeyPressed) { return null;