From ea7c8ee1c7f8491778ed256504f2e84db49285d8 Mon Sep 17 00:00:00 2001 From: Connor Yoh Date: Fri, 22 Aug 2025 12:53:06 +0100 Subject: [PATCH] Circular dependencies with navigation fixes, types broken out --- .../tools/automate/AutomationCreation.tsx | 2 +- frontend/src/contexts/NavigationContext.tsx | 77 +++++++++-------- frontend/src/contexts/ToolWorkflowContext.tsx | 84 +++++++++++-------- frontend/src/hooks/useSuggestedTools.ts | 9 +- frontend/src/hooks/useToolManagement.tsx | 35 ++------ frontend/src/hooks/useUrlSync.ts | 2 +- frontend/src/types/navigation.ts | 42 ++++++++++ frontend/src/types/navigationActions.ts | 21 +++++ frontend/src/utils/urlRouting.ts | 23 ++--- 9 files changed, 173 insertions(+), 122 deletions(-) create mode 100644 frontend/src/types/navigation.ts create mode 100644 frontend/src/types/navigationActions.ts diff --git a/frontend/src/components/tools/automate/AutomationCreation.tsx b/frontend/src/components/tools/automate/AutomationCreation.tsx index 8126ebc65..49b12c396 100644 --- a/frontend/src/components/tools/automate/AutomationCreation.tsx +++ b/frontend/src/components/tools/automate/AutomationCreation.tsx @@ -27,7 +27,7 @@ interface AutomationCreationProps { export default function AutomationCreation({ mode, existingAutomation, onBack, onComplete, toolRegistry }: AutomationCreationProps) { const { t } = useTranslation(); - + const { automationName, setAutomationName, diff --git a/frontend/src/contexts/NavigationContext.tsx b/frontend/src/contexts/NavigationContext.tsx index c85eef158..532eb20bb 100644 --- a/frontend/src/contexts/NavigationContext.tsx +++ b/frontend/src/contexts/NavigationContext.tsx @@ -1,5 +1,6 @@ import React, { createContext, useContext, useReducer, useCallback } from 'react'; import { useNavigationUrlSync } from '../hooks/useUrlSync'; +import { ModeType, isValidMode, getDefaultMode } from '../types/navigation'; /** * NavigationContext - Complete navigation management system @@ -9,32 +10,13 @@ import { useNavigationUrlSync } from '../hooks/useUrlSync'; * maintain clear separation of concerns. */ -// Navigation mode types - complete list to match fileContext.ts -export type ModeType = - | 'viewer' - | 'pageEditor' - | 'fileEditor' - | 'merge' - | 'split' - | 'compress' - | 'ocr' - | 'convert' - | 'sanitize' - | 'addPassword' - | 'changePermissions' - | 'addWatermark' - | 'removePassword' - | 'single-large-page' - | 'repair' - | 'unlockPdfForms' - | 'removeCertificateSign'; - // Navigation state interface NavigationState { currentMode: ModeType; hasUnsavedChanges: boolean; pendingNavigation: (() => void) | null; showNavigationWarning: boolean; + selectedToolKey: string | null; // Add tool selection to navigation state } // Navigation actions @@ -42,7 +24,8 @@ type NavigationAction = | { type: 'SET_MODE'; payload: { mode: ModeType } } | { type: 'SET_UNSAVED_CHANGES'; payload: { hasChanges: boolean } } | { type: 'SET_PENDING_NAVIGATION'; payload: { navigationFn: (() => void) | null } } - | { type: 'SHOW_NAVIGATION_WARNING'; payload: { show: boolean } }; + | { type: 'SHOW_NAVIGATION_WARNING'; payload: { show: boolean } } + | { type: 'SET_SELECTED_TOOL'; payload: { toolKey: string | null } }; // Navigation reducer const navigationReducer = (state: NavigationState, action: NavigationAction): NavigationState => { @@ -59,6 +42,9 @@ const navigationReducer = (state: NavigationState, action: NavigationAction): Na case 'SHOW_NAVIGATION_WARNING': return { ...state, showNavigationWarning: action.payload.show }; + case 'SET_SELECTED_TOOL': + return { ...state, selectedToolKey: action.payload.toolKey }; + default: return state; } @@ -66,10 +52,11 @@ const navigationReducer = (state: NavigationState, action: NavigationAction): Na // Initial state const initialState: NavigationState = { - currentMode: 'pageEditor', + currentMode: getDefaultMode(), hasUnsavedChanges: false, pendingNavigation: null, - showNavigationWarning: false + showNavigationWarning: false, + selectedToolKey: null }; // Navigation context actions interface @@ -80,6 +67,9 @@ export interface NavigationContextActions { requestNavigation: (navigationFn: () => void) => void; confirmNavigation: () => void; cancelNavigation: () => void; + selectTool: (toolKey: string) => void; + clearToolSelection: () => void; + handleToolSelect: (toolId: string) => void; } // Split context values @@ -88,6 +78,7 @@ export interface NavigationContextStateValue { hasUnsavedChanges: boolean; pendingNavigation: (() => void) | null; showNavigationWarning: boolean; + selectedToolKey: string | null; } export interface NavigationContextActionsValue { @@ -145,6 +136,31 @@ export const NavigationProvider: React.FC<{ // Clear navigation without executing dispatch({ type: 'SET_PENDING_NAVIGATION', payload: { navigationFn: null } }); dispatch({ type: 'SHOW_NAVIGATION_WARNING', payload: { show: false } }); + }, []), + + selectTool: useCallback((toolKey: string) => { + dispatch({ type: 'SET_SELECTED_TOOL', payload: { toolKey } }); + }, []), + + clearToolSelection: useCallback(() => { + dispatch({ type: 'SET_SELECTED_TOOL', payload: { toolKey: null } }); + }, []), + + handleToolSelect: useCallback((toolId: string) => { + // Handle special cases + if (toolId === 'allTools') { + dispatch({ type: 'SET_SELECTED_TOOL', payload: { toolKey: null } }); + return; + } + + // Special-case: if tool is a dedicated reader tool, enter reader mode + if (toolId === 'read' || toolId === 'view-pdf') { + dispatch({ type: 'SET_SELECTED_TOOL', payload: { toolKey: null } }); + return; + } + + dispatch({ type: 'SET_SELECTED_TOOL', payload: { toolKey: toolId } }); + dispatch({ type: 'SET_MODE', payload: { mode: 'fileEditor' as ModeType } }); }, []) }; @@ -152,7 +168,8 @@ export const NavigationProvider: React.FC<{ currentMode: state.currentMode, hasUnsavedChanges: state.hasUnsavedChanges, pendingNavigation: state.pendingNavigation, - showNavigationWarning: state.showNavigationWarning + showNavigationWarning: state.showNavigationWarning, + selectedToolKey: state.selectedToolKey }; const actionsValue: NavigationContextActionsValue = { @@ -212,16 +229,8 @@ export const useNavigationGuard = () => { }; }; -// Utility functions for mode handling -export const isValidMode = (mode: string): mode is ModeType => { - const validModes: ModeType[] = [ - 'viewer', 'pageEditor', 'fileEditor', 'merge', 'split', - 'compress', 'ocr', 'convert', 'addPassword', 'changePermissions', 'sanitize' - ]; - return validModes.includes(mode as ModeType); -}; - -export const getDefaultMode = (): ModeType => 'pageEditor'; +// Re-export utility functions from types for backward compatibility +export { isValidMode, getDefaultMode, type ModeType } from '../types/navigation'; // TODO: This will be expanded for URL-based routing system // - URL parsing utilities diff --git a/frontend/src/contexts/ToolWorkflowContext.tsx b/frontend/src/contexts/ToolWorkflowContext.tsx index cd273829a..ca91daa2a 100644 --- a/frontend/src/contexts/ToolWorkflowContext.tsx +++ b/frontend/src/contexts/ToolWorkflowContext.tsx @@ -8,7 +8,7 @@ import { useToolManagement } from '../hooks/useToolManagement'; import { PageEditorFunctions } from '../types/pageEditor'; import { ToolRegistryEntry } from '../data/toolsTaxonomy'; import { useToolWorkflowUrlSync } from '../hooks/useUrlSync'; -import { useNavigationActions } from './NavigationContext'; +import { useNavigationActions, useNavigationState } from './NavigationContext'; // State interface interface ToolWorkflowState { @@ -106,21 +106,18 @@ interface ToolWorkflowProviderProps { export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) { const [state, dispatch] = useReducer(toolWorkflowReducer, initialState); - // File context for view changes + // Navigation actions and state are available since we're inside NavigationProvider const { actions } = useNavigationActions(); - // Wrapper to convert string to ModeType - const handleViewChange = (view: string) => { - actions.setMode(view as any); // ToolWorkflowContext should validate this - }; - + const navigationState = useNavigationState(); + // Tool management hook const { - selectedToolKey, - selectedTool, toolRegistry, - selectTool, - clearToolSelection, + getSelectedTool, } = useToolManagement(); + + // Get selected tool from navigation context + const selectedTool = getSelectedTool(navigationState.selectedToolKey); // UI Action creators const setSidebarsVisible = useCallback((visible: boolean) => { @@ -149,28 +146,27 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) { // Workflow actions (compound actions that coordinate multiple state changes) const handleToolSelect = useCallback((toolId: string) => { - // Special-case: if tool is a dedicated reader tool, enter reader mode and do not go to toolContent - if (toolId === 'read' || toolId === 'view-pdf') { - setReaderMode(true); - setLeftPanelView('toolPicker'); - clearToolSelection(); - setSearchQuery(''); - return; - } - - selectTool(toolId); - handleViewChange('fileEditor' as any); // ToolWorkflowContext should validate this - setLeftPanelView('toolContent'); - setReaderMode(false); - // Clear search so the tool content becomes visible immediately + actions.handleToolSelect(toolId); + + // Clear search query when selecting a tool setSearchQuery(''); - }, [selectTool, handleViewChange, setLeftPanelView, setReaderMode, setSearchQuery, clearToolSelection]); + + // Handle view switching logic + if (toolId === 'allTools' || toolId === 'read' || toolId === 'view-pdf') { + setLeftPanelView('toolPicker'); + if (toolId === 'read' || toolId === 'view-pdf') { + setReaderMode(true); + } + } else { + setLeftPanelView('toolContent'); + } + }, [actions, setLeftPanelView, setReaderMode, setSearchQuery]); const handleBackToTools = useCallback(() => { setLeftPanelView('toolPicker'); setReaderMode(false); - clearToolSelection(); - }, [setLeftPanelView, setReaderMode, clearToolSelection]); + actions.clearToolSelection(); + }, [setLeftPanelView, setReaderMode, actions]); const handleReaderToggle = useCallback(() => { setReaderMode(true); @@ -190,13 +186,13 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) { ); // Enable URL synchronization for tool selection - useToolWorkflowUrlSync(selectedToolKey, selectTool, clearToolSelection, true); + useToolWorkflowUrlSync(navigationState.selectedToolKey, actions.selectTool, actions.clearToolSelection, true); - // Simple context value with basic memoization - const contextValue : ToolWorkflowContextValue ={ + // Properly memoized context value + const contextValue = useMemo((): ToolWorkflowContextValue => ({ // State ...state, - selectedToolKey, + selectedToolKey: navigationState.selectedToolKey, selectedTool, toolRegistry, @@ -207,8 +203,8 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) { setPreviewFile, setPageEditorFunctions, setSearchQuery, - selectTool, - clearToolSelection, + selectTool: actions.selectTool, + clearToolSelection: actions.clearToolSelection, // Workflow Actions handleToolSelect, @@ -218,7 +214,25 @@ export function ToolWorkflowProvider({ children }: ToolWorkflowProviderProps) { // Computed filteredTools, isPanelVisible, - }; + }), [ + state, + navigationState.selectedToolKey, + selectedTool, + toolRegistry, + setSidebarsVisible, + setLeftPanelView, + setReaderMode, + setPreviewFile, + setPageEditorFunctions, + setSearchQuery, + actions.selectTool, + actions.clearToolSelection, + handleToolSelect, + handleBackToTools, + handleReaderToggle, + filteredTools, + isPanelVisible, + ]); return ( diff --git a/frontend/src/hooks/useSuggestedTools.ts b/frontend/src/hooks/useSuggestedTools.ts index 1216650c7..d9e218451 100644 --- a/frontend/src/hooks/useSuggestedTools.ts +++ b/frontend/src/hooks/useSuggestedTools.ts @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import { useToolWorkflow } from '../contexts/ToolWorkflowContext'; +import { useNavigationActions, useNavigationState } from '../contexts/NavigationContext'; // Material UI Icons import CompressIcon from '@mui/icons-material/Compress'; @@ -44,7 +44,8 @@ const ALL_SUGGESTED_TOOLS: Omit[] = [ ]; export function useSuggestedTools(): SuggestedTool[] { - const { handleToolSelect, selectedToolKey } = useToolWorkflow(); + const { actions } = useNavigationActions(); + const { selectedToolKey } = useNavigationState(); return useMemo(() => { // Filter out the current tool @@ -53,7 +54,7 @@ export function useSuggestedTools(): SuggestedTool[] { // Add navigation function to each tool return filteredTools.map(tool => ({ ...tool, - navigate: () => handleToolSelect(tool.name) + navigate: () => actions.handleToolSelect(tool.name) })); - }, [selectedToolKey, handleToolSelect]); + }, [selectedToolKey, actions]); } diff --git a/frontend/src/hooks/useToolManagement.tsx b/frontend/src/hooks/useToolManagement.tsx index e991677a9..9ea048f65 100644 --- a/frontend/src/hooks/useToolManagement.tsx +++ b/frontend/src/hooks/useToolManagement.tsx @@ -5,19 +5,16 @@ import { getAllEndpoints, type ToolRegistryEntry } from "../data/toolsTaxonomy"; import { useMultipleEndpointsEnabled } from "./useEndpointConfig"; interface ToolManagementResult { - selectedToolKey: string | null; selectedTool: ToolRegistryEntry | null; toolSelectedFileIds: string[]; toolRegistry: Record; - selectTool: (toolKey: string) => void; - clearToolSelection: () => void; setToolSelectedFileIds: (fileIds: string[]) => void; + getSelectedTool: (toolKey: string | null) => ToolRegistryEntry | null; } export const useToolManagement = (): ToolManagementResult => { const { t } = useTranslation(); - const [selectedToolKey, setSelectedToolKey] = useState(null); const [toolSelectedFileIds, setToolSelectedFileIds] = useState([]); // Build endpoints list from registry entries with fallback to legacy mapping @@ -56,35 +53,15 @@ export const useToolManagement = (): ToolManagementResult => { return availableToolRegistry; }, [isToolAvailable, t, baseRegistry]); - useEffect(() => { - if (!endpointsLoading && selectedToolKey && !toolRegistry[selectedToolKey]) { - const firstAvailableTool = Object.keys(toolRegistry)[0]; - if (firstAvailableTool) { - setSelectedToolKey(firstAvailableTool); - } else { - setSelectedToolKey(null); - } - } - }, [endpointsLoading, selectedToolKey, toolRegistry]); - - const selectTool = useCallback((toolKey: string) => { - setSelectedToolKey(toolKey); - }, []); - - const clearToolSelection = useCallback(() => { - setSelectedToolKey(null); - }, []); - - const selectedTool = selectedToolKey ? toolRegistry[selectedToolKey] : null; + const getSelectedTool = useCallback((toolKey: string | null): ToolRegistryEntry | null => { + return toolKey ? toolRegistry[toolKey] || null : null; + }, [toolRegistry]); return { - selectedToolKey, - selectedTool, + selectedTool: getSelectedTool(null), // This will be unused, kept for compatibility toolSelectedFileIds, toolRegistry, - selectTool, - clearToolSelection, setToolSelectedFileIds, - + getSelectedTool, }; }; diff --git a/frontend/src/hooks/useUrlSync.ts b/frontend/src/hooks/useUrlSync.ts index ef181c04b..4a50a36ba 100644 --- a/frontend/src/hooks/useUrlSync.ts +++ b/frontend/src/hooks/useUrlSync.ts @@ -3,7 +3,7 @@ */ import { useEffect, useCallback } from 'react'; -import { ModeType } from '../contexts/NavigationContext'; +import { ModeType } from '../types/navigation'; import { parseToolRoute, updateToolRoute, clearToolRoute } from '../utils/urlRouting'; /** diff --git a/frontend/src/types/navigation.ts b/frontend/src/types/navigation.ts new file mode 100644 index 000000000..61aa24cc3 --- /dev/null +++ b/frontend/src/types/navigation.ts @@ -0,0 +1,42 @@ +/** + * Shared navigation types to avoid circular dependencies + */ + +// Navigation mode types - complete list to match contexts +export type ModeType = + | 'viewer' + | 'pageEditor' + | 'fileEditor' + | 'merge' + | 'split' + | 'compress' + | 'ocr' + | 'convert' + | 'sanitize' + | 'addPassword' + | 'changePermissions' + | 'addWatermark' + | 'removePassword' + | 'single-large-page' + | 'repair' + | 'unlockPdfForms' + | 'removeCertificateSign'; + +// Utility functions for mode handling +export const isValidMode = (mode: string): mode is ModeType => { + const validModes: ModeType[] = [ + 'viewer', 'pageEditor', 'fileEditor', 'merge', 'split', + 'compress', 'ocr', 'convert', 'addPassword', 'changePermissions', + 'sanitize', 'addWatermark', 'removePassword', 'single-large-page', + 'repair', 'unlockPdfForms', 'removeCertificateSign' + ]; + return validModes.includes(mode as ModeType); +}; + +export const getDefaultMode = (): ModeType => 'pageEditor'; + +// Route parsing result +export interface ToolRoute { + mode: ModeType; + toolKey: string | null; +} \ No newline at end of file diff --git a/frontend/src/types/navigationActions.ts b/frontend/src/types/navigationActions.ts new file mode 100644 index 000000000..b227dac9d --- /dev/null +++ b/frontend/src/types/navigationActions.ts @@ -0,0 +1,21 @@ +/** + * Navigation action interfaces to break circular dependencies + */ + +import { ModeType } from './navigation'; + +export interface NavigationActions { + setMode: (mode: ModeType) => void; + setHasUnsavedChanges: (hasChanges: boolean) => void; + showNavigationWarning: (show: boolean) => void; + requestNavigation: (navigationFn: () => void) => void; + confirmNavigation: () => void; + cancelNavigation: () => void; +} + +export interface NavigationState { + currentMode: ModeType; + hasUnsavedChanges: boolean; + pendingNavigation: (() => void) | null; + showNavigationWarning: boolean; +} \ No newline at end of file diff --git a/frontend/src/utils/urlRouting.ts b/frontend/src/utils/urlRouting.ts index fbf6c1c3c..05493423a 100644 --- a/frontend/src/utils/urlRouting.ts +++ b/frontend/src/utils/urlRouting.ts @@ -3,12 +3,7 @@ * Provides clean URL routing for the V2 tool system */ -import { ModeType } from '../contexts/NavigationContext'; - -export interface ToolRoute { - mode: ModeType; - toolKey?: string; -} +import { ModeType, isValidMode as isValidModeType, getDefaultMode, ToolRoute } from '../types/navigation'; /** * Parse the current URL to extract tool routing information @@ -45,7 +40,7 @@ export function parseToolRoute(): ToolRoute { // Check for query parameter fallback (e.g., ?tool=split) const toolParam = searchParams.get('tool'); - if (toolParam && isValidMode(toolParam)) { + if (toolParam && isValidModeType(toolParam)) { return { mode: toolParam as ModeType, toolKey: toolParam @@ -54,7 +49,8 @@ export function parseToolRoute(): ToolRoute { // Default to page editor for home page return { - mode: 'pageEditor' + mode: getDefaultMode(), + toolKey: null }; } @@ -137,16 +133,7 @@ export function getToolDisplayName(toolKey: string): string { return displayNames[toolKey] || toolKey; } -/** - * Check if a mode is valid - */ -function isValidMode(mode: string): mode is ModeType { - const validModes: ModeType[] = [ - 'viewer', 'pageEditor', 'fileEditor', 'merge', 'split', - 'compress', 'ocr', 'convert', 'addPassword', 'changePermissions', 'sanitize' - ]; - return validModes.includes(mode as ModeType); -} +// Note: isValidMode is now imported from types/navigation.ts /** * Generate shareable URL for current tool state