From 7558180773b34e4c445f63dbeb0970032ed6bafd Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 13 Jun 2025 16:40:19 +0200 Subject: [PATCH 01/10] keep skeleton tab up to date with active tree --- .../right-border-tabs/trees_tab/skeleton_tab_view.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx index d13cd59ed92..1b7047f7224 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx @@ -793,6 +793,16 @@ class SkeletonTabView extends React.PureComponent { }); }; + componentDidUpdate(prevProps: Props) { + if (this.props.skeletonTracing?.activeTreeId !== prevProps.skeletonTracing?.activeTreeId) { + if (this.props.skeletonTracing?.activeTreeId != null) { + this.setState({ + selectedTreeIds: [this.props.skeletonTracing.activeTreeId], + }); + } + } + } + render() { const { skeletonTracing } = this.props; From fe1891f5ef73ed07c283ffa47ce10b2209d5209b Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Tue, 15 Jul 2025 15:10:50 +0200 Subject: [PATCH 02/10] changelog --- unreleased_changes/8689.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 unreleased_changes/8689.md diff --git a/unreleased_changes/8689.md b/unreleased_changes/8689.md new file mode 100644 index 00000000000..512d5379ae1 --- /dev/null +++ b/unreleased_changes/8689.md @@ -0,0 +1,2 @@ +### Fixed +- Fixed that the skeleton tab was not always reflecting the active tree. This makes it easier to navigate between comments and trees, for example. From 8e24beb2b3666cff8a1016cd45828a35f8539547 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 21 Jul 2025 10:48:28 +0200 Subject: [PATCH 03/10] add activeTree is null case to componentDidUpdate --- .../view/right-border-tabs/trees_tab/skeleton_tab_view.tsx | 4 ++++ .../right-border-tabs/trees_tab/tree_hierarchy_view.tsx | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx index 3c637bff311..7d41185c193 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx @@ -799,6 +799,10 @@ class SkeletonTabView extends React.PureComponent { this.setState({ selectedTreeIds: [this.props.skeletonTracing.activeTreeId], }); + } else { + this.setState({ + selectedTreeIds: [], + }); } } } diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx index 0d46a4fe51f..5c8ff196571 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx @@ -98,12 +98,14 @@ function TreeHierarchyView(props: Props) { useEffect(() => { // scroll to active tree if it changes - if (treeRef.current && activeTreeId) { + if (activeTreeId) { const activeTreeKey = getNodeKey(GroupTypeEnum.TREE, activeTreeId); // For some React rendering/timing reasons, the target element might not be rendered yet. That messes with calculating the offsets for scrolling. Hence delay this a bit setTimeout(() => { - if (treeRef.current) treeRef.current.scrollTo({ key: activeTreeKey, align: "auto" }); + if (treeRef.current) { + treeRef.current.scrollTo({ key: activeTreeKey, align: "auto" }); + } }, 50); // Make sure to select the active tree (for highlighting etc) From a060431daf1757da3ccb4ce44f5b23966bf6d2e3 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 21 Jul 2025 16:10:46 +0200 Subject: [PATCH 04/10] set longer delay for the first scroll in tree tab --- .../trees_tab/tree_hierarchy_view.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx index 5c8ff196571..42ee1ba6299 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx @@ -289,10 +289,16 @@ function TreeHierarchyView(props: Props) { }, 300); }, [activeTreeId]); - useEffect( - () => treeRef.current?.scrollTo({ key: selectedKeys[0], align: "auto" }), - [selectedKeys[0]], - ); + // Scroll to the active key after the component has been rendered. + // This is necessary outside of the useEffect hooks because a longer delay is needed to ensure the active tree has been rendered. + setTimeout(() => { + if (activeTreeId && treeRef.current) { + treeRef.current.scrollTo({ + key: getNodeKey(GroupTypeEnum.TREE, activeTreeId), + align: "auto", + }); + } + }, 900); return ( <> From 1fdf5ff3f346b6a99fef74052d9d87b31b88403c Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Tue, 22 Jul 2025 14:42:25 +0200 Subject: [PATCH 05/10] add middleware to listen to redux actions in react components --- .../viewer/model/helpers/event_bus.ts | 12 ++++++++++ .../model/helpers/event_emitter_middleware.ts | 7 ++++++ .../viewer/model/helpers/listener_helpers.ts | 12 ++++++++++ .../model/reducers/skeletontracing_reducer.ts | 1 + frontend/javascripts/viewer/store.ts | 8 ++++++- .../trees_tab/tree_hierarchy_view.tsx | 24 ++++++++++--------- 6 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 frontend/javascripts/viewer/model/helpers/event_bus.ts create mode 100644 frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts diff --git a/frontend/javascripts/viewer/model/helpers/event_bus.ts b/frontend/javascripts/viewer/model/helpers/event_bus.ts new file mode 100644 index 00000000000..3e74847df67 --- /dev/null +++ b/frontend/javascripts/viewer/model/helpers/event_bus.ts @@ -0,0 +1,12 @@ +import { type Emitter, createNanoEvents } from "nanoevents"; + +type ActionPayload = { + type: string; + [key: string]: any; +}; + +type Events = { + [actionType: string]: (action: ActionPayload) => void; +}; + +export const eventBus: Emitter = createNanoEvents(); diff --git a/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts b/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts new file mode 100644 index 00000000000..7c9c1072d38 --- /dev/null +++ b/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts @@ -0,0 +1,7 @@ +import type { Middleware } from "redux"; +import { eventBus } from "./event_bus"; + +export const eventEmitterMiddleware: Middleware = () => (next) => (action) => { + eventBus.emit(action.type, action); + return next(action); +}; diff --git a/frontend/javascripts/viewer/model/helpers/listener_helpers.ts b/frontend/javascripts/viewer/model/helpers/listener_helpers.ts index d2b73eff330..83b429e38c3 100644 --- a/frontend/javascripts/viewer/model/helpers/listener_helpers.ts +++ b/frontend/javascripts/viewer/model/helpers/listener_helpers.ts @@ -1,5 +1,7 @@ +import { useEffect } from "react"; import type { WebknossosState } from "viewer/store"; import Store from "viewer/store"; +import { eventBus } from "./event_bus"; // Allows to listen on a certain property of the store. // This function should only be used when listening to the Store @@ -41,4 +43,14 @@ export function listenToStoreProperty( // return the unsubscribe function return Store.subscribe(handleChange); } + +export function useReduxActionListener(actionType: string, callback: (action: any) => void) { + useEffect(() => { + const unsubscribe = eventBus.on(actionType, callback); + return () => { + unsubscribe(); + }; + }, [actionType, callback]); +} + export default {}; diff --git a/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts b/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts index dfca78bd3b1..678aab2e07b 100644 --- a/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts +++ b/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts @@ -558,6 +558,7 @@ function SkeletonTracingReducer( } case "EXPAND_PARENT_GROUPS_OF_TREE": { + console.log("Expanding parent groups of tree in reducer"); const { tree } = action; if (tree.groupId == null || skeletonTracing == null) { return state; diff --git a/frontend/javascripts/viewer/store.ts b/frontend/javascripts/viewer/store.ts index 0c220532519..a0e4e0b33ab 100644 --- a/frontend/javascripts/viewer/store.ts +++ b/frontend/javascripts/viewer/store.ts @@ -78,6 +78,7 @@ import UiReducer from "viewer/model/reducers/ui_reducer"; import UserReducer from "viewer/model/reducers/user_reducer"; import ViewModeReducer from "viewer/model/reducers/view_mode_reducer"; import VolumeTracingReducer from "viewer/model/reducers/volumetracing_reducer"; +import { eventEmitterMiddleware } from "./model/helpers/event_emitter_middleware"; import FlycamInfoCacheReducer from "./model/reducers/flycam_info_cache_reducer"; import OrganizationReducer from "./model/reducers/organization_reducer"; import type { StartAIJobModalState } from "./view/action-bar/starting_job_modals"; @@ -610,7 +611,12 @@ export const combinedReducer = reduceReducers( const store = createStore( enableBatching(combinedReducer as any), defaultState, - applyMiddleware(actionLoggerMiddleware, overwriteActionMiddleware, sagaMiddleware as Middleware), + applyMiddleware( + actionLoggerMiddleware, + overwriteActionMiddleware, + sagaMiddleware as Middleware, + eventEmitterMiddleware, + ), ); export function startSaga(saga: Saga) { diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx index 42ee1ba6299..c51165daf99 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx @@ -15,6 +15,7 @@ import { toggleTreeAction, toggleTreeGroupAction, } from "viewer/model/actions/skeletontracing_actions"; +import { useReduxActionListener } from "viewer/model/helpers/listener_helpers"; import type { Tree, TreeGroup, TreeMap } from "viewer/model/types/tree_types"; import { api } from "viewer/singletons"; import { Store } from "viewer/singletons"; @@ -280,25 +281,26 @@ function TreeHierarchyView(props: Props) { ); if (expandedGroups == null) return; setExpandedGroups(expandedGroups); - setTimeout(() => { - if (treeRef.current && activeTreeId) - treeRef.current.scrollTo({ - key: getNodeKey(GroupTypeEnum.TREE, activeTreeId), - align: "auto", - }); - }, 300); + setTimeout(scrollToActiveTree, 300); }, [activeTreeId]); - // Scroll to the active key after the component has been rendered. - // This is necessary outside of the useEffect hooks because a longer delay is needed to ensure the active tree has been rendered. - setTimeout(() => { + const scrollToActiveTree = () => { if (activeTreeId && treeRef.current) { treeRef.current.scrollTo({ key: getNodeKey(GroupTypeEnum.TREE, activeTreeId), align: "auto", }); } - }, 900); + }; + + // Scroll to the active key after the component has been rendered. + // This is necessary outside of the useEffect hooks because a longer delay is needed to ensure the active tree has been rendered. + setTimeout(scrollToActiveTree, 900); + + useReduxActionListener("EXPAND_PARENT_GROUPS_OF_TREE", () => { + console.log("EXPAND_PARENT_GROUPS_OF_TREE action received, scrolling to active tree"); + scrollToActiveTree(); + }); return ( <> From 4ea68080bd3d333ad44670c75fdefb83dd340f59 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Tue, 22 Jul 2025 15:48:16 +0200 Subject: [PATCH 06/10] add new action to focus trees --- .../viewer/model/actions/skeletontracing_actions.tsx | 9 +++++++++ .../viewer/model/reducers/skeletontracing_reducer.ts | 1 - frontend/javascripts/viewer/view/context_menu.tsx | 2 ++ .../right-border-tabs/trees_tab/tree_hierarchy_view.tsx | 5 +---- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx b/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx index 6b044c1fb20..24cf77f8715 100644 --- a/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx +++ b/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx @@ -27,6 +27,7 @@ type SetTreeVisibilityAction = ReturnType; type SetExpandedTreeGroupsByKeysAction = ReturnType; type SetExpandedTreeGroupsByIdsAction = ReturnType; type ExpandParentGroupsOfTreeAction = ReturnType; +type FocusTreeAction = ReturnType; type ToggleAllTreesAction = ReturnType; type ToggleInactiveTreesAction = ReturnType; type ToggleTreeGroupAction = ReturnType; @@ -119,6 +120,7 @@ export type SkeletonTracingAction = | SetExpandedTreeGroupsByKeysAction | SetExpandedTreeGroupsByIdsAction | ExpandParentGroupsOfTreeAction + | FocusTreeAction | ToggleInactiveTreesAction | ToggleTreeGroupAction | NoAction @@ -401,6 +403,13 @@ export const expandParentGroupsOfTreeAction = (tree: Tree) => tree, }) as const; +export const focusTreeAction = (tree: Tree) => { + return { + type: "FOCUS_TREE", + tree, + } as const; +}; + export const setTreeVisibilityAction = (treeId: number | null | undefined, isVisible: boolean) => ({ type: "SET_TREE_VISIBILITY", diff --git a/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts b/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts index 678aab2e07b..dfca78bd3b1 100644 --- a/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts +++ b/frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts @@ -558,7 +558,6 @@ function SkeletonTracingReducer( } case "EXPAND_PARENT_GROUPS_OF_TREE": { - console.log("Expanding parent groups of tree in reducer"); const { tree } = action; if (tree.groupId == null || skeletonTracing == null) { return state; diff --git a/frontend/javascripts/viewer/view/context_menu.tsx b/frontend/javascripts/viewer/view/context_menu.tsx index e4fcfcaa723..d53088d4948 100644 --- a/frontend/javascripts/viewer/view/context_menu.tsx +++ b/frontend/javascripts/viewer/view/context_menu.tsx @@ -110,6 +110,7 @@ import { deleteBranchpointByIdAction, deleteEdgeAction, expandParentGroupsOfTreeAction, + focusTreeAction, mergeTreesAction, setActiveNodeAction, setTreeVisibilityAction, @@ -624,6 +625,7 @@ function getNodeContextMenuOptions({ key: "focus-tree", onClick: () => { Store.dispatch(expandParentGroupsOfTreeAction(clickedTree)); + Store.dispatch(focusTreeAction(clickedTree)); }, label: "Focus Tree in Skeleton Tab", } diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx index c51165daf99..43f9e8f022e 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx @@ -297,10 +297,7 @@ function TreeHierarchyView(props: Props) { // This is necessary outside of the useEffect hooks because a longer delay is needed to ensure the active tree has been rendered. setTimeout(scrollToActiveTree, 900); - useReduxActionListener("EXPAND_PARENT_GROUPS_OF_TREE", () => { - console.log("EXPAND_PARENT_GROUPS_OF_TREE action received, scrolling to active tree"); - scrollToActiveTree(); - }); + useReduxActionListener("FOCUS_TREE", scrollToActiveTree); return ( <> From 7cba7b6b107a727bdccd8437ac220c1779acfcc7 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 28 Jul 2025 11:57:24 +0200 Subject: [PATCH 07/10] clean up scrolling methods --- .../trees_tab/tree_hierarchy_view.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx index 43f9e8f022e..d5754253260 100644 --- a/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx +++ b/frontend/javascripts/viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view.tsx @@ -100,14 +100,8 @@ function TreeHierarchyView(props: Props) { useEffect(() => { // scroll to active tree if it changes if (activeTreeId) { - const activeTreeKey = getNodeKey(GroupTypeEnum.TREE, activeTreeId); - // For some React rendering/timing reasons, the target element might not be rendered yet. That messes with calculating the offsets for scrolling. Hence delay this a bit - setTimeout(() => { - if (treeRef.current) { - treeRef.current.scrollTo({ key: activeTreeKey, align: "auto" }); - } - }, 50); + setTimeout(() => scrollToActiveTree(), 50); // Make sure to select the active tree (for highlighting etc) // Remember, the active tree can be changed by actions outside of this component @@ -281,7 +275,6 @@ function TreeHierarchyView(props: Props) { ); if (expandedGroups == null) return; setExpandedGroups(expandedGroups); - setTimeout(scrollToActiveTree, 300); }, [activeTreeId]); const scrollToActiveTree = () => { @@ -295,8 +288,12 @@ function TreeHierarchyView(props: Props) { // Scroll to the active key after the component has been rendered. // This is necessary outside of the useEffect hooks because a longer delay is needed to ensure the active tree has been rendered. - setTimeout(scrollToActiveTree, 900); + // biome-ignore lint/correctness/useExhaustiveDependencies: This should be equivalent to componentDidMount + useEffect(() => { + setTimeout(scrollToActiveTree, 900); + }, []); + // Allow scrolling to the active tree even if it is not changed useReduxActionListener("FOCUS_TREE", scrollToActiveTree); return ( From 101af550644dfdd4e9a771448a5a69c5f82643fb Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 28 Jul 2025 13:24:01 +0200 Subject: [PATCH 08/10] improve redux action listener --- .../viewer/model/helpers/listener_helpers.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/viewer/model/helpers/listener_helpers.ts b/frontend/javascripts/viewer/model/helpers/listener_helpers.ts index 83b429e38c3..6e51c312656 100644 --- a/frontend/javascripts/viewer/model/helpers/listener_helpers.ts +++ b/frontend/javascripts/viewer/model/helpers/listener_helpers.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; import type { WebknossosState } from "viewer/store"; import Store from "viewer/store"; import { eventBus } from "./event_bus"; @@ -44,13 +44,15 @@ export function listenToStoreProperty( return Store.subscribe(handleChange); } -export function useReduxActionListener(actionType: string, callback: (action: any) => void) { +export function useReduxActionListener(actionType: string, callback: () => void) { + const callbackRef = useRef(callback); + callbackRef.current = callback; useEffect(() => { - const unsubscribe = eventBus.on(actionType, callback); + const unsubscribe = eventBus.on(actionType, callbackRef.current); return () => { unsubscribe(); }; - }, [actionType, callback]); + }, [actionType]); } export default {}; From 670591e4d74f1d7936c3f6a4cd83fc8f82961a42 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Mon, 28 Jul 2025 13:31:21 +0200 Subject: [PATCH 09/10] improve function return type --- .../viewer/model/actions/skeletontracing_actions.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx b/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx index 24cf77f8715..68ae473f179 100644 --- a/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx +++ b/frontend/javascripts/viewer/model/actions/skeletontracing_actions.tsx @@ -403,12 +403,11 @@ export const expandParentGroupsOfTreeAction = (tree: Tree) => tree, }) as const; -export const focusTreeAction = (tree: Tree) => { - return { +export const focusTreeAction = (tree: Tree) => + ({ type: "FOCUS_TREE", tree, - } as const; -}; + }) as const; export const setTreeVisibilityAction = (treeId: number | null | undefined, isVisible: boolean) => ({ From 624affed03a1267d13393dab9be4b79e9f2db048 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Tue, 29 Jul 2025 11:20:28 +0200 Subject: [PATCH 10/10] improve typing of middleware --- .../model/helpers/event_emitter_middleware.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts b/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts index 7c9c1072d38..cd7b385a1c5 100644 --- a/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts +++ b/frontend/javascripts/viewer/model/helpers/event_emitter_middleware.ts @@ -1,7 +1,11 @@ -import type { Middleware } from "redux"; +import type { Dispatch } from "redux"; +import type { Action } from "viewer/model/actions/actions"; import { eventBus } from "./event_bus"; -export const eventEmitterMiddleware: Middleware = () => (next) => (action) => { - eventBus.emit(action.type, action); - return next(action); -}; +export const eventEmitterMiddleware = + () => + (next: Dispatch) => + (action: A) => { + eventBus.emit(action.type, action); + return next(action); + };