Merge pull request #1317 from silx-kit/fix-selection-tool

Fix infinite render loop in axial selection tool stories

apps/storybook/src/AxialSelectToZoom.stories.tsx

@@ -20,6 +20,7 @@ import { useMockData } from './hooks';
 const { oneD, twoD } = mockValues;
 
 const Template: Story<AxialSelectToZoomProps> = (args) => {
+  const { modifierKey } = args;
   const domain = useDomain(oneD);
   assertDefined(domain);
 
@@ -28,7 +29,7 @@ const Template: Story<AxialSelectToZoomProps> = (args) => {
       abscissaConfig={{ visDomain: [0, oneD.length], showGrid: true }}
       ordinateConfig={{ visDomain: domain, showGrid: true }}
     >
-      <Pan />
+      <Pan modifierKey={modifierKey?.length === 0 ? 'Control' : undefined} />
       <Zoom />
       <AxialSelectToZoom {...args} />
       <ResetZoomButton />
@@ -78,7 +79,7 @@ export const DisabledInsideEqualAspectCanvas: Story<AxialSelectToZoomProps> = (
       ordinateConfig={{ visDomain: [0, 20], showGrid: true }}
       aspect="equal"
     >
-      <Pan />
+      <Pan modifierKey="Control" />
       <Zoom />
       <AxialSelectToZoom {...args} />
       <ResetZoomButton />
@@ -93,6 +94,11 @@ export const DisabledInsideEqualAspectCanvas: Story<AxialSelectToZoomProps> = (
   );
 };
 
+DisabledInsideEqualAspectCanvas.argTypes = {
+  modifierKey: { control: false },
+  disabled: { control: false },
+};
+
 export default {
   title: 'Building Blocks/Interactions/AxialSelectToZoom',
   component: AxialSelectToZoom,

apps/storybook/src/DefaultInteractions.stories.tsx

@@ -1,35 +1,63 @@
-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<DefaultInteractionsConfig> = (
   args
 ) => {
+  const { values, domain } = useMockData(twoD, [20, 41]);
+
   return (
     <VisCanvas
-      abscissaConfig={{ visDomain: [0, 10], showGrid: true }}
-      ordinateConfig={{ visDomain: [0, 10], showGrid: true }}
+      abscissaConfig={{ visDomain: [0, values.shape[1]], showGrid: true }}
+      ordinateConfig={{ visDomain: [0, values.shape[0]], showGrid: true }}
       aspect="auto"
     >
       <DefaultInteractions {...args} />
       <ResetZoomButton />
+
+      <HeatmapMesh
+        values={values}
+        domain={domain}
+        colorMap="Viridis"
+        scaleType={ScaleType.Linear}
+      />
     </VisCanvas>
   );
 };
 
 export const InsideEqualAspectCanvas: Story<DefaultInteractionsConfig> = (
   args
 ) => {
+  const { values, domain } = useMockData(twoD, [20, 41]);
+
   return (
     <VisCanvas
-      abscissaConfig={{ visDomain: [0, 10], showGrid: true }}
-      ordinateConfig={{ visDomain: [0, 10], showGrid: true }}
+      abscissaConfig={{ visDomain: [0, values.shape[1]], showGrid: true }}
+      ordinateConfig={{ visDomain: [0, values.shape[0]], showGrid: true }}
       aspect="equal"
     >
       <DefaultInteractions {...args} />
       <ResetZoomButton />
+
+      <HeatmapMesh
+        values={values}
+        domain={domain}
+        colorMap="Viridis"
+        scaleType={ScaleType.Linear}
+      />
     </VisCanvas>
   );
 };

apps/storybook/src/SelectToZoom.stories.tsx

@@ -17,13 +17,14 @@ const { twoD } = mockValues;
 
 const Template: Story<SelectToZoomProps> = (args) => {
   const { values, domain } = useMockData(twoD, [20, 41]);
+  const { modifierKey } = args;
 
   return (
     <VisCanvas
       abscissaConfig={{ visDomain: [0, values.shape[1]], showGrid: true }}
       ordinateConfig={{ visDomain: [0, values.shape[0]], showGrid: true }}
     >
-      <Pan />
+      <Pan modifierKey={modifierKey?.length === 0 ? 'Control' : undefined} />
       <Zoom />
       <SelectToZoom {...args} />
       <ResetZoomButton />
@@ -42,14 +43,15 @@ export const InsideAutoAspectCanvas = Template.bind({});
 
 export const InsideEqualAspectCanvas: Story<SelectToZoomProps> = (args) => {
   const { values, domain } = useMockData(twoD, [20, 41]);
+  const { modifierKey } = args;
 
   return (
     <VisCanvas
       abscissaConfig={{ visDomain: [0, values.shape[1]], showGrid: true }}
       ordinateConfig={{ visDomain: [0, values.shape[0]], showGrid: true }}
       aspect="equal"
     >
-      <Pan />
+      <Pan modifierKey={modifierKey?.length === 0 ? 'Control' : undefined} />
       <Zoom />
       <SelectToZoom {...args} />
       <ResetZoomButton />

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<TemplateProps> = (args) => {
   const { selectionType, selectionModifierKey, panModifierKey, ...svgProps } =
     args;
 
-  const [activeSelection, setActiveSelection] = useState<Selection>();
+  const [activeSelection, setActiveSelection] = useThrottledState<
+    Selection | undefined
+  >(undefined, 50);
+
   const SelectionComponent = SELECTION_COMPONENTS[selectionType];
 
   if (selectionModifierKey === panModifierKey) {
@@ -170,9 +174,9 @@ export const Persisted: Story<TemplateProps> = (args) => {
       <ResetZoomButton />
 
       <SelectionTool
+        modifierKey={selectionModifierKey}
         onSelectionStart={() => setPersistedSelection(undefined)}
         onSelectionEnd={setPersistedSelection}
-        modifierKey={selectionModifierKey}
       >
         {({ data: [dataStart, dataEnd] }) =>
           !persistedSelection && (
@@ -192,8 +196,8 @@ export const Persisted: Story<TemplateProps> = (args) => {
 };
 Persisted.args = {
   selectionType: 'rectangle',
-  selectionModifierKey: 'Control',
-  panModifierKey: undefined,
+  selectionModifierKey: undefined,
+  panModifierKey: 'Control',
 };
 Persisted.argTypes = {
   selectionModifierKey: {
@@ -215,7 +219,10 @@ export const AxialSelection: Story<TemplateProps & { axis: Axis }> = (args) => {
     ...svgProps
   } = args;
 
-  const [activeSelection, setActiveSelection] = useState<Selection>();
+  const [activeSelection, setActiveSelection] = useThrottledState<
+    Selection | undefined
+  >(undefined, 50);
+
   const SelectionComponent = SELECTION_COMPONENTS[selectionType];
 
   if (selectionModifierKey === panModifierKey) {

packages/lib/src/interactions/SelectionTool.tsx

@@ -1,8 +1,9 @@
 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 { useCallback, useEffect, useState } from 'react';
+import { useRef } from 'react';
+import { useCallback, useEffect } from 'react';
 
 import { useVisCanvasContext } from '../vis/shared/VisCanvasProvider';
 import {
@@ -28,17 +29,23 @@ function SelectionTool(props: Props) {
     id = 'Selection',
     modifierKey,
     disabled,
-    transformSelection: customTransformSelection,
+    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();
 
-  const [startEvt, setStartEvt] = useState<CanvasEvent<PointerEvent>>();
+  const startEvtRef = useRef<CanvasEvent<PointerEvent>>();
   const [selection, setSelection] = useRafState<Selection>();
 
   const modifierKeys = getModifierKeyArray(modifierKey);
@@ -52,25 +59,18 @@ function SelectionTool(props: Props) {
 
   const computeSelection = useCallback(
     (evt: CanvasEvent<PointerEvent>): 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);
 
-      return {
+      return transformSelectionRef.current({
         world: [worldStart, boundWorldEnd],
         data: [dataStart, worldToData(boundWorldEnd)],
-      };
+      });
     },
-    [camera, startEvt, worldToData]
-  );
-
-  const transformSelection = useCallback(
-    (selec: Selection): Selection => {
-      return customTransformSelection ? customTransformSelection(selec) : selec;
-    },
-    [customTransformSelection]
+    [camera, transformSelectionRef, worldToData]
   );
 
   const onPointerDown = useCallback(
@@ -82,75 +82,65 @@ function SelectionTool(props: Props) {
 
       const { target, pointerId } = sourceEvent;
       (target as Element).setPointerCapture(pointerId);
+      startEvtRef.current = evt;
 
-      setStartEvt(evt);
-
-      if (onSelectionStart) {
-        onSelectionStart(evt);
-      }
+      onSelectionStartRef.current?.(evt);
     },
-    [onSelectionStart, shouldInteract]
+    [onSelectionStartRef, shouldInteract]
   );
 
   const onPointerMove = useCallback(
     (evt: CanvasEvent<PointerEvent>) => {
-      if (startEvt) {
+      if (startEvtRef.current) {
         setSelection(computeSelection(evt));
       }
     },
-    [startEvt, setSelection, computeSelection]
+    [setSelection, computeSelection]
   );
 
   const onPointerUp = useCallback(
     (evt: CanvasEvent<PointerEvent>) => {
-      if (!startEvt) {
+      if (!startEvtRef.current) {
         return;
       }
 
       const { sourceEvent } = evt;
       const { target, pointerId } = sourceEvent;
       (target as Element).releasePointerCapture(pointerId);
 
-      setStartEvt(undefined);
-      setSelection(undefined);
-
-      if (onSelectionEnd && shouldInteract(sourceEvent)) {
-        onSelectionEnd(transformSelection(computeSelection(evt)));
+      if (onSelectionEndRef.current && shouldInteract(sourceEvent)) {
+        onSelectionEndRef.current(computeSelection(evt));
       }
+
+      startEvtRef.current = undefined;
+      setSelection(undefined);
     },
-    [
-      startEvt,
-      setSelection,
-      onSelectionEnd,
-      shouldInteract,
-      transformSelection,
-      computeSelection,
-    ]
+    [onSelectionEndRef, setSelection, shouldInteract, computeSelection]
   );
 
   useCanvasEvents({ onPointerDown, onPointerMove, onPointerUp });
 
   useKeyboardEvent(
     'Escape',
     () => {
-      setStartEvt(undefined);
+      startEvtRef.current = undefined;
       setSelection(undefined);
     },
     [],
     { event: 'keydown' }
   );
 
   useEffect(() => {
-    if (onSelectionChange && selection && isModifierKeyPressed) {
-      onSelectionChange(transformSelection(selection));
+    if (selection && isModifierKeyPressed) {
+      onSelectionChangeRef.current?.(selection);
     }
-  }, [selection, isModifierKeyPressed, onSelectionChange, transformSelection]);
+  }, [selection, isModifierKeyPressed, onSelectionChangeRef]);
 
   if (!selection || !isModifierKeyPressed) {
     return null;
   }
 
-  return <>{children(transformSelection(selection))}</>;
+  return <>{children(selection)}</>;
 }
 
 export type { Props as SelectionProps };