Merge pull request #287 from allen-cell-animated/fix/multi-state-update

Use reducers for viewer state

src/aics-image-viewer/components/App/index.tsx

@@ -157,6 +157,32 @@ const axisToLoaderPriority: Record<AxisName | "t", PrefetchDirection> = {
   x: PrefetchDirection.X_PLUS,
 };
 
+const initializeOneChannelSetting = (
+  channel: string,
+  index: number,
+  defaultColor: ColorArray,
+  viewerChannelSettings?: ViewerChannelSettings,
+  defaultChannelState = DEFAULT_CHANNEL_STATE
+): ChannelState => {
+  let initSettings = {} as Partial<ViewerChannelSetting>;
+  if (viewerChannelSettings) {
+    // search for channel in settings using groups, names and match values
+    initSettings = findFirstChannelMatch(channel, index, viewerChannelSettings) ?? {};
+  }
+
+  return {
+    name: initSettings.name ?? channel ?? "Channel " + index,
+    volumeEnabled: initSettings.enabled ?? defaultChannelState.volumeEnabled,
+    isosurfaceEnabled: initSettings.surfaceEnabled ?? defaultChannelState.isosurfaceEnabled,
+    colorizeEnabled: initSettings.colorizeEnabled ?? defaultChannelState.colorizeEnabled,
+    colorizeAlpha: initSettings.colorizeAlpha ?? defaultChannelState.colorizeAlpha,
+    isovalue: initSettings.isovalue ?? defaultChannelState.isovalue,
+    opacity: initSettings.surfaceOpacity ?? defaultChannelState.opacity,
+    color: colorHexToArray(initSettings.color ?? "") ?? defaultColor,
+    controlPoints: defaultChannelState.controlPoints,
+  };
+};
+
 const setIndicatorPositions = (view3d: View3d, panelOpen: boolean, hasTime: boolean): void => {
   const CLIPPING_PANEL_HEIGHT = 150;
   // Move scale bars this far to the left when showing time series, to make room for timestep indicator
@@ -293,37 +319,6 @@ const App: React.FC<AppProps> = (props) => {
     }
   };
 
-  // TODO: Refactor this out of App index?
-  const initializeOneChannelSetting = (
-    aimg: Volume | null,
-    channel: string,
-    index: number,
-    defaultColor: ColorArray,
-    viewerChannelSettings?: ViewerChannelSettings,
-    defaultChannelState = DEFAULT_CHANNEL_STATE
-  ): ChannelState => {
-    // note that this modifies aimg also
-    const newControlPoints = aimg ? initializeLut(aimg, index) : undefined;
-
-    let initSettings = {} as Partial<ViewerChannelSetting>;
-    if (viewerChannelSettings) {
-      // search for channel in settings using groups, names and match values
-      initSettings = findFirstChannelMatch(channel, index, viewerChannelSettings) ?? {};
-    }
-
-    return {
-      name: initSettings.name ?? channel ?? "Channel " + index,
-      volumeEnabled: initSettings.enabled ?? defaultChannelState.volumeEnabled,
-      isosurfaceEnabled: initSettings.surfaceEnabled ?? defaultChannelState.isosurfaceEnabled,
-      colorizeEnabled: initSettings.colorizeEnabled ?? defaultChannelState.colorizeEnabled,
-      colorizeAlpha: initSettings.colorizeAlpha ?? defaultChannelState.colorizeAlpha,
-      isovalue: initSettings.isovalue ?? defaultChannelState.isovalue,
-      opacity: initSettings.surfaceOpacity ?? defaultChannelState.opacity,
-      color: colorHexToArray(initSettings.color ?? "") ?? defaultColor,
-      controlPoints: newControlPoints ?? defaultChannelState.controlPoints,
-    };
-  };
-
   const setChannelStateForNewImage = (channelNames: string[]): ChannelState[] | undefined => {
     const grouping = makeChannelIndexGrouping(channelNames, props.viewerChannelSettings);
     setChannelGroupedByType(grouping);
@@ -335,7 +330,7 @@ const App: React.FC<AppProps> = (props) => {
 
     const newChannelSettings = channelNames.map((channel, index) => {
       const color = (INIT_COLORS[index] ? INIT_COLORS[index].slice() : [226, 205, 179]) as ColorArray;
-      return initializeOneChannelSetting(null, channel, index, color, props.viewerChannelSettings);
+      return initializeOneChannelSetting(channel, index, color, props.viewerChannelSettings);
     });
     setChannelSettings(newChannelSettings);
     return newChannelSettings;

src/aics-image-viewer/components/ChannelsWidget.tsx

@@ -11,12 +11,7 @@ import type { IsosurfaceFormat } from "../shared/types";
 import ChannelsWidgetRow from "./ChannelsWidgetRow";
 import SharedCheckBox from "./shared/SharedCheckBox";
 import { connectToViewerState } from "./ViewerStateProvider";
-import type {
-  ChannelSettingUpdater,
-  ChannelState,
-  ChannelStateKey,
-  MultipleChannelSettingsUpdater,
-} from "./ViewerStateProvider/types";
+import type { ChannelSettingUpdater, ChannelState, ChannelStateKey } from "./ViewerStateProvider/types";
 
 export type ChannelsWidgetProps = {
   // From parent
@@ -33,14 +28,13 @@ export type ChannelsWidgetProps = {
   // From viewer state
   channelSettings: ChannelState[];
   changeChannelSetting: ChannelSettingUpdater;
-  changeMultipleChannelSettings: MultipleChannelSettingsUpdater;
 };
 
 const ChannelsWidget: React.FC<ChannelsWidgetProps> = (props: ChannelsWidgetProps) => {
   const { channelGroupedByType, channelSettings, channelDataChannels, filterFunc, viewerChannelSettings } = props;
 
   const createCheckboxHandler = (key: ChannelStateKey, value: boolean) => (channelArray: number[]) => {
-    props.changeMultipleChannelSettings(channelArray, key, value);
+    props.changeChannelSetting(channelArray, key, value);
   };
 
   const showVolumes = createCheckboxHandler("volumeEnabled", true);
@@ -133,8 +127,4 @@ const ChannelsWidget: React.FC<ChannelsWidgetProps> = (props: ChannelsWidgetProp
   return <Collapse bordered={false} defaultActiveKey={firstKey} items={rows} collapsible="icon" />;
 };
 
-export default connectToViewerState(ChannelsWidget, [
-  "channelSettings",
-  "changeChannelSetting",
-  "changeMultipleChannelSettings",
-]);
+export default connectToViewerState(ChannelsWidget, ["channelSettings", "changeChannelSetting"]);

src/aics-image-viewer/components/ViewerStateProvider/index.tsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useContext, useMemo, useState, useRef } from "react";
+import React, { useCallback, useContext, useMemo, useReducer, useRef } from "react";
 
 import type {
   ViewerStateContextType,
@@ -7,7 +7,6 @@ import type {
   ViewerSettingUpdater,
   ChannelSettingUpdater,
   ChannelState,
-  MultipleChannelSettingsUpdater,
   PartialIfObject,
 } from "./types";
 import { ImageType, RenderMode, ViewMode } from "../../shared/enums";
@@ -70,23 +69,14 @@ const VIEWER_SETTINGS_CHANGE_HANDLERS: ViewerSettingChangeHandlers = {
   }),
 };
 
-const extractViewerSettings = (context: ViewerStateContextType): ViewerState => {
-  const {
-    channelSettings: _channelSettings,
-    changeViewerSetting: _changeViewerSetting,
-    changeChannelSetting: _changeChannelSetting,
-    changeMultipleChannelSettings: _changeMultipleChannelSettings,
-    applyColorPresets: _applyColorPresets,
-    ...settings
-  } = context;
-  return settings;
+type ViewerStateAction<K extends keyof ViewerState> = {
+  key: K;
+  value: PartialIfObject<ViewerState[K]>;
 };
 
-/** Changes a key in a given `ViewerState` object, keeping the object in a valid state and applying partial values */
-const applyChangeToViewerSettings = <K extends keyof ViewerState>(
+const viewerSettingsReducer = <K extends keyof ViewerState>(
   viewerSettings: ViewerState,
-  key: K,
-  value: PartialIfObject<ViewerState[K]>
+  { key, value }: ViewerStateAction<K>
 ): ViewerState => {
   const changeHandler = VIEWER_SETTINGS_CHANGE_HANDLERS[key];
 
@@ -105,6 +95,55 @@ const applyChangeToViewerSettings = <K extends keyof ViewerState>(
   }
 };
 
+/** Utility type to explicitly assert that one or more properties will *not* be defined on an object */
+type WithExplicitlyUndefined<K extends keyof any, T> = T & { [key in K]?: never };
+
+/** Set channel setting `key` on one or more channels specified by `index` to value `value`. */
+type ChannelSettingUniformUpdateAction<K extends keyof ChannelState> = {
+  index: number | number[];
+  key: K;
+  value: ChannelState[K];
+};
+/** Set the values of channel setting `key` for all channels from an array of values ordered by channel index */
+type ChannelSettingArrayUpdateAction<K extends keyof ChannelState> = {
+  key: K;
+  value: ChannelState[K][];
+};
+/** Initialize list of channel states */
+type ChannelSettingInitAction = {
+  value: ChannelState[];
+};
+
+type ChannelStateAction<K extends keyof ChannelState> =
+  | ChannelSettingUniformUpdateAction<K>
+  | WithExplicitlyUndefined<"index", ChannelSettingArrayUpdateAction<K>>
+  | WithExplicitlyUndefined<"index" | "key", ChannelSettingInitAction>;
+
+const channelSettingsReducer = <K extends keyof ChannelState>(
+  channelSettings: ChannelState[],
+  { index, key, value }: ChannelStateAction<K>
+): ChannelState[] => {
+  if (key === undefined) {
+    // ChannelSettingInitAction
+    return value as ChannelState[];
+  } else if (index === undefined) {
+    // ChannelSettingArrayUpdateAction
+    return channelSettings.map((channel, idx) => {
+      return value[idx] ? { ...channel, [key]: value[idx] } : channel;
+    });
+  } else if (Array.isArray(index)) {
+    // ChannelSettingUniformUpdateAction on potentially multiple channels
+    return channelSettings.map((channel, idx) => (index.includes(idx) ? { ...channel, [key]: value } : channel));
+  } else {
+    // ChannelSettingUniformUpdateAction on a single channel
+    const newSettings = channelSettings.slice();
+    if (index >= 0 && index < channelSettings.length) {
+      newSettings[index] = { ...newSettings[index], [key]: value };
+    }
+    return newSettings;
+  }
+};
+
 const nullfn = (): void => {};
 
 const DEFAULT_VIEWER_CONTEXT: ViewerStateContextType = {
@@ -113,7 +152,6 @@ const DEFAULT_VIEWER_CONTEXT: ViewerStateContextType = {
   changeViewerSetting: nullfn,
   setChannelSettings: nullfn,
   changeChannelSetting: nullfn,
-  changeMultipleChannelSettings: nullfn,
   applyColorPresets: nullfn,
 };
 
@@ -128,54 +166,34 @@ export const ViewerStateContext = React.createContext<{ ref: ContextRefType }>(D
 
 /** Provides a central store for the state of the viewer, and the methods to update it. */
 const ViewerStateProvider: React.FC<{ viewerSettings?: Partial<ViewerState> }> = (props) => {
-  const [viewerSettings, setViewerSettings] = useState<ViewerState>({ ...DEFAULT_VIEWER_SETTINGS });
-  const [channelSettings, setChannelSettings] = useState<ChannelState[]>([]);
+  const [viewerSettings, viewerDispatch] = useReducer(viewerSettingsReducer, { ...DEFAULT_VIEWER_SETTINGS });
+  const [channelSettings, channelDispatch] = useReducer(channelSettingsReducer, []);
   // Provide viewer state via a ref, so that closures that run asynchronously can capture the ref instead of the
   // specific values they need and always have the most up-to-date state.
   const ref = useRef(DEFAULT_VIEWER_CONTEXT);
 
-  // Below callbacks get no dependencies since we're accessing state via the ref
-
-  const changeViewerSetting = useCallback<ViewerSettingUpdater>((key, value) => {
-    const currentSettings = extractViewerSettings(ref.current);
-    setViewerSettings(applyChangeToViewerSettings(currentSettings, key, value));
-  }, []);
+  const changeViewerSetting = useCallback<ViewerSettingUpdater>((key, value) => viewerDispatch({ key, value }), []);
 
   const changeChannelSetting = useCallback<ChannelSettingUpdater>((index, key, value) => {
-    const newChannelSettings = ref.current.channelSettings.slice();
-    newChannelSettings[index] = { ...newChannelSettings[index], [key]: value };
-    setChannelSettings(newChannelSettings);
+    channelDispatch({ index, key, value });
   }, []);
 
-  const changeMultipleChannelSettings = useCallback<MultipleChannelSettingsUpdater>((indices, key, value) => {
-    const newChannelSettings = ref.current.channelSettings.map((settings, idx) =>
-      indices.includes(idx) ? { ...settings, [key]: value } : settings
-    );
-    setChannelSettings(newChannelSettings);
-  }, []);
+  const applyColorPresets = useCallback((value: ColorArray[]): void => channelDispatch({ key: "color", value }), []);
 
-  const applyColorPresets = useCallback((presets: ColorArray[]): void => {
-    const newChannelSettings = ref.current.channelSettings.map((channel, idx) =>
-      presets[idx] ? { ...channel, color: presets[idx] } : channel
-    );
-    setChannelSettings(newChannelSettings);
-  }, []);
+  const setChannelSettings = useCallback((channels: ChannelState[]) => channelDispatch({ value: channels }), []);
 
   // Sync viewer settings prop with state
   // React docs seem to be fine with syncing state with props directly in the render function, but that caused an
   // infinite render loop, so now it's in a `useMemo`:
   // https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
   useMemo(() => {
-    let newSettings = { ...viewerSettings };
     if (props.viewerSettings) {
       for (const key of Object.keys(props.viewerSettings) as (keyof ViewerState)[]) {
-        if (newSettings[key] !== props.viewerSettings[key]) {
-          // Update viewer settings one at a time to allow change handlers to keep state valid
-          newSettings = applyChangeToViewerSettings(newSettings, key, props.viewerSettings[key] as any);
+        if (viewerSettings[key] !== props.viewerSettings[key]) {
+          changeViewerSetting(key, props.viewerSettings[key] as any);
         }
       }
     }
-    setViewerSettings(newSettings);
   }, [props.viewerSettings]);
 
   const context = useMemo(() => {
@@ -185,7 +203,6 @@ const ViewerStateProvider: React.FC<{ viewerSettings?: Partial<ViewerState> }> =
       changeViewerSetting,
       setChannelSettings,
       changeChannelSetting,
-      changeMultipleChannelSettings,
       applyColorPresets,
     };
 

src/aics-image-viewer/components/ViewerStateProvider/types.ts

@@ -66,18 +66,16 @@ export interface ChannelState {
 }
 
 export type ChannelStateKey = keyof ChannelState;
-export type ChannelSettingUpdater = <K extends ChannelStateKey>(index: number, key: K, value: ChannelState[K]) => void;
-export type MultipleChannelSettingsUpdater = <K extends ChannelStateKey>(
-  indices: number[],
+export type ChannelSettingUpdater = <K extends ChannelStateKey>(
+  index: number | number[],
   key: K,
   value: ChannelState[K]
 ) => void;
 
 export type ViewerStateContextType = ViewerState & {
   channelSettings: ChannelState[];
   changeViewerSetting: ViewerSettingUpdater;
-  setChannelSettings: React.Dispatch<React.SetStateAction<ChannelState[]>>;
   changeChannelSetting: ChannelSettingUpdater;
-  changeMultipleChannelSettings: MultipleChannelSettingsUpdater;
+  setChannelSettings: (settings: ChannelState[]) => void;
   applyColorPresets: (presets: ColorArray[]) => void;
 };