From 3dece871ea504ffd6b9bc391b41dc8a805864c0c Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Mon, 4 Nov 2024 15:03:36 -0800 Subject: [PATCH 01/13] add comment explaining the purpose behind compareAgentTrees (#608) --- src/util/index.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/index.ts b/src/util/index.ts index d5b950a5..4ba05790 100644 --- a/src/util/index.ts +++ b/src/util/index.ts @@ -107,6 +107,14 @@ export const roundToTimeStepPrecision = ( return Math.round(input * multiplier) / multiplier; }; +/** +Compare two instaces of UIDisplayData to see if they have the same agents +and display states. +This data structure is used to store different color settings. +We don't want to ever try and apply the color settings from one trajectory +to another, even if by chance they shared the same file name, or other +metadata. +*/ export const compareAgentTrees = (a: UIDisplayData, b: UIDisplayData) => { if (a.length !== b.length) { return false; From 482e5a0d9a7fd7033199dbb388e2a1eca8ac2e6c Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Tue, 12 Nov 2024 08:31:19 -0800 Subject: [PATCH 02/13] remove erroneous overflow bar (#611) --- src/components/SideBarContents/style.css | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/SideBarContents/style.css b/src/components/SideBarContents/style.css index 8f286278..62a43230 100644 --- a/src/components/SideBarContents/style.css +++ b/src/components/SideBarContents/style.css @@ -2,7 +2,6 @@ display: flex; flex-direction: column; height: 100%; - overflow-y: auto; } .container label { From 88eb4d675e4689556bf0156ced9c28c6238f019b Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Wed, 13 Nov 2024 15:31:44 -0800 Subject: [PATCH 03/13] keyboard navigation : feature/kb open dropdowns (#595) * prototype focus styling for header elements * use custom dropdown wrapper to keep stylesheets dry * focus style on AICS logo button * updated focus and hover stylings for header buttons and dropdown items * remove commented code * add custom hook for focus mangement in dropdown menus * move dropdown focus concerns from hook into CustomDropdown * use same mouseenter handler for button and menu in CustomDropdown * define button click handler outside props in CustomDropdown * remove nested conditional in key handler * preserve focus and keyboard opening of dropdowns after quick hover event --- src/components/CustomDropdown/index.tsx | 107 +++++++++++++++++++++++- src/components/NavButton/index.tsx | 62 +++++++------- src/constants/index.ts | 1 + src/constants/interfaces.ts | 6 ++ 4 files changed, 146 insertions(+), 30 deletions(-) diff --git a/src/components/CustomDropdown/index.tsx b/src/components/CustomDropdown/index.tsx index f5ab5fbf..4dbe58e9 100644 --- a/src/components/CustomDropdown/index.tsx +++ b/src/components/CustomDropdown/index.tsx @@ -1,6 +1,13 @@ -import React, { ReactNode } from "react"; +import React, { + KeyboardEventHandler, + ReactNode, + useEffect, + useRef, + useState, +} from "react"; import { Dropdown, DropDownProps, MenuProps } from "antd"; -import { ButtonClass } from "../../constants/interfaces"; +import { ButtonClass, DropdownState } from "../../constants/interfaces"; +import { DROPDOWN_HOVER_DELAY } from "../../constants"; import NavButton from "../NavButton"; import styles from "./style.css"; @@ -22,16 +29,112 @@ const CustomDropdown: React.FC = ({ placement, disabled, }) => { + const [dropdownState, setDropdownState] = useState( + DropdownState.CLOSED + ); + + const triggerRef = useRef(null); + const dropdownRef = useRef(null); + const closeTimeoutRef = useRef(null); + + /** + * Prevents the menu wrapper from capturing focus, + * this prevents losing focus to the body + * when "Escape" is pressed. + */ + useEffect(() => { + const element = dropdownRef.current; + if (element) { + const menuElement = element.querySelector( + ".ant-dropdown-menu" + ) as HTMLElement; + if (menuElement) { + if (dropdownState === DropdownState.FORCED_OPEN) { + menuElement.setAttribute("tabIndex", "-1"); + } else if (dropdownState === DropdownState.CLOSED) { + menuElement.setAttribute("tabIndex", "0"); + } + } + } + }, [dropdownRef, dropdownState]); + + /** + * Manually handling keydown and hover behavior because + * our focus management overrides the defaults of the antd components. + */ + const openTriggers = new Set(["Enter", " ", "ArrowDown"]); + + const handleKeyDown: KeyboardEventHandler = (event) => { + if (event.key === "Escape") { + event.preventDefault(); + setDropdownState(DropdownState.CLOSED); + triggerRef.current?.focus(); + } + if ( + openTriggers.has(event.key) && + dropdownState !== DropdownState.FORCED_OPEN + ) { + event.preventDefault(); + if (closeTimeoutRef.current) { + clearTimeout(closeTimeoutRef.current); + } + setDropdownState(DropdownState.FORCED_OPEN); // Opened by keyboard + } + }; + + const handleMouseEnter = () => { + if (dropdownState === DropdownState.CLOSED) { + setDropdownState(DropdownState.OPEN); + } + if (closeTimeoutRef.current) { + clearTimeout(closeTimeoutRef.current); + } + }; + + const handleMouseLeaveWithDelay = () => { + if (dropdownState !== DropdownState.FORCED_OPEN) { + closeTimeoutRef.current = setTimeout(() => { + setDropdownState(DropdownState.CLOSED); + }, DROPDOWN_HOVER_DELAY); + } + }; + + const buttonClickHandler = () => { + setDropdownState( + dropdownState === DropdownState.CLOSED + ? DropdownState.OPEN + : DropdownState.CLOSED + ); + }; + return ( ( +
+ {menu} +
+ )} > } titleText={titleText} icon={icon} buttonType={buttonType} + clickHandler={buttonClickHandler} + onKeyDown={handleKeyDown} + onMouseEnter={handleMouseEnter} + onMouseLeave={handleMouseLeaveWithDelay} /> ); diff --git a/src/components/NavButton/index.tsx b/src/components/NavButton/index.tsx index b6e7fe71..f989bbb2 100644 --- a/src/components/NavButton/index.tsx +++ b/src/components/NavButton/index.tsx @@ -1,7 +1,6 @@ -import React, { ReactNode } from "react"; +import React, { ReactNode, forwardRef } from "react"; import { Button, ButtonProps } from "antd"; import classNames from "classnames"; - import styles from "./style.css"; import { ButtonClass } from "../../constants/interfaces"; @@ -13,32 +12,39 @@ export interface NavButtonProps extends ButtonProps { isDisabled?: boolean; } -const NavButton: React.FC = ({ - className, - titleText, - buttonType = ButtonClass.Action, - icon, - clickHandler, - isDisabled, - ...props -}) => { - // NavButtons default to action button styling, provide secondary or primary to override - const buttonClassNames = classNames( - className, - styles.navButton, - styles[buttonType], - { [styles.disabled]: isDisabled } - ); +const NavButton = forwardRef( + ( + { + className, + titleText, + buttonType = ButtonClass.Action, + icon, + clickHandler, + isDisabled, + ...props + }, + ref + ) => { + const buttonClassNames = classNames( + className, + styles.navButton, + styles[buttonType], + { [styles.disabled]: isDisabled } + ); + + return ( + + ); + } +); - return ( - - ); -}; +NavButton.displayName = "NavButton"; export default NavButton; diff --git a/src/constants/index.ts b/src/constants/index.ts index 273bf714..9e57f406 100644 --- a/src/constants/index.ts +++ b/src/constants/index.ts @@ -69,3 +69,4 @@ export const MAX_CONVERSION_FILE_SIZE = 2e8; // 200 MB export const CONTROLS_MIN_WIDTH = 650; export const CONTROLS_MIN_HEIGHT = 320; export const SCALE_BAR_MIN_WIDTH = 550; +export const DROPDOWN_HOVER_DELAY = 300; diff --git a/src/constants/interfaces.ts b/src/constants/interfaces.ts index 1abd73b2..16e71b44 100644 --- a/src/constants/interfaces.ts +++ b/src/constants/interfaces.ts @@ -92,3 +92,9 @@ export interface ColorChange { agent: SelectionEntry; color: string; } + +export enum DropdownState { + OPEN = "open", + CLOSED = "closed", + FORCED_OPEN = "forced_open", +} From 9fe9f69c7d523ac61463df06cbbba377e3d6a9dc Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Wed, 13 Nov 2024 15:31:59 -0800 Subject: [PATCH 04/13] fix: stop recording icon (#610) * proper classnames to display stop recording icon * correct positioning of record icons --- .../RecordMoviesComponent/index.tsx | 10 ++++--- .../RecordMoviesComponent/style.css | 26 ++++++++++--------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/components/RecordMoviesComponent/index.tsx b/src/components/RecordMoviesComponent/index.tsx index 294e1141..056a5c8d 100644 --- a/src/components/RecordMoviesComponent/index.tsx +++ b/src/components/RecordMoviesComponent/index.tsx @@ -63,14 +63,14 @@ const RecordMovieComponent = (props: RecordMovieComponentProps) => { * In this icon we are stacking glyphs to create multicolor icons via icomoon */ const startRecordingIcon = ( - +
- +
); const activeRecordingIcon = ( @@ -81,7 +81,11 @@ const RecordMovieComponent = (props: RecordMovieComponentProps) => { if (!isRecording) { return startRecordingIcon; } else if (isHovering) { - return "stop-record-icon"; + return classNames( + styles.iconContainer, + "icon-moon", + "stop-record-icon" + ); } else return activeRecordingIcon; }; diff --git a/src/components/RecordMoviesComponent/style.css b/src/components/RecordMoviesComponent/style.css index 660501af..660d5161 100644 --- a/src/components/RecordMoviesComponent/style.css +++ b/src/components/RecordMoviesComponent/style.css @@ -5,6 +5,8 @@ } .icon-container { + display: flex; + align-items: center; height: 100%; width: 100%; } @@ -22,18 +24,18 @@ } @keyframes pulse-red { - 0% { - transform: scale(0.95); - box-shadow: 0 0 0 0 #FF5252B3; - } - 70% { - transform: scale(1); - box-shadow: 0 0 0 10px #FF525200; - } - 100% { - transform: scale(0.95); - box-shadow: 0 0 0 0 #FF525200; - } + 0% { + transform: scale(0.95); + box-shadow: 0 0 0 0 #ff5252b3; + } + 70% { + transform: scale(1); + box-shadow: 0 0 0 10px #ff525200; + } + 100% { + transform: scale(0.95); + box-shadow: 0 0 0 0 #ff525200; + } } .status-container { From 5c070a9484719acf88145da134cdee3fb545a8b2 Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Mon, 18 Nov 2024 12:52:22 -0800 Subject: [PATCH 05/13] Feature/selector org (#606) * add compoundSelectors to redux * add ColorSettings enum and use container specific selector in ModelPanel * add currentColorSettings to selection branch state * rename ColorSetting from plural * fix typo in selection branch spread in getCurrentUIData test * add explanatory comment to compoundSelectors --- src/containers/ModelPanel/index.tsx | 6 +- src/containers/ModelPanel/selectors.test.ts | 75 ++++++++++++++++-- src/containers/ModelPanel/selectors.ts | 26 +++++- .../compoundSelectors.test.ts | 79 +++++++++++++++++++ src/state/compoundSelectors/index.ts | 34 ++++++++ src/state/selection/reducer.ts | 2 + src/state/selection/selectors/basic.ts | 2 + src/state/selection/types.ts | 5 ++ src/state/trajectory/selectors/index.ts | 26 +----- .../trajectory/selectors/selectors.test.ts | 67 +--------------- 10 files changed, 220 insertions(+), 102 deletions(-) create mode 100644 src/state/compoundSelectors/compoundSelectors.test.ts create mode 100644 src/state/compoundSelectors/index.ts diff --git a/src/containers/ModelPanel/index.tsx b/src/containers/ModelPanel/index.tsx index 66b3f57e..b21a2967 100644 --- a/src/containers/ModelPanel/index.tsx +++ b/src/containers/ModelPanel/index.tsx @@ -10,10 +10,7 @@ import { requestTrajectory, changeToNetworkedFile, } from "../../state/trajectory/actions"; -import { - getUiDisplayDataTree, - getIsNetworkedFile, -} from "../../state/trajectory/selectors"; +import { getIsNetworkedFile } from "../../state/trajectory/selectors"; import { AgentRenderingCheckboxMap, ChangeAgentsRenderingStateAction, @@ -44,6 +41,7 @@ import { getSelectAllVisibilityMap, getSelectNoneVisibilityMap, getIsSharedCheckboxIndeterminate, + getUiDisplayDataTree, } from "./selectors"; import styles from "./style.css"; diff --git a/src/containers/ModelPanel/selectors.test.ts b/src/containers/ModelPanel/selectors.test.ts index 8f2acb2c..50146f21 100644 --- a/src/containers/ModelPanel/selectors.test.ts +++ b/src/containers/ModelPanel/selectors.test.ts @@ -1,7 +1,9 @@ +import { initialState, State } from "../../state"; import { getSelectAllVisibilityMap, getSelectNoneVisibilityMap, getIsSharedCheckboxIndeterminate, + getUiDisplayDataTree, } from "./selectors"; const mockUiDisplayData = [ @@ -50,9 +52,8 @@ const mockUiDisplayData = [ describe("ModelPanel selectors", () => { describe("getSelectAllVisibilityMap", () => { it("Returns an agent visibility map with all possible states", () => { - const result = getSelectAllVisibilityMap.resultFunc( - mockUiDisplayData - ); + const result = + getSelectAllVisibilityMap.resultFunc(mockUiDisplayData); const expected = { agentWithChildren1: ["", "state1"], agentWithChildren2: ["", "state1"], @@ -64,9 +65,8 @@ describe("ModelPanel selectors", () => { describe("getSelectNoneVisibilityMap", () => { it("Returns an agent visibility map with none of the possible states", () => { - const result = getSelectNoneVisibilityMap.resultFunc( - mockUiDisplayData - ); + const result = + getSelectNoneVisibilityMap.resultFunc(mockUiDisplayData); const expected = { agentWithChildren1: [], agentWithChildren2: [], @@ -164,4 +164,67 @@ describe("ModelPanel selectors", () => { expect(result).toBe(true); }); }); + describe("getUiDisplayDataTree", () => { + it("returns an empty array if ui display data is empty", () => { + expect(getUiDisplayDataTree(initialState)).toStrictEqual([]); + }); + it("correctly maps agent display info to an array of display data", () => { + const state: State = { + ...initialState, + trajectory: { + ...initialState.trajectory, + defaultUIData: [ + { + name: "agent1", + displayStates: [], + color: "#bbbbbb", + }, + { + name: "agent2", + color: "#aaaaaa", + displayStates: [ + { + name: "state1", + id: "state1_id", + color: "#000000", + }, + { + name: "state2", + id: "state2_id", + color: "#000000", + }, + ], + }, + ], + }, + }; + + const expected = [ + { + title: "agent1", + key: "agent1", + children: [], + color: "#bbbbbb", + }, + { + title: "agent2", + key: "agent2", + color: "#aaaaaa", + children: [ + { + color: "#000000", + label: "state1", + value: "state1_id", + }, + { + color: "#000000", + label: "state2", + value: "state2_id", + }, + ], + }, + ]; + expect(getUiDisplayDataTree(state)).toStrictEqual(expected); + }); + }); }); diff --git a/src/containers/ModelPanel/selectors.ts b/src/containers/ModelPanel/selectors.ts index dd896220..da201768 100644 --- a/src/containers/ModelPanel/selectors.ts +++ b/src/containers/ModelPanel/selectors.ts @@ -2,9 +2,33 @@ import { createSelector } from "reselect"; import { isEmpty } from "lodash"; import { AgentDisplayNode } from "../../components/AgentTree"; -import { getUiDisplayDataTree } from "../../state/trajectory/selectors"; import { getAgentVisibilityMap } from "../../state/selection/selectors"; import { AgentRenderingCheckboxMap } from "../../state/selection/types"; +import { getCurrentUIData } from "../../state/compoundSelectors"; +import { UIDisplayData } from "@aics/simularium-viewer"; + +export const getUiDisplayDataTree = createSelector( + [getCurrentUIData], + (uiDisplayData: UIDisplayData) => { + if (!uiDisplayData.length) { + return []; + } + return uiDisplayData.map((agent) => ({ + title: agent.name, + key: agent.name, + color: agent.color, + children: agent.displayStates.length + ? [ + ...agent.displayStates.map((state) => ({ + label: state.name, + value: state.id, + color: state.color, + })), + ] + : [], + })); + } +); // Returns an agent visibility map that indicates all states should be visible export const getSelectAllVisibilityMap = createSelector( diff --git a/src/state/compoundSelectors/compoundSelectors.test.ts b/src/state/compoundSelectors/compoundSelectors.test.ts new file mode 100644 index 00000000..008ca7ff --- /dev/null +++ b/src/state/compoundSelectors/compoundSelectors.test.ts @@ -0,0 +1,79 @@ +import { getCurrentUIData } from "."; +import { initialState } from ".."; +import { ColorSetting } from "../selection/types"; + +describe("getCurrentUIData", () => { + it("returns empty array if default UI data has not been entered yet", () => { + expect(getCurrentUIData(initialState)).toEqual([]); + 1; + }); + it("returns selectedUIDisplayData if colorSetting is equal to ColorSetting.UserSelected", () => { + expect( + getCurrentUIData({ + ...initialState, + trajectory: { + ...initialState.trajectory, + defaultUIData: [ + { + name: "agent1", + displayStates: [], + color: "#bbbbbb", + }, + ], + }, + selection: { + ...initialState.selection, + currentColorSetting: ColorSetting.UserSelected, + selectedUIDisplayData: [ + { + name: "agent1", + displayStates: [], + color: "#000", + }, + ], + }, + }) + ).toEqual([ + { + name: "agent1", + displayStates: [], + color: "#000", + }, + ]); + }); + + it("returns defaultUIData if colorSetting is euqal to ColorSetting.Default", () => { + expect( + getCurrentUIData({ + ...initialState, + trajectory: { + ...initialState.trajectory, + defaultUIData: [ + { + name: "agent1", + displayStates: [], + color: "#bbbbbb", + }, + ], + }, + selection: { + ...initialState.selection, + currentColorSetting: ColorSetting.Default, + selectedUIDisplayData: [ + { + name: "agent1", + displayStates: [], + color: "#000", + }, + ], + }, + }) + ).toEqual([ + { + name: "agent1", + displayStates: [], + color: "#bbbbbb", + }, + ]); + }); +}); diff --git a/src/state/compoundSelectors/index.ts b/src/state/compoundSelectors/index.ts new file mode 100644 index 00000000..0f9f50d7 --- /dev/null +++ b/src/state/compoundSelectors/index.ts @@ -0,0 +1,34 @@ +import { createSelector } from "reselect"; +import { UIDisplayData } from "@aics/simularium-viewer"; + +import { getDefaultUIDisplayData } from "../trajectory/selectors"; +import { + getCurrentColorSetting, + getSelectedUIDisplayData, +} from "../selection/selectors"; +import { ColorSetting } from "../selection/types"; + +/** + * compoundSelectors are selectors that consume state from multiple branches + * of state, so don't belong in a particular branch's selectors file, + * and are consumed by multiple containers, and so don't belong in a particular + * container's selectors file. + */ + +export const getCurrentUIData = createSelector( + [getCurrentColorSetting, getSelectedUIDisplayData, getDefaultUIDisplayData], + ( + colorSetting: ColorSetting, + sessionData: UIDisplayData, + defaultData: UIDisplayData + ) => { + const fileHasBeenParsed = defaultData.length > 0; + if (!fileHasBeenParsed) { + return []; + } + if (colorSetting === ColorSetting.UserSelected) { + return sessionData; + } + return defaultData; + } +); diff --git a/src/state/selection/reducer.ts b/src/state/selection/reducer.ts index d0b06472..8b502b07 100644 --- a/src/state/selection/reducer.ts +++ b/src/state/selection/reducer.ts @@ -26,6 +26,7 @@ import { SetRecentColorsAction, SetSelectedAgentMetadataAction, SetSelectedUIDisplayDataAction, + ColorSetting, } from "./types"; export const initialState = { @@ -36,6 +37,7 @@ export const initialState = { recentColors: [], selectedAgentMetadata: {}, selectedUIDisplayData: [], + currentColorSetting: ColorSetting.Default, }; const actionToConfigMap: TypeToDescriptionMap = { diff --git a/src/state/selection/selectors/basic.ts b/src/state/selection/selectors/basic.ts index e9b91488..671aab37 100644 --- a/src/state/selection/selectors/basic.ts +++ b/src/state/selection/selectors/basic.ts @@ -13,3 +13,5 @@ export const getSelectedAgentMetadata = (state: State) => state.selection.selectedAgentMetadata; export const getSelectedUIDisplayData = (state: State) => state.selection.selectedUIDisplayData; +export const getCurrentColorSetting = (state: State) => + state.selection.currentColorSetting; diff --git a/src/state/selection/types.ts b/src/state/selection/types.ts index db427384..dc1239bc 100644 --- a/src/state/selection/types.ts +++ b/src/state/selection/types.ts @@ -87,3 +87,8 @@ export interface SetSelectedUIDisplayDataAction { payload: UIDisplayData; type: string; } + +export enum ColorSetting { + UserSelected = "userSelected", + Default = "default", +} diff --git a/src/state/trajectory/selectors/index.ts b/src/state/trajectory/selectors/index.ts index ecd074b2..ea882f4f 100644 --- a/src/state/trajectory/selectors/index.ts +++ b/src/state/trajectory/selectors/index.ts @@ -1,12 +1,11 @@ import { createSelector } from "reselect"; -import { UIDisplayData } from "@aics/simularium-viewer"; import { isNetworkSimFileInterface, LocalSimFile, NetworkedSimFile, } from "../types"; -import { getSimulariumFile, getDefaultUIDisplayData } from "./basic"; +import { getSimulariumFile } from "./basic"; export const getIsNetworkedFile = createSelector( [getSimulariumFile], @@ -18,27 +17,4 @@ export const getIsNetworkedFile = createSelector( } ); -export const getUiDisplayDataTree = createSelector( - [getDefaultUIDisplayData], - (uiDisplayData: UIDisplayData) => { - if (!uiDisplayData.length) { - return []; - } - return uiDisplayData.map((agent) => ({ - title: agent.name, - key: agent.name, - color: agent.color, - children: agent.displayStates.length - ? [ - ...agent.displayStates.map((state) => ({ - label: state.name, - value: state.id, - color: state.color, - })), - ] - : [], - })); - } -); - export * from "./basic"; diff --git a/src/state/trajectory/selectors/selectors.test.ts b/src/state/trajectory/selectors/selectors.test.ts index f236abe7..0fb77378 100644 --- a/src/state/trajectory/selectors/selectors.test.ts +++ b/src/state/trajectory/selectors/selectors.test.ts @@ -1,7 +1,7 @@ import { initialState } from "../../index"; import { State } from "../../types"; -import { getIsNetworkedFile, getUiDisplayDataTree } from "."; +import { getIsNetworkedFile } from "."; describe("trajectory composed selectors", () => { describe("getIsNetworkedFile", () => { @@ -37,69 +37,4 @@ describe("trajectory composed selectors", () => { expect(getIsNetworkedFile(state)).toBe(true); }); }); - - describe("getUiDisplayDataTree", () => { - it("returns an empty array if ui display data is empty", () => { - expect(getUiDisplayDataTree(initialState)).toStrictEqual([]); - }); - it("correctly maps agent display info to an array of display data", () => { - const state: State = { - ...initialState, - trajectory: { - ...initialState.trajectory, - defaultUIData: [ - { - name: "agent1", - displayStates: [], - color: "#bbbbbb", - }, - { - name: "agent2", - color: "#aaaaaa", - displayStates: [ - { - name: "state1", - id: "state1_id", - color: "#000000", - }, - { - name: "state2", - id: "state2_id", - color: "#000000", - }, - ], - }, - ], - }, - }; - - const expected = [ - { - title: "agent1", - key: "agent1", - children: [], - color: "#bbbbbb", - }, - { - title: "agent2", - key: "agent2", - color: "#aaaaaa", - children: [ - { - color: "#000000", - label: "state1", - value: "state1_id", - }, - { - color: "#000000", - label: "state2", - value: "state2_id", - }, - ], - }, - ]; - - expect(getUiDisplayDataTree(state)).toStrictEqual(expected); - }); - }); }); From 27c7c069724755bedbfd9c6d11730dd7def53313 Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Fri, 22 Nov 2024 17:14:29 -0800 Subject: [PATCH 06/13] retrieve stored color data from browser and apply it (#607) * add compoundSelectors to redux * retrieve stored color data from browser and apply it * add ColorSettings enum and use container specific selector in ModelPanel * add currentColorSettings to selection branch state * make applyColorChangeToUiDisplayData a util with tests * fix ids in broken test * rename ColorSetting from plural * fix typo in selection branch spread in getCurrentUIData test * rename ColorSettings to ColorSetting across the board * streamline color setting logics and always retrieve settings after parsing file * remove parsing check from current ui data selector --- src/components/AgentTree/index.tsx | 7 +- src/components/ColorPicker/index.tsx | 7 +- src/containers/ModelPanel/index.tsx | 12 +-- src/containers/ViewerPanel/index.tsx | 6 ++ src/containers/ViewerPanel/selectors.ts | 4 +- src/state/compoundSelectors/index.ts | 11 +-- src/state/selection/actions.ts | 39 ++++++-- src/state/selection/constants.ts | 10 ++- src/state/selection/logics.ts | 99 +++++++++++++-------- src/state/selection/reducer.ts | 17 ++++ src/state/selection/types.ts | 19 ++-- src/state/trajectory/logics.ts | 13 +-- src/util/index.ts | 29 +++++- src/util/test/index.test.ts | 113 +++++++++++++++++++++--- 14 files changed, 287 insertions(+), 99 deletions(-) diff --git a/src/components/AgentTree/index.tsx b/src/components/AgentTree/index.tsx index 2f30dc9c..1e5ce4f8 100644 --- a/src/components/AgentTree/index.tsx +++ b/src/components/AgentTree/index.tsx @@ -6,11 +6,11 @@ import { map, filter, isEmpty } from "lodash"; import { ChangeAgentsRenderingStateAction, - ApplyUserColorAction, SetRecentColorsAction, SetVisibleAction, AgentRenderingCheckboxMap, } from "../../state/selection/types"; +import { ColorChange } from "../../constants/interfaces"; import SharedCheckbox from "../SharedCheckbox"; import AgentTreeSubmenu from "../AgentTreeSubmenu"; import TreeNode from "../TreeNode"; @@ -19,6 +19,8 @@ import { CHECKBOX_TYPE_STAR } from "../../constants"; import ColorPicker from "../ColorPicker"; import NoTypeMappingText from "../NoTrajectoriesText/NoTypeMappingText"; +import styles from "./style.css"; + const { Text } = Typography; interface CheckBoxWithColor extends CheckboxOptionType { @@ -45,11 +47,10 @@ interface AgentTreeProps { payloadForSelectNone: AgentRenderingCheckboxMap; isSharedCheckboxIndeterminate: boolean; recentColors: string[]; - applyUserColor: ActionCreator; + applyUserColor: (colorChange: ColorChange) => void; setRecentColors: ActionCreator; } const CHECKBOX_SPAN_NO = 2; -import styles from "./style.css"; class AgentTree extends React.Component { onSubCheckboxChange = (key: string, values: string[]) => { diff --git a/src/components/ColorPicker/index.tsx b/src/components/ColorPicker/index.tsx index 9c1b1296..3181813f 100644 --- a/src/components/ColorPicker/index.tsx +++ b/src/components/ColorPicker/index.tsx @@ -7,10 +7,7 @@ import { useDebounce } from "use-debounce"; import { AGENT_COLORS } from "../../containers/ViewerPanel/constants"; import { ColorChange } from "../../constants/interfaces"; -import { - ApplyUserColorAction, - SetRecentColorsAction, -} from "../../state/selection/types"; +import { SetRecentColorsAction } from "../../state/selection/types"; import styles from "./style.css"; @@ -20,7 +17,7 @@ interface ColorPickerProps { agentName: string; tags: string[]; recentColors: string[]; - applyUserColor: ActionCreator; + applyUserColor: (colorChange: ColorChange) => void; setRecentColors: ActionCreator; } diff --git a/src/containers/ModelPanel/index.tsx b/src/containers/ModelPanel/index.tsx index b21a2967..7c742825 100644 --- a/src/containers/ModelPanel/index.tsx +++ b/src/containers/ModelPanel/index.tsx @@ -16,14 +16,14 @@ import { ChangeAgentsRenderingStateAction, SetVisibleAction, SetRecentColorsAction, - ApplyUserColorAction, + HandleColorChangeAction, } from "../../state/selection/types"; import { turnAgentsOnByDisplayKey, highlightAgentsByDisplayKey, setAgentsVisible, setRecentColors, - applyUserColor, + handleColorChange, } from "../../state/selection/actions"; import { getAgentVisibilityMap, @@ -60,9 +60,9 @@ interface ModelPanelProps { isNetworkedFile: boolean; changeToNetworkedFile: ActionCreator; recentColors: string[]; - applyUserColor: ActionCreator; setRecentColors: ActionCreator; selectedAgentMetadata: AgentMetadata; + handleColorChange: ActionCreator; } const ModelPanel: React.FC = ({ @@ -79,9 +79,9 @@ const ModelPanel: React.FC = ({ isNetworkedFile, changeToNetworkedFile: loadNetworkFile, recentColors, - applyUserColor, setRecentColors, selectedAgentMetadata, + handleColorChange, }): JSX.Element => { const checkboxTree = ( = ({ payloadForSelectNone={payloadForSelectNone} isSharedCheckboxIndeterminate={isSharedCheckboxIndeterminate} recentColors={recentColors} - applyUserColor={applyUserColor} + applyUserColor={handleColorChange} setRecentColors={setRecentColors} /> ); @@ -145,8 +145,8 @@ const dispatchToPropsMap = { turnAgentsOnByDisplayKey, highlightAgentsByDisplayKey, setAgentsVisible, - applyUserColor, setRecentColors, + handleColorChange, }; export default connect(mapStateToProps, dispatchToPropsMap)(ModelPanel); diff --git a/src/containers/ViewerPanel/index.tsx b/src/containers/ViewerPanel/index.tsx index 0e3bd87b..5adf47f0 100644 --- a/src/containers/ViewerPanel/index.tsx +++ b/src/containers/ViewerPanel/index.tsx @@ -22,6 +22,7 @@ import trajectoryStateBranch from "../../state/trajectory"; import viewerStateBranch from "../../state/viewer"; import { ChangeTimeAction, + GetDisplayDataFromBrowserAction, SetSelectedAgentMetadataAction, SetVisibleAction, } from "../../state/selection/types"; @@ -113,6 +114,7 @@ interface ViewerPanelProps { receiveConvertedFile: ActionCreator; conversionProcessingData: ConversionProcessingData; setSelectedAgentMetadata: ActionCreator; + getDisplayDataFromBrowserStorage: ActionCreator; } interface ViewerPanelState { @@ -340,12 +342,14 @@ class ViewerPanel extends React.Component { setBuffering, isLooping, setUrlParams, + getDisplayDataFromBrowserStorage, } = this.props; if (this.state.isInitialPlay) { receiveTrajectory({ firstFrameTime: timeData.time, lastFrameTime: (numFrames - 1) * timeStep + timeData.time, }); + getDisplayDataFromBrowserStorage(); if (hasUrlParamsSettings()) { // these are settings that need to be applied after the trajectory // is loaded but before the user can interact with the trajectory @@ -669,6 +673,8 @@ const dispatchToPropsMap = { setUrlParams: trajectoryStateBranch.actions.setUrlParams, setSelectedAgentMetadata: selectionStateBranch.actions.setSelectedAgentMetadata, + getDisplayDataFromBrowserStorage: + selectionStateBranch.actions.getDisplayDataFromBrowserStorage, }; export default connect(mapStateToProps, dispatchToPropsMap)(ViewerPanel); diff --git a/src/containers/ViewerPanel/selectors.ts b/src/containers/ViewerPanel/selectors.ts index be0a2403..546736c6 100644 --- a/src/containers/ViewerPanel/selectors.ts +++ b/src/containers/ViewerPanel/selectors.ts @@ -11,15 +11,15 @@ import { getAgentsToHide, getCurrentTime, getHighlightedAgents, - getSelectedUIDisplayData, } from "../../state/selection/selectors"; import { AgentRenderingCheckboxMap } from "../../state/selection/types"; import { roundTimeForDisplay } from "../../util"; import { DisplayTimes } from "./types"; import { isNetworkSimFileInterface } from "../../state/trajectory/types"; +import { getCurrentUIData } from "../../state/compoundSelectors"; export const getSelectionStateInfoForViewer = createSelector( - [getHighlightedAgents, getAgentsToHide, getSelectedUIDisplayData], + [getHighlightedAgents, getAgentsToHide, getCurrentUIData], (highlightedAgents, hiddenAgents, appliedColors): SelectionStateInfo => ({ highlightedAgents, hiddenAgents, diff --git a/src/state/compoundSelectors/index.ts b/src/state/compoundSelectors/index.ts index 0f9f50d7..ce2c1132 100644 --- a/src/state/compoundSelectors/index.ts +++ b/src/state/compoundSelectors/index.ts @@ -22,13 +22,8 @@ export const getCurrentUIData = createSelector( sessionData: UIDisplayData, defaultData: UIDisplayData ) => { - const fileHasBeenParsed = defaultData.length > 0; - if (!fileHasBeenParsed) { - return []; - } - if (colorSetting === ColorSetting.UserSelected) { - return sessionData; - } - return defaultData; + return colorSetting === ColorSetting.UserSelected + ? sessionData + : defaultData; } ); diff --git a/src/state/selection/actions.ts b/src/state/selection/actions.ts index d7aad6fd..c384b98c 100644 --- a/src/state/selection/actions.ts +++ b/src/state/selection/actions.ts @@ -10,8 +10,10 @@ import { RESET_AGENT_SELECTIONS_AND_HIGHLIGHTS, SET_RECENT_COLORS, SET_SELECTED_AGENT, - APPLY_USER_COLOR, SET_SELECTED_DISPLAY_DATA, + GET_DISPLAY_DATA_FROM_BROWSER, + SET_CURRENT_COLOR_SETTINGS, + HANDLE_COLOR_CHANGE, } from "./constants"; import { ChangeAgentsRenderingStateAction, @@ -20,10 +22,12 @@ import { SetVisibleAction, AgentRenderingCheckboxMap, ResetAction, - ApplyUserColorAction, SetRecentColorsAction, SetSelectedAgentMetadataAction, SetSelectedUIDisplayDataAction, + ColorSetting, + SetCurrentColorSettingAction, + HandleColorChangeAction, } from "./types"; export function changeTime(time: number): ChangeTimeAction { @@ -67,13 +71,6 @@ export function highlightAgentsByDisplayKey( }; } -export function applyUserColor(payload: ColorChange): ApplyUserColorAction { - return { - payload, - type: APPLY_USER_COLOR, - }; -} - export function selectMetadata( key: string, payload: string | number @@ -115,3 +112,27 @@ export function setSelectedUIDisplayData( type: SET_SELECTED_DISPLAY_DATA, }; } + +export function handleColorChange( + colorChange: ColorChange +): HandleColorChangeAction { + return { + payload: colorChange, + type: HANDLE_COLOR_CHANGE, + }; +} + +export function getDisplayDataFromBrowserStorage() { + return { + type: GET_DISPLAY_DATA_FROM_BROWSER, + }; +} + +export function setCurrentColorSetting(payload: { + currentColorSetting: ColorSetting; +}): SetCurrentColorSettingAction { + return { + payload, + type: SET_CURRENT_COLOR_SETTINGS, + }; +} diff --git a/src/state/selection/constants.ts b/src/state/selection/constants.ts index 41e10757..f72230fc 100644 --- a/src/state/selection/constants.ts +++ b/src/state/selection/constants.ts @@ -18,9 +18,17 @@ export const SET_AGENTS_VISIBLE = makeSelectConstant("set-agents-visible"); export const RESET_AGENT_SELECTIONS_AND_HIGHLIGHTS = makeSelectConstant( "reset-selections-and-highlighted-agents" ); -export const APPLY_USER_COLOR = makeSelectConstant("apply-user-color"); export const SET_RECENT_COLORS = makeSelectConstant("set-recent-colors"); export const SET_SELECTED_AGENT = makeSelectConstant("set-selected-agent"); export const SET_SELECTED_DISPLAY_DATA = makeSelectConstant( "set-selected-display-data" ); +export const HANDLE_COLOR_CHANGE = makeSelectConstant( + "store-display-data-in-browser" +); +export const GET_DISPLAY_DATA_FROM_BROWSER = makeSelectConstant( + "get-display-data-from-browser" +); +export const SET_CURRENT_COLOR_SETTINGS = makeSelectConstant( + "set-current-color-settings" +); diff --git a/src/state/selection/logics.ts b/src/state/selection/logics.ts index 643c4816..c8a85f22 100644 --- a/src/state/selection/logics.ts +++ b/src/state/selection/logics.ts @@ -1,46 +1,73 @@ import { createLogic } from "redux-logic"; -import { UIDisplayData } from "@aics/simularium-viewer"; import { ReduxLogicDeps } from "../types"; -import { getSimulariumFile } from "../trajectory/selectors"; +import { + getDefaultUIDisplayData, + getSimulariumFile, +} from "../trajectory/selectors"; +import { getCurrentUIData } from "../compoundSelectors"; +import { + GET_DISPLAY_DATA_FROM_BROWSER, + HANDLE_COLOR_CHANGE, +} from "./constants"; +import { setCurrentColorSetting, setSelectedUIDisplayData } from "./actions"; +import { applyColorChangeToUiDisplayData, isSameAgentTree } from "../../util"; +import { ColorSetting } from "./types"; -import { APPLY_USER_COLOR } from "./constants"; -import { getSelectedUIDisplayData } from "./selectors"; -import { setSelectedUIDisplayData } from "./actions"; - -const storeColorsLogic = createLogic({ - process(deps: ReduxLogicDeps, dispatch, done) { - const { action, getState } = deps; - const uiData: UIDisplayData = getSelectedUIDisplayData(getState()); +const handleColorChangeLogic = createLogic({ + validate(deps: ReduxLogicDeps, allow, reject) { + const { getState, action } = deps; const colorChange = action.payload; - const newUiData = uiData.map((agent) => { - const newAgent = { ...agent }; - if (agent.name === colorChange.agent.name) { - if (colorChange.agent.tags.includes("")) { - newAgent.color = colorChange.color; - } - const newDisplayStates = agent.displayStates.map( - (state: any) => { - if (colorChange.agent.tags.includes(state.id)) { - return { - ...state, - color: colorChange.color, - }; - } - return state; - } - ); - newAgent.displayStates = newDisplayStates; - } - return newAgent; + const currentDisplayData = getCurrentUIData(getState()); + const newDisplayData = applyColorChangeToUiDisplayData( + colorChange, + currentDisplayData + ); + if (newDisplayData !== currentDisplayData) { + const fileKey = getSimulariumFile(getState()).name; + localStorage.setItem(fileKey, JSON.stringify(newDisplayData)); + allow(setSelectedUIDisplayData(newDisplayData)); + } else { + reject(action); + } + }, + process() { + return setCurrentColorSetting({ + currentColorSetting: ColorSetting.UserSelected, }); - dispatch(setSelectedUIDisplayData(newUiData)); - // store color changes in local browser storage + }, + type: HANDLE_COLOR_CHANGE, +}); + +const getDisplayDataFromBrowserLogic = createLogic({ + validate(deps: ReduxLogicDeps, allow, reject) { + const { getState, action } = deps; const fileKey = getSimulariumFile(getState()).name; - localStorage.setItem(fileKey, JSON.stringify(newUiData)); - done(); + const storedColorChanges = localStorage.getItem(fileKey) || "[]"; + if (storedColorChanges === "[]" || storedColorChanges === "undefined") { + reject(action); + } + const storedUIData = JSON.parse(storedColorChanges); + const defaultUIData = getDefaultUIDisplayData(getState()); + + const validSettings = isSameAgentTree(storedUIData, defaultUIData); + if (!validSettings) { + console.warn( + "Agent structures do not match, not applying color settings from browser storage" + ); + reject(action); + } + setCurrentColorSetting({ + currentColorSetting: ColorSetting.UserSelected, + }); + allow(setSelectedUIDisplayData(storedUIData)); + }, + process() { + return setCurrentColorSetting({ + currentColorSetting: ColorSetting.UserSelected, + }); }, - type: APPLY_USER_COLOR, + type: GET_DISPLAY_DATA_FROM_BROWSER, }); -export default [storeColorsLogic]; +export default [handleColorChangeLogic, getDisplayDataFromBrowserLogic]; diff --git a/src/state/selection/reducer.ts b/src/state/selection/reducer.ts index 8b502b07..e3f3f2ac 100644 --- a/src/state/selection/reducer.ts +++ b/src/state/selection/reducer.ts @@ -14,6 +14,7 @@ import { SET_RECENT_COLORS, SET_SELECTED_AGENT, SET_SELECTED_DISPLAY_DATA, + SET_CURRENT_COLOR_SETTINGS, } from "./constants"; import { ChangeAgentsRenderingStateAction, @@ -27,6 +28,7 @@ import { SetSelectedAgentMetadataAction, SetSelectedUIDisplayDataAction, ColorSetting, + SetCurrentColorSettingAction, } from "./types"; export const initialState = { @@ -49,6 +51,8 @@ const actionToConfigMap: TypeToDescriptionMap = { ...state, agentVisibilityMap: initialState.agentVisibilityMap, agentHighlightMap: initialState.agentHighlightMap, + setSelectedUIDisplayData: initialState.selectedUIDisplayData, + currentColorSetting: initialState.currentColorSetting, }; }, }, @@ -174,6 +178,19 @@ const actionToConfigMap: TypeToDescriptionMap = { }; }, }, + [SET_CURRENT_COLOR_SETTINGS]: { + accepts: (action: AnyAction): action is SetCurrentColorSettingAction => + action.type === SET_CURRENT_COLOR_SETTINGS, + perform: ( + state: SelectionStateBranch, + action: SetCurrentColorSettingAction + ) => { + return { + ...state, + currentColorSetting: action.payload.currentColorSetting, + }; + }, + }, }; export default makeReducer( diff --git a/src/state/selection/types.ts b/src/state/selection/types.ts index dc1239bc..c9b1e174 100644 --- a/src/state/selection/types.ts +++ b/src/state/selection/types.ts @@ -68,11 +68,6 @@ export interface ResetAction { type: string; } -export interface ApplyUserColorAction { - payload: ColorChange; - type: string; -} - export interface SetRecentColorsAction { payload: string[]; type: string; @@ -88,6 +83,20 @@ export interface SetSelectedUIDisplayDataAction { type: string; } +export interface HandleColorChangeAction { + payload: ColorChange; + type: string; +} + +export interface SetCurrentColorSettingAction { + payload: { currentColorSetting: ColorSetting }; + type: string; +} + +export interface GetDisplayDataFromBrowserAction { + type: string; +} + export enum ColorSetting { UserSelected = "userSelected", Default = "default", diff --git a/src/state/trajectory/logics.ts b/src/state/trajectory/logics.ts index 4c2a2252..65c44e61 100644 --- a/src/state/trajectory/logics.ts +++ b/src/state/trajectory/logics.ts @@ -28,7 +28,6 @@ import { ViewerStatus } from "../viewer/types"; import { changeTime, resetAgentSelectionsAndHighlights, - setSelectedUIDisplayData, } from "../selection/actions"; import { setSimulariumController } from "../simularium/actions"; import { getSimulariumController } from "../simularium/selectors"; @@ -63,7 +62,6 @@ import { CONVERT_FILE, RECEIVE_CONVERTED_FILE, CANCEL_CONVERSION, - SET_DEFAULT_UI_DATA, } from "./constants"; import { ReceiveAction, @@ -257,6 +255,7 @@ const loadLocalFile = createLogic({ } clearOutFileTrajectoryUrlParam(); + clearSimulariumFile({ newFile: true }); simulariumController .changeFile( { @@ -640,15 +639,6 @@ const cancelConversionLogic = createLogic({ type: CANCEL_CONVERSION, }); -const setInitialSelectedUIData = createLogic({ - process(deps: ReduxLogicDeps, dispatch) { - const { action } = deps; - const uiData = action.payload; - dispatch(setSelectedUIDisplayData(uiData)); - }, - type: SET_DEFAULT_UI_DATA, -}); - export default [ requestPlotDataLogic, loadLocalFile, @@ -661,5 +651,4 @@ export default [ convertFileLogic, receiveConvertedFileLogic, cancelConversionLogic, - setInitialSelectedUIData, ]; diff --git a/src/util/index.ts b/src/util/index.ts index 4ba05790..83a1f9b7 100644 --- a/src/util/index.ts +++ b/src/util/index.ts @@ -1,6 +1,6 @@ import { forOwn, isFunction } from "lodash"; import React from "react"; -import { IconGlyphs } from "../constants/interfaces"; +import { ColorChange, IconGlyphs } from "../constants/interfaces"; import { UIDisplayData } from "@aics/simularium-viewer"; type AnyFunction = () => any; @@ -115,7 +115,7 @@ We don't want to ever try and apply the color settings from one trajectory to another, even if by chance they shared the same file name, or other metadata. */ -export const compareAgentTrees = (a: UIDisplayData, b: UIDisplayData) => { +export const isSameAgentTree = (a: UIDisplayData, b: UIDisplayData) => { if (a.length !== b.length) { return false; } @@ -137,3 +137,28 @@ export const compareAgentTrees = (a: UIDisplayData, b: UIDisplayData) => { } return true; }; + +export const applyColorChangeToUiDisplayData = ( + colorChange: ColorChange, + uiDisplayData: UIDisplayData +): UIDisplayData => { + return uiDisplayData.map((agent) => { + const newAgent = { ...agent }; + if (agent.name === colorChange.agent.name) { + if (colorChange.agent.tags.includes("")) { + newAgent.color = colorChange.color; + } + const newDisplayStates = agent.displayStates.map((state: any) => { + if (colorChange.agent.tags.includes(state.id)) { + return { + ...state, + color: colorChange.color, + }; + } + return state; + }); + newAgent.displayStates = newDisplayStates; + } + return newAgent; + }); +}; diff --git a/src/util/test/index.test.ts b/src/util/test/index.test.ts index bcdd7242..c05d8db3 100644 --- a/src/util/test/index.test.ts +++ b/src/util/test/index.test.ts @@ -14,7 +14,8 @@ import { formatFloatForDisplay, copyToClipboard, roundToTimeStepPrecision, - compareAgentTrees, + isSameAgentTree, + applyColorChangeToUiDisplayData, } from "../"; import { getFileIdFromUrl, @@ -240,6 +241,98 @@ describe("General utilities", () => { expect(roundTimeForDisplay(1.20000001)).toBe(1.2); }); }); + + describe("applyColorChangeToUiDisplayData", () => { + const mockUIDisplayData: UIDisplayData = [ + { + name: "agent1", + color: "#0000ff", + displayStates: [ + { name: "state1", id: "state1", color: "#0000ff" }, + { name: "state2", id: "state2", color: "#0000ff" }, + ], + }, + { + name: "agent2", + color: "#ff0000", + displayStates: [ + { name: "state3", id: "state3", color: "#ff0000" }, + { name: "state4", id: "state4", color: "#ff0000" }, + ], + }, + ]; + + it("should update agent color when tags include empty string", () => { + const colorChange = { + color: "#00ff00", + agent: { + name: "agent1", + tags: [""], + }, + }; + + const result = applyColorChangeToUiDisplayData( + colorChange, + mockUIDisplayData + ); + + expect(result[0].color).toBe("#00ff00"); + expect(result[1].color).toBe("#ff0000"); + }); + + it("should update specific display states based on tags", () => { + const colorChange = { + color: "#ffff00", + agent: { + name: "agent1", + tags: ["state1"], + }, + }; + + const result = applyColorChangeToUiDisplayData( + colorChange, + mockUIDisplayData + ); + + expect(result[0].displayStates[0].color).toBe("#ffff00"); // state1 updated + expect(result[0].displayStates[1].color).toBe("#0000ff"); // state2 unchanged + }); + + it("should update multiple display states for agents with multiple tags", () => { + const colorChange = { + color: "#800080", + agent: { + name: "agent1", + tags: ["state1", "state2"], + }, + }; + + const result = applyColorChangeToUiDisplayData( + colorChange, + mockUIDisplayData + ); + + expect(result[0].displayStates[0].color).toBe("#800080"); + expect(result[0].displayStates[1].color).toBe("#800080"); + }); + + it("should not modify data when agent name does not match", () => { + const colorChange = { + color: "#ffA500", + agent: { + name: "nonexistentAgent", + tags: [""], + }, + }; + + const result = applyColorChangeToUiDisplayData( + colorChange, + mockUIDisplayData + ); + + expect(result).toEqual(mockUIDisplayData); + }); + }); }); describe("User Url handling", () => { @@ -534,7 +627,7 @@ describe("User Url handling", () => { expect(result).toBe(1); }); }); - describe("compareAgentTrees", () => { + describe("isSameAgentTree", () => { it("should return false if the arrays are different lengths", () => { const firstUIData: UIDisplayData = [ { @@ -544,7 +637,7 @@ describe("User Url handling", () => { }, ]; const secondUIData: UIDisplayData = []; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return false if the names are different", () => { @@ -562,7 +655,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return false if some entry names match, but others don't", () => { @@ -590,7 +683,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return false if displayState lengths don't match", () => { @@ -608,7 +701,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return false if displayState names don't match", () => { @@ -628,7 +721,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return false if displayState ids don't match", () => { @@ -648,7 +741,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(false); }); it("should return true if the agent tree structures match", () => { @@ -666,7 +759,7 @@ describe("User Url handling", () => { color: "", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(true); }); it("should return true if the agent tree structures match but have different color properties", () => { @@ -696,7 +789,7 @@ describe("User Url handling", () => { color: "blue", }, ]; - const result = compareAgentTrees(firstUIData, secondUIData); + const result = isSameAgentTree(firstUIData, secondUIData); expect(result).toBe(true); }); }); From 2709efef0bf5bab52acef854bbf4d5b1e67fea5f Mon Sep 17 00:00:00 2001 From: Joe Heffernan Date: Wed, 27 Nov 2024 16:01:00 -0800 Subject: [PATCH 07/13] use native onClick in place of clickHandler prop in custom buttons (#612) --- src/components/CameraControls/CameraHomeButton.tsx | 2 +- src/components/CameraControls/index.tsx | 14 +++++++------- src/components/CustomDropdown/index.tsx | 2 +- src/components/DownloadTrajectoryMenu/index.tsx | 2 +- src/components/HelpMenu/index.tsx | 2 +- src/components/LoadFileMenu/index.tsx | 2 +- src/components/NavButton/index.tsx | 5 ++--- src/components/NavButtonWithTooltip/index.tsx | 6 +++--- src/components/PlaybackControls/index.tsx | 8 ++++---- src/components/RecordMoviesComponent/index.tsx | 2 +- src/components/ShareTrajectoryButton/index.tsx | 2 +- src/components/ViewportButton/index.tsx | 7 +++---- src/containers/AppHeader/index.tsx | 2 +- src/containers/ViewerPanel/index.tsx | 2 +- 14 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/components/CameraControls/CameraHomeButton.tsx b/src/components/CameraControls/CameraHomeButton.tsx index 40e626ad..a7f34ffe 100644 --- a/src/components/CameraControls/CameraHomeButton.tsx +++ b/src/components/CameraControls/CameraHomeButton.tsx @@ -15,7 +15,7 @@ export const CameraHomeButton: React.FC = ({ tooltipText={"Home view (H)"} tooltipPlacement="left" icon={getIconGlyphClasses(IconGlyphs.Reset)} - clickHandler={resetCamera} + onClick={resetCamera} /> ); }; diff --git a/src/components/CameraControls/index.tsx b/src/components/CameraControls/index.tsx index aa22365a..0e4ffb3a 100644 --- a/src/components/CameraControls/index.tsx +++ b/src/components/CameraControls/index.tsx @@ -159,13 +159,13 @@ const CameraControls = ({ tooltipText="Zoom in ( ↑ )" tooltipPlacement="left" icon={ZoomIn} - clickHandler={zoomIn} + onClick={zoomIn} />
@@ -176,7 +176,7 @@ const CameraControls = ({ tooltipPlacement="left" icon={getIconGlyphClasses(IconGlyphs.Rotate)} radioGroupPosition={"top"} - clickHandler={() => setMode(ROTATE)} + onClick={() => setMode(ROTATE)} active={mode === ROTATE} /> setMode(PAN)} + onClick={() => setMode(PAN)} active={mode === PAN} />
@@ -192,7 +192,7 @@ const CameraControls = ({ tooltipText={"Focus (F)"} tooltipPlacement="left" icon={getIconGlyphClasses(IconGlyphs.Focus)} - clickHandler={() => { + onClick={() => { saveFocusMode(!isFocused); }} active={isFocused} @@ -203,7 +203,7 @@ const CameraControls = ({ tooltipPlacement="left" icon={getIconGlyphClasses(IconGlyphs.Orthographic)} radioGroupPosition={"top"} - clickHandler={() => { + onClick={() => { setCameraProjectionType(ORTHOGRAPHIC); }} active={cameraProjectionType === ORTHOGRAPHIC} @@ -213,7 +213,7 @@ const CameraControls = ({ tooltipPlacement="left" icon={getIconGlyphClasses(IconGlyphs.Perspective)} radioGroupPosition={"bottom"} - clickHandler={() => { + onClick={() => { setCameraProjectionType(PERSPECTIVE); }} active={cameraProjectionType === PERSPECTIVE} diff --git a/src/components/CustomDropdown/index.tsx b/src/components/CustomDropdown/index.tsx index 4dbe58e9..885d1635 100644 --- a/src/components/CustomDropdown/index.tsx +++ b/src/components/CustomDropdown/index.tsx @@ -131,7 +131,7 @@ const CustomDropdown: React.FC = ({ titleText={titleText} icon={icon} buttonType={buttonType} - clickHandler={buttonClickHandler} + onClick={buttonClickHandler} onKeyDown={handleKeyDown} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeaveWithDelay} diff --git a/src/components/DownloadTrajectoryMenu/index.tsx b/src/components/DownloadTrajectoryMenu/index.tsx index dbe06769..c570da56 100644 --- a/src/components/DownloadTrajectoryMenu/index.tsx +++ b/src/components/DownloadTrajectoryMenu/index.tsx @@ -46,7 +46,7 @@ const DownloadTrajectoryMenu = ({ { label: ( { + onClick={() => { setModalVisible(!modalVisible); }} buttonType={ButtonClass.DropdownItem} diff --git a/src/components/LoadFileMenu/index.tsx b/src/components/LoadFileMenu/index.tsx index 4aa83f7b..830bc012 100644 --- a/src/components/LoadFileMenu/index.tsx +++ b/src/components/LoadFileMenu/index.tsx @@ -94,7 +94,7 @@ const LoadFileMenu = ({ label: ( ), diff --git a/src/components/NavButton/index.tsx b/src/components/NavButton/index.tsx index f989bbb2..05f8f3a1 100644 --- a/src/components/NavButton/index.tsx +++ b/src/components/NavButton/index.tsx @@ -8,7 +8,6 @@ export interface NavButtonProps extends ButtonProps { titleText?: string; buttonType?: string; icon?: ReactNode; - clickHandler?: () => void; isDisabled?: boolean; } @@ -19,7 +18,7 @@ const NavButton = forwardRef( titleText, buttonType = ButtonClass.Action, icon, - clickHandler, + onClick, isDisabled, ...props }, @@ -37,7 +36,7 @@ const NavButton = forwardRef( {...props} ref={ref} className={buttonClassNames} - onClick={!isDisabled ? clickHandler : undefined} + onClick={!isDisabled ? onClick : undefined} > {titleText} {icon} diff --git a/src/components/NavButtonWithTooltip/index.tsx b/src/components/NavButtonWithTooltip/index.tsx index 6511fa8d..7781ef66 100644 --- a/src/components/NavButtonWithTooltip/index.tsx +++ b/src/components/NavButtonWithTooltip/index.tsx @@ -24,7 +24,7 @@ const NavButtonWithTooltip: React.FC = ({ tooltipPlacement, buttonType, icon, - clickHandler, + onClick, isDisabled = false, ...props }) => { @@ -53,7 +53,7 @@ const NavButtonWithTooltip: React.FC = ({ icon, buttonType, isDisabled, - clickHandler, + onClick, onMouseEnter, onMouseLeave, }; @@ -69,7 +69,7 @@ const NavButtonWithTooltip: React.FC = ({ trigger={["hover", "focus"]} open={tooltipVisible} > - + ); }; diff --git a/src/components/PlaybackControls/index.tsx b/src/components/PlaybackControls/index.tsx index 939fd0ad..c6d4eb45 100644 --- a/src/components/PlaybackControls/index.tsx +++ b/src/components/PlaybackControls/index.tsx @@ -129,7 +129,7 @@ const PlayBackControls = ({ tooltipText={isPlaying ? "Pause" : "Play"} tooltipPlacement="top" icon={isPlaying ? Pause : Play} - clickHandler={isPlaying ? pauseHandler : playHandler} + onClick={isPlaying ? () => pauseHandler() : () => playHandler()} disabled={loading || isEmpty} loading={loading} /> @@ -169,7 +169,7 @@ const PlayBackControls = ({ tooltipText="Skip 1 frame back" tooltipPlacement="top" icon={getIconGlyphClasses(IconGlyphs.StepBack)} - clickHandler={prevHandler} + onClick={prevHandler} disabled={isStepBackDisabled || loading || isEmpty} loading={loading} /> @@ -178,7 +178,7 @@ const PlayBackControls = ({ tooltipText="Skip 1 frame ahead" tooltipPlacement="top" icon={getIconGlyphClasses(IconGlyphs.StepForward)} - clickHandler={nextHandler} + onClick={nextHandler} disabled={isStepForwardDisabled || loading || isEmpty} loading={loading} /> @@ -204,7 +204,7 @@ const PlayBackControls = ({ tooltipText={isLooping ? "Turn off looping" : "Turn on looping"} tooltipPlacement="top" icon={getIconGlyphClasses(IconGlyphs.Loop)} - clickHandler={loopHandler} + onClick={loopHandler} disabled={isEmpty} active={isLooping} loading={loading} diff --git a/src/components/RecordMoviesComponent/index.tsx b/src/components/RecordMoviesComponent/index.tsx index 056a5c8d..0eaa1ad3 100644 --- a/src/components/RecordMoviesComponent/index.tsx +++ b/src/components/RecordMoviesComponent/index.tsx @@ -114,7 +114,7 @@ const RecordMovieComponent = (props: RecordMovieComponentProps) => { tooltipWhenDisabled={true} tooltipText={getTooltipText()} icon={getIcon()} - clickHandler={isRecording ? stop : start} + onClick={isRecording ? stop : start} disabled={!supportedBrowser} onMouseEnter={() => setIsHovering(true)} onMouseLeave={() => setIsHovering(false)} diff --git a/src/components/ShareTrajectoryButton/index.tsx b/src/components/ShareTrajectoryButton/index.tsx index ab36b728..ec98bd03 100644 --- a/src/components/ShareTrajectoryButton/index.tsx +++ b/src/components/ShareTrajectoryButton/index.tsx @@ -34,7 +34,7 @@ const ShareTrajectoryButton = ({ tooltipPlacement="bottomLeft" titleText="Share" icon={Share} - clickHandler={handleShare} + onClick={handleShare} isDisabled={isDisabled} tooltipText={{ default: "Share trajectory", diff --git a/src/components/ViewportButton/index.tsx b/src/components/ViewportButton/index.tsx index 7d66cd52..a33cadcb 100644 --- a/src/components/ViewportButton/index.tsx +++ b/src/components/ViewportButton/index.tsx @@ -16,7 +16,6 @@ interface ViewportButtonProps extends ButtonProps { */ icon?: ReactNode | string; radioGroupPosition?: "top" | "bottom"; - clickHandler?: () => void; disabled?: boolean; active?: boolean; loading?: boolean; @@ -28,7 +27,7 @@ const ViewportButton: React.FC = ({ tooltipPlacement, tooltipWhenDisabled, icon, - clickHandler, + onClick, disabled, loading, active, @@ -61,7 +60,7 @@ const ViewportButton: React.FC = ({ const getTooltip = () => !disabled || tooltipWhenDisabled ? tooltipText : ""; - const getClickHandler = () => (!disabled ? clickHandler : undefined); + const onClickProp = !disabled ? onClick : undefined; const buttonClassNames = classNames([ className, @@ -79,7 +78,7 @@ const ViewportButton: React.FC = ({