From d70b13952aaa74e2c503c31c2c39fda39aa8308d Mon Sep 17 00:00:00 2001 From: hugo Date: Sat, 21 Feb 2026 18:17:35 +0100 Subject: [PATCH] feat: phase 5 refactor --- TODO.md | 8 ++-- src/renderer/App.tsx | 31 +++++++++++++- .../components/ActivityBar/ActivityBar.tsx | 15 +++---- src/renderer/navigation/activityExecution.ts | 34 +++++++++++++++ .../navigation/activityExecution.test.ts | 41 +++++++++++++++++++ 5 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 src/renderer/navigation/activityExecution.ts create mode 100644 tests/renderer/navigation/activityExecution.test.ts diff --git a/TODO.md b/TODO.md index 115dabb..db1a1e9 100644 --- a/TODO.md +++ b/TODO.md @@ -217,8 +217,8 @@ Pure tab-policy helpers that decide: - [x] Keep special cases explicit (e.g., tab ID transforms such as git-diff commit/file parsing). ## Phase 5 — Menu/Event Harmonization -- Route menu-driven view/tab actions through the same behavior layer where applicable. -- Reduce drift between keyboard/menu/toolbar interactions. +- [x] Route menu-driven view/tab actions through the same behavior layer where applicable. +- [x] Reduce drift between keyboard/menu/toolbar interactions. ## Phase 6 — Cleanup + Exception Review - Reassess every exception in this file and decide keep/remove. @@ -259,4 +259,6 @@ Pure tab-policy helpers that decide: - [x] Phase 3 completion slice: centralized `chat` and `import` tab-open specs and migrated sidebar call sites. - [x] Phase 3 completion slice: centralized `git-diff` file/commit ID/spec/open helpers and reused shared parsing in `TabBar`. - [x] Phase 4 complete: introduced `editorRouting` registry/resolver and migrated `Editor` to declarative route->view rendering. -- [ ] Next implementation slice: Phase 5 (menu/event harmonization). +- [x] Phase 5 complete: introduced shared activity action executor and reused it in `ActivityBar` and menu view handlers. +- [x] Phase 5 complete: `menu:viewPosts` and `menu:viewMedia` now follow the same action pipeline as activity-bar clicks. +- [ ] Next implementation slice: Phase 6 (cleanup + exception review). diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 19eba8c..654b8d1 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -4,6 +4,7 @@ import { useAppStore, PostData, MediaData, TaskProgress } from './store'; import { loadTabsForProject, saveTabsForProject } from './utils'; import { openSingletonToolTab } from './navigation/tabPolicy'; import { persistSiteValidationReport } from './navigation/siteValidationPersistence'; +import { executeActivityClick } from './navigation/activityExecution'; import { ensureRendererPicoThemeStylesheet, getRendererPicoTheme } from './utils/picoTheme'; import { useI18n } from './i18n'; import './App.css'; @@ -216,13 +217,39 @@ const App: React.FC = () => { unsubscribers.push( window.electronAPI?.on('menu:viewPosts', () => { - setActiveView('posts'); + const state = useAppStore.getState(); + executeActivityClick( + { + activeView: state.activeView, + sidebarVisible: state.sidebarVisible, + tabs: state.tabs, + activeTabId: state.activeTabId, + }, + 'posts', + { + setActiveView: state.setActiveView, + toggleSidebar: state.toggleSidebar, + }, + ); }) || (() => {}) ); unsubscribers.push( window.electronAPI?.on('menu:viewMedia', () => { - setActiveView('media'); + const state = useAppStore.getState(); + executeActivityClick( + { + activeView: state.activeView, + sidebarVisible: state.sidebarVisible, + tabs: state.tabs, + activeTabId: state.activeTabId, + }, + 'media', + { + setActiveView: state.setActiveView, + toggleSidebar: state.toggleSidebar, + }, + ); }) || (() => {}) ); diff --git a/src/renderer/components/ActivityBar/ActivityBar.tsx b/src/renderer/components/ActivityBar/ActivityBar.tsx index d1d88d4..8b113d2 100644 --- a/src/renderer/components/ActivityBar/ActivityBar.tsx +++ b/src/renderer/components/ActivityBar/ActivityBar.tsx @@ -2,12 +2,12 @@ import React from 'react'; import { useAppStore } from '../../store'; import { useI18n } from '../../i18n'; import { - getActivityClickActions, getActivityConfig, isActivityActive, type ActivityId, type ActivitySnapshot, } from '../../navigation/activityBehavior'; +import { executeActivityClick as runActivityClick } from '../../navigation/activityExecution'; import './ActivityBar.css'; // Simple SVG icons @@ -74,15 +74,10 @@ export const ActivityBar: React.FC = () => { }; const executeActivityClick = (activityId: ActivityId) => { - const actions = getActivityClickActions(snapshot, activityId); - - for (const action of actions) { - if (action.type === 'toggleSidebar') { - toggleSidebar(); - } else if (action.type === 'setActiveView') { - setActiveView(action.view); - } - } + runActivityClick(snapshot, activityId, { + toggleSidebar, + setActiveView, + }); }; const getTitle = (activityId: ActivityId) => `${t(getActivityConfig(activityId).labelKey)} ${t('activity.toggleHint')}`; diff --git a/src/renderer/navigation/activityExecution.ts b/src/renderer/navigation/activityExecution.ts new file mode 100644 index 0000000..bb1b60a --- /dev/null +++ b/src/renderer/navigation/activityExecution.ts @@ -0,0 +1,34 @@ +import { + getActivityClickActions, + type ActivityAction, + type ActivityId, + type ActivitySnapshot, +} from './activityBehavior'; +import type { SidebarView } from './sidebarViewRegistry'; + +export interface ActivityExecutionHandlers { + setActiveView: (view: SidebarView) => void; + toggleSidebar: () => void; +} + +export function executeActivityClickActions( + actions: ActivityAction[], + handlers: ActivityExecutionHandlers, +): void { + for (const action of actions) { + if (action.type === 'toggleSidebar') { + handlers.toggleSidebar(); + continue; + } + + handlers.setActiveView(action.view); + } +} + +export function executeActivityClick( + snapshot: ActivitySnapshot, + activityId: ActivityId, + handlers: ActivityExecutionHandlers, +): void { + executeActivityClickActions(getActivityClickActions(snapshot, activityId), handlers); +} diff --git a/tests/renderer/navigation/activityExecution.test.ts b/tests/renderer/navigation/activityExecution.test.ts new file mode 100644 index 0000000..801a309 --- /dev/null +++ b/tests/renderer/navigation/activityExecution.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + executeActivityClick, + executeActivityClickActions, +} from '../../../src/renderer/navigation/activityExecution'; +import type { ActivityAction, ActivitySnapshot } from '../../../src/renderer/navigation/activityBehavior'; + +const snapshot: ActivitySnapshot = { + activeView: 'media', + sidebarVisible: false, + tabs: [], + activeTabId: null, +}; + +describe('activityExecution', () => { + it('applies action descriptors in order', () => { + const actions: ActivityAction[] = [ + { type: 'setActiveView', view: 'posts' }, + { type: 'toggleSidebar' }, + ]; + + const setActiveView = vi.fn(); + const toggleSidebar = vi.fn(); + + executeActivityClickActions(actions, { setActiveView, toggleSidebar }); + + expect(setActiveView).toHaveBeenCalledWith('posts'); + expect(toggleSidebar).toHaveBeenCalledTimes(1); + expect(setActiveView.mock.invocationCallOrder[0]).toBeLessThan(toggleSidebar.mock.invocationCallOrder[0]); + }); + + it('executes canonical click behavior from activity snapshot', () => { + const setActiveView = vi.fn(); + const toggleSidebar = vi.fn(); + + executeActivityClick(snapshot, 'posts', { setActiveView, toggleSidebar }); + + expect(setActiveView).toHaveBeenCalledWith('posts'); + expect(toggleSidebar).toHaveBeenCalledTimes(1); + }); +});