fix(sign): prevent custom workbench auto-revert when opening sign editor

`unregisterCustomWorkbenchView` depended on `[actions]` from
NavigationContext. Because `actions` gets a new reference every time
`workbench` changes (setWorkbench captures state.workbench in its
closure), the registration effect in CombinedSign re-ran on each
workbench update, firing cleanup → clearCustomWorkbenchViewData →
auto-revert effect reverted the workbench back to default.

Fix: introduce `actionsRef` (updated each render, stable identity) and
use it in `unregisterCustomWorkbenchView` (now has empty dep array) and
the auto-revert effect (removes `actions` from deps). Also cleaned up
diagnostic console.log calls added during debugging.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Connor Yoh
2026-03-27 17:31:35 +00:00
co-authored by Claude Sonnet 4.6
parent a651061554
commit e194fc7e36
2 changed files with 13 additions and 26 deletions
@@ -114,12 +114,6 @@ export const NavigationProvider: React.FC<{
const setWorkbench = useCallback((workbench: WorkbenchType) => {
// Check for unsaved changes using registered checker or state
const hasUnsavedChanges = unsavedChangesCheckerRef.current?.() || state.hasUnsavedChanges;
console.log('[NavigationContext] setWorkbench:', {
from: state.workbench,
to: workbench,
hasChecker: !!unsavedChangesCheckerRef.current,
hasUnsavedChanges
});
// If we're leaving pageEditor, viewer, or custom workbench and have unsaved changes, request navigation
const leavingWorkbenchWithChanges =
@@ -134,16 +128,9 @@ export const NavigationProvider: React.FC<{
}
const performWorkbenchChange = () => {
// When leaving a custom workbench, clear the selected tool
console.log('[NavigationContext] performWorkbenchChange executing', {
from: state.workbench,
to: workbench,
isCustom: state.workbench.startsWith('custom:')
});
if (state.workbench.startsWith('custom:')) {
console.log('[NavigationContext] Clearing tool and changing workbench to:', workbench);
dispatch({ type: 'SET_TOOL_AND_WORKBENCH', payload: { toolId: null, workbench } });
} else {
console.log('[NavigationContext] Just changing workbench to:', workbench);
dispatch({ type: 'SET_WORKBENCH', payload: { workbench } });
}
};
@@ -206,19 +193,13 @@ export const NavigationProvider: React.FC<{
}, [state.hasUnsavedChanges]);
const confirmNavigation = useCallback(() => {
console.log('[NavigationContext] confirmNavigation called', {
hasPendingNav: !!state.pendingNavigation,
currentWorkbench: state.workbench,
currentTool: state.selectedTool
});
if (state.pendingNavigation) {
state.pendingNavigation();
}
dispatch({ type: 'SET_PENDING_NAVIGATION', payload: { navigationFn: null } });
dispatch({ type: 'SHOW_NAVIGATION_WARNING', payload: { show: false } });
console.log('[NavigationContext] confirmNavigation completed');
}, [state.pendingNavigation, state.workbench, state.selectedTool]);
}, [state.pendingNavigation]);
const cancelNavigation = useCallback(() => {
dispatch({ type: 'SET_PENDING_NAVIGATION', payload: { navigationFn: null } });
@@ -3,7 +3,7 @@
* Eliminates prop drilling with a single, simple context
*/
import React, { createContext, useContext, useReducer, useCallback, useMemo, useEffect } from 'react';
import React, { createContext, useContext, useReducer, useCallback, useMemo, useEffect, useRef } from 'react';
import { useToolManagement, type ToolAvailabilityMap } from '@app/hooks/useToolManagement';
import { PageEditorFunctions } from '@app/types/pageEditor';
import { ToolRegistryEntry, ToolRegistry } from '@app/data/toolsTaxonomy';
@@ -117,6 +117,12 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) {
// Navigation actions and state are available since we're inside NavigationProvider
const { actions } = useNavigationActions();
const navigationState = useNavigationState();
const workbenchRef = useRef(navigationState.workbench);
const actionsRef = useRef(actions);
useEffect(() => {
workbenchRef.current = navigationState.workbench;
actionsRef.current = actions;
});
// Tool management hook
const { toolRegistry, getSelectedTool, toolAvailability } = useToolManagement();
@@ -198,10 +204,10 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) {
return updated;
});
if (removedView && navigationState.workbench === removedView.workbenchId) {
actions.setWorkbench(getDefaultWorkbench());
if (removedView && workbenchRef.current === removedView.workbenchId) {
actionsRef.current.setWorkbench(getDefaultWorkbench());
}
}, [actions, navigationState.workbench]);
}, []); // stable — uses refs for mutable values
const setCustomWorkbenchViewData = useCallback((id: string, dataOrUpdater: any | ((prev: any) => any)) => {
setCustomViewData(prev => {
@@ -242,9 +248,9 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) {
const currentCustomView = customWorkbenchViews.find(view => view.workbenchId === navigationState.workbench);
if (!currentCustomView || currentCustomView.data == null) {
actions.setWorkbench(getDefaultWorkbench());
actionsRef.current.setWorkbench(getDefaultWorkbench());
}
}, [actions, customWorkbenchViews, navigationState.workbench, navigationState.pendingNavigation, navigationState.showNavigationWarning]);
}, [customWorkbenchViews, navigationState.workbench, navigationState.pendingNavigation, navigationState.showNavigationWarning]);
// Persisted via PreferencesContext; no direct localStorage writes needed here