diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 16cd926..0000000 --- a/TODO.md +++ /dev/null @@ -1,264 +0,0 @@ -# Activity/Sidebar/Editor Behavior Migration Plan - -## Goal -Create one shared behavior system for Activity Bar buttons, sidebar ownership, and editor/tab opening rules so all activities behave consistently by default. - -## Scope -- Renderer only (`src/renderer/**`) -- Activity Bar button behavior -- Sidebar switching/ownership -- Editor panel opening via tabs -- Tab policy for singleton tools (settings/tags/style/docs/etc.) - -## Non-goals -- Re-designing visuals -- Changing i18n copy beyond parity fixes -- Rewriting store architecture in one step - ---- - -## Current Behavior Exceptions (as-is) - -### A. Activity icon active-state exceptions -1. No known per-activity active-state exceptions in `ActivityBar`. - - All activity icons now use `sidebar-owner` active behavior. - -### B. Click behavior exceptions -1. No known per-activity click behavior exceptions in `ActivityBar`. - - Activity buttons now follow one shared posts-style switch/toggle pattern. -2. Sidebar section navigation (`SettingsNav`/`TagsNav`) now uses the shared section activation API. - -### C. Sidebar content exceptions -1. Sidebars use global Variant B conditional mounting. -2. `SettingsNav`/`TagsNav` section state is persisted for remount-safe UX. - -### D. Editor content exceptions -1. Editor is primarily tab-driven for tool views (`settings`, `tags`, `chat`, `import`, etc.). -2. Sidebar ownership (`activeView`) and editor content (active tab) are intentionally decoupled. -3. Some menu commands open tabs directly without harmonizing sidebar ownership. - -### E. API shape exceptions -1. Activity behavior logic is centralized via shared behavior + execution helpers. -2. Open-tab semantics are centralized in shared tab policy helpers by tab kind. - ---- - -## Decisions (confirmed) - -1. **Active strategy is unified** - - Keep only `sidebar-owner`. - - Remove `sidebar-or-tab` special case. - - Consequence: Settings icon is active only when Settings owns the visible sidebar. - -2. **Click behavior is unified to Posts behavior** - - All activity buttons follow the same switch/toggle sidebar logic pattern. - - `import`, `settings`, and `tags` should not have unique click semantics. - - If a view needs to open/ensure a tab, that responsibility moves outside button-click special cases (e.g., explicit nav/editor actions), not activity click branching. - -3. **Sidebar/editor decoupling stays** - - Sidebar owner (`activeView`) and editor content (active tab) remain intentionally independent. - - Menu actions opening editors directly are valid and remain supported. - -4. **Editor tab management should be centralized next** - - Add shared tab policy/behavior layer to remove per-editor inconsistencies. - -5. **API-shape exception outlook** - - Most API-shape exceptions should disappear as behavior is centralized. - - A few explicit policy descriptors will still remain (not exceptions): e.g., tab kind, transience defaults, singleton keys. - -6. **Unified section activation API is required** - - Introduce one shared section activation API for editor/tool views. - - Sidebars should trigger section activation through that API instead of ad-hoc per-view scroll/open logic. - -7. **Sidebar mounting strategy is Variant B (conditional mount)** - - Mount only the currently active sidebar; unmount inactive sidebars. - - Rationale: simplest and most consistent behavior model now. - - Performance tuning can be revisited later if real bottlenecks appear. - - Given expected limited content sizes, this is acceptable as default. - -8. **Sidebar section actions always activate/open corresponding editor** - - Clicking a sidebar section/navigation action must always ensure the matching editor tab is active. - - If the editor tab is not open, it should be auto-opened first, then section activation should run. - ---- - -## Proposed Target Architecture - -## 1) Activity Registry (single source of truth) -A declarative registry describing each activity: -- `id` -- `sidebarView` -- `placement` (`top`/`bottom`) -- `labelKey` (i18n) -- `activeStrategy` (`sidebar-owner` only) -- `clickStrategy` (single unified strategy for all activity buttons) - -## 2) Shared Activity Behavior Engine -Pure functions that compute: -- `isActivityActive(snapshot, activityId)` -- `getActivityClickActions(snapshot, activityId)` - -No UI code inside this engine. Output is action descriptors consumed by UI components. - -## 3) Reusable Render Adapters -- `ActivityBar` renders from registry + icon map -- `Sidebar` picks section from registry/sidebar view mapping -- Later: `Editor` tool-tab mapping can also be declared instead of chained conditionals - -## 4) Shared Tab Policy Layer (new) -Pure tab-policy helpers that decide: -- singleton vs multi-instance -- transient vs pinned defaults -- promotion rules (transient -> pinned) -- when tab-open should happen from sidebar actions vs explicit editor/menu actions - ---- - -## Sidebar Mounting Variants: Behavior Tradeoffs - -### Variant A — Keep-mounted sidebars (hidden with CSS) -**How it works**: all sidebar components stay mounted; inactive ones are hidden. - -**Benefits** -- Preserves in-component UI state (expanded sections, scroll position, local filters) without extra persistence code. -- Fast switching (no remount cost). - -**Costs** -- Higher memory usage. -- More background effects/subscriptions unless each component guards itself. -- Risk of inactive sidebars still doing work. - -### Variant B — Conditionally mounted sidebars (mount/unmount) -**How it works**: only current sidebar component is mounted. - -**Benefits** -- Lower memory/CPU baseline. -- Clear lifecycle boundaries and fewer hidden side effects. -- Simpler mental model for ownership. - -**Costs** -- Local UI state resets unless persisted explicitly. -- Potentially more reload work when switching back. - -### Variant C — Hybrid (current) -**How it works**: some sidebars keep-mounted, others conditional. - -**Benefits** -- Tactical optimization where needed. - -**Costs** -- Inconsistent behavior and harder debugging. -- More “why is this one different?” maintenance overhead. - -### Recommendation for consistency -- Pick **A** or **B** globally. -- If preserving local sidebar UI state is important, pick **A**. -- If simplicity + lower background work is the priority, pick **B**. -- Avoid **C** long-term. - ---- - -## Phased Migration Plan - -## Phase 0 — Baseline Contract Tests (safety net) -- Add behavior tests that lock target UX for all activities. -- Remove/replace tests that validate deprecated exceptions. -- Outcome: refactor-safe baseline. - -## Phase 1 — Extract Activity Behavior Core (start here) -- [x] Add shared activity behavior module + typed activity registry for activity buttons. -- [x] Migrate `ActivityBar` to use shared behavior engine. -- [x] Keep visible behavior unchanged. -- [x] Add/adjust tests for helper + `ActivityBar` integration. - -## Phase 1b — Enforce Unified Activity Rules (new) -- [x] Remove `sidebar-or-tab` from activity behavior. -- [x] Make settings active-state `sidebar-owner` only. -- [x] Unify click behavior for `import/settings/tags` to posts pattern. -- [x] Update/expand contract tests for unified activity behavior and `ActivityBar` integration. - -## Phase 2 — Normalize Sidebar Mounting + Ownership Mapping -- [x] Introduce a sidebar view registry mapping in one place. -- [x] Pick one global mounting strategy (A or B) and apply to all sidebars. -- [x] Add explicit state persistence for remount-safe sidebar section UX where needed. -- [x] Introduce unified section activation API and migrate `SettingsNav`/`TagsNav` to use it. - -## Phase 3 — Centralize Editor/Tab Management (expanded) -- [x] Implement shared tab policy layer for singleton tool tabs. -- [x] Centralize singleton tab policies (`settings`, `tags`, `style`, `documentation`, `metadata-diff`, `site-validation`). -- [x] Centralize post/media transient-vs-pinned open intent rules via shared entity-tab policy helper. -- [x] Remove duplicated tab-open logic from sidebars/nav/components for centralized policy-covered tab types. -- [x] Ensure site-validation tab reopens with persisted last report and refreshes only on explicit validation trigger. -- [x] Centralize multi-instance tab opening policies for `chat`, `import`, and `git-diff` (file/commit resources). - -### Editor Tab Management Variations (current analysis) -1. **Singleton, pinned tool tabs** - - `settings`, `tags`, `style`, `documentation` typically open as non-transient singleton tabs. -2. **Ephemeral/transient tabs with promotion** - - Post/media browsing flows can open transient tabs that become pinned on explicit user action (double-click or pin). -3. **Multi-instance tool tabs** - - `chat` and `import` create one tab per entity ID (conversation/definition), usually non-transient. -4. **Derived-ID utility tabs** - - `git-diff` uses structured IDs (`git-diff:` / commit variants), with special title derivation. -5. **Menu-opened utility tabs** - - `metadata-diff`, `site-validation`, and others are opened from menu commands with their own transience defaults. - -### Standardization target for tab management -- One registry describing for each tab type: - - `instanceMode`: `singleton | per-entity | per-resource` - - `defaultTransient`: `true | false` - - `promotable`: `true | false` - - `idStrategy`: canonical ID normalization/resolution - - `titleStrategy`: shared title resolver hooks - -## Phase 4 — Editor Routing Registry -- [x] Replace long `if/else` tab-type rendering chain with declarative tab->component mapping. -- [x] Keep special cases explicit (e.g., tab ID transforms such as git-diff commit/file parsing). - -## Phase 5 — Menu/Event Harmonization -- [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 -- [x] Reassess every exception in this file and decide keep/remove. -- [x] Remove dead paths and duplicate helpers. -- [x] Update tests and docs. - ---- - -## Decision Checklist (updated) -- [x] Remove `sidebar-or-tab`; use `sidebar-owner` only. -- [x] Unify activity click behavior to posts pattern for all activities. -- [x] Keep sidebar/editor decoupling. -- [x] Implement a unified section activation API for editor/tool sections. -- [x] Choose global sidebar mounting variant: **B conditional mount**. -- [x] Define sidebar action rule: section clicks always activate/open corresponding editor. -- [x] Migrate existing `SettingsNav`/`TagsNav` auto-open + delayed scroll to the unified section activation API that enforces this rule. - ---- - -## Work Log -- [x] Documented migration strategy and current exception matrix. -- [x] Phase 1 slice complete: shared activity behavior extracted and wired into `ActivityBar`. -- [x] Decisions captured: unified active strategy, unified click behavior, editor/sidebar decoupling retained. -- [x] Decision captured: unified section activation API required. -- [x] Decision captured: sidebar mounting strategy Variant B (conditional mount). -- [x] Decision captured: sidebar section actions always activate/open corresponding editor. -- [x] Phase 1b complete: removed `sidebar-or-tab`, unified activity click behavior, and updated tests. -- [x] Added `Edit Preferences` menu item (`CmdOrCtrl+,`) to open preferences editor directly. -- [x] Phase 2 slice complete: added shared section activation API and migrated `SettingsNav`/`TagsNav`. -- [x] Phase 2 slice complete: normalized posts/pages sidebar rendering to Variant B conditional mount. -- [x] Phase 2 slice complete: introduced canonical sidebar view registry and applied it across navigation/store/sidebar mapping. -- [x] Phase 2 slice complete: persisted settings/tags active section selection for remount-safe UX. -- [x] Phase 3 slice complete: added singleton tab policy helper and migrated menu/sidebar singleton tool openings to it. -- [x] Phase 3 slice complete: site validation report is persisted per project and reloaded by `SiteValidationView`. -- [x] Phase 3 slice complete: site validation view refreshes via explicit `bds:site-validation-updated` event (no auto-validate on mount). -- [x] Phase 3 continuation slice complete: added shared entity tab intent helper (`preview`/`pin`) for `post`/`media`. -- [x] Phase 3 continuation slice complete: migrated post/media open paths in Sidebar, Editor, LinkedMediaPanel, and Panel to shared entity-tab policy. -- [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. -- [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. -- [x] Phase 6 complete: removed unused activity behavior metadata/constants and refreshed the exception matrix to current architecture. -- [ ] Next implementation slice: optional follow-up (further simplification/perf tuning only if new drift appears). diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 654b8d1..d07f5c8 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -5,6 +5,7 @@ import { loadTabsForProject, saveTabsForProject } from './utils'; import { openSingletonToolTab } from './navigation/tabPolicy'; import { persistSiteValidationReport } from './navigation/siteValidationPersistence'; import { executeActivityClick } from './navigation/activityExecution'; +import { createAndFocusPost } from './navigation/postCreation'; import { ensureRendererPicoThemeStylesheet, getRendererPicoTheme } from './utils/picoTheme'; import { useI18n } from './i18n'; import './App.css'; @@ -186,14 +187,30 @@ const App: React.FC = () => { // Menu events unsubscribers.push( window.electronAPI?.on('menu:newPost', async () => { - const post = await window.electronAPI?.posts.create({ - title: '', - content: '', + const state = useAppStore.getState(); + await createAndFocusPost({ + createPost: async (input) => (await window.electronAPI?.posts.create(input)) as { id: string } | null | undefined, + setSelectedPost: state.setSelectedPost, + ensurePostsSidebar: () => { + const next = useAppStore.getState(); + executeActivityClick( + { + activeView: next.activeView, + sidebarVisible: next.sidebarVisible, + tabs: next.tabs, + activeTabId: next.activeTabId, + }, + 'posts', + { + setActiveView: next.setActiveView, + toggleSidebar: next.toggleSidebar, + }, + ); + }, + onError: (error) => { + console.error('Failed to create post:', error); + }, }); - if (post) { - setSelectedPost((post as PostData).id); - setActiveView('posts'); - } }) || (() => {}) ); diff --git a/src/renderer/components/Sidebar/Sidebar.tsx b/src/renderer/components/Sidebar/Sidebar.tsx index acc3369..d0483a9 100644 --- a/src/renderer/components/Sidebar/Sidebar.tsx +++ b/src/renderer/components/Sidebar/Sidebar.tsx @@ -9,6 +9,7 @@ import { scrollToTagsSection, TagsCategory } from '../TagsView'; import { activateSidebarSection } from '../../navigation/sectionActivation'; import { getPersistedSidebarSection, setPersistedSidebarSection } from '../../navigation/sidebarUiPersistence'; import { openChatTab, openEntityTab, openImportTab, openSingletonToolTab } from '../../navigation/tabPolicy'; +import { createAndFocusPost } from '../../navigation/postCreation'; import type { SidebarView } from '../../navigation/sidebarViewRegistry'; import { useI18n } from '../../i18n'; import './Sidebar.css'; @@ -743,21 +744,14 @@ const PostsList: React.FC = ({ mode, isActive }) => { }, [posts, searchQuery, selectedYear, selectedMonth, selectedTags, selectedCategories, isPagesMode]); const handleCreatePost = async () => { - // Create a real post immediately in the database with default empty content - try { - const { setSelectedPost: selectPost } = useAppStore.getState(); - const newPost = await window.electronAPI?.posts.create({ - title: '', - content: '', - tags: [], - categories: [], - }); - if (newPost) { - selectPost(newPost.id); - } - } catch (error) { - console.error('Failed to create post:', error); - } + const { setSelectedPost: selectPost } = useAppStore.getState(); + await createAndFocusPost({ + createPost: async (input) => (await window.electronAPI?.posts.create(input)) as { id: string } | null | undefined, + setSelectedPost: selectPost, + onError: (error) => { + console.error('Failed to create post:', error); + }, + }); }; const handleLoadMore = async () => { diff --git a/src/renderer/navigation/postCreation.ts b/src/renderer/navigation/postCreation.ts new file mode 100644 index 0000000..ecf71f0 --- /dev/null +++ b/src/renderer/navigation/postCreation.ts @@ -0,0 +1,33 @@ +export interface CreateAndFocusPostOptions { + createPost: (input: { + title: string; + content: string; + tags: string[]; + categories: string[]; + }) => Promise<{ id: string } | null | undefined>; + setSelectedPost: (postId: string) => void; + ensurePostsSidebar?: () => void; + onError?: (error: unknown) => void; +} + +export async function createAndFocusPost(options: CreateAndFocusPostOptions): Promise { + try { + const post = await options.createPost({ + title: '', + content: '', + tags: [], + categories: [], + }); + + if (!post) { + return null; + } + + options.setSelectedPost(post.id); + options.ensurePostsSidebar?.(); + return post.id; + } catch (error) { + options.onError?.(error); + return null; + } +} diff --git a/tests/renderer/navigation/duplicationGuards.test.ts b/tests/renderer/navigation/duplicationGuards.test.ts new file mode 100644 index 0000000..3cb3710 --- /dev/null +++ b/tests/renderer/navigation/duplicationGuards.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest'; +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; + +const root = path.resolve(__dirname, '../../..'); + +async function read(relativePath: string): Promise { + return readFile(path.join(root, relativePath), 'utf8'); +} + +describe('duplication guards for shared navigation features', () => { + it('keeps menu view actions on shared activity executor', async () => { + const app = await read('src/renderer/App.tsx'); + + expect(app).toContain("window.electronAPI?.on('menu:viewPosts'"); + expect(app).toContain("window.electronAPI?.on('menu:viewMedia'"); + expect(app).toContain('executeActivityClick('); + + expect(app).not.toMatch(/menu:viewPosts[\s\S]*setActiveView\('posts'\)/); + expect(app).not.toMatch(/menu:viewMedia[\s\S]*setActiveView\('media'\)/); + }); + + it('keeps activity bar execution on shared action executor', async () => { + const activityBar = await read('src/renderer/components/ActivityBar/ActivityBar.tsx'); + + expect(activityBar).toContain("from '../../navigation/activityExecution'"); + expect(activityBar).toContain('runActivityClick('); + + expect(activityBar).not.toContain('getActivityClickActions('); + expect(activityBar).not.toMatch(/for \(const action of actions\)/); + }); + + it('keeps sidebar and git sidebar tab openings on tabPolicy helpers', async () => { + const sidebar = await read('src/renderer/components/Sidebar/Sidebar.tsx'); + const gitSidebar = await read('src/renderer/components/GitSidebar/GitSidebar.tsx'); + + expect(sidebar).toContain('openChatTab('); + expect(sidebar).toContain('openImportTab('); + expect(sidebar).toContain('openEntityTab('); + + expect(sidebar).not.toMatch(/openTab\(\{\s*type:\s*'chat'/); + expect(sidebar).not.toMatch(/openTab\(\{\s*type:\s*'import'/); + + expect(gitSidebar).toContain('openGitDiffFileTab('); + expect(gitSidebar).toContain('openGitDiffCommitTab('); + expect(gitSidebar).not.toMatch(/openTab\(\{\s*type:\s*'git-diff'/); + }); + + it('keeps editor/tabbar routing on shared resolver and parser', async () => { + const editor = await read('src/renderer/components/Editor/Editor.tsx'); + const tabBar = await read('src/renderer/components/TabBar/TabBar.tsx'); + + expect(editor).toContain('resolveEditorRoute(activeTab)'); + expect(editor).not.toContain("activeTab?.type === 'settings'"); + expect(editor).not.toContain("activeTab?.type === 'git-diff'"); + + expect(tabBar).toContain('parseGitDiffTabId('); + expect(tabBar).not.toContain('getCommitHashFromGitDiffTabId'); + expect(tabBar).not.toContain('getGitDiffResource('); + }); + + it('keeps post creation on shared create-and-focus helper', async () => { + const app = await read('src/renderer/App.tsx'); + const sidebar = await read('src/renderer/components/Sidebar/Sidebar.tsx'); + + expect(app).toContain('createAndFocusPost('); + expect(sidebar).toContain('createAndFocusPost('); + + expect(app).not.toMatch(/menu:newPost[\s\S]*window\.electronAPI\?\.posts\.create\(\{/); + expect(sidebar).not.toMatch(/handleCreatePost[\s\S]*window\.electronAPI\?\.posts\.create\(\{/); + }); +}); diff --git a/tests/renderer/navigation/postCreation.test.ts b/tests/renderer/navigation/postCreation.test.ts new file mode 100644 index 0000000..178db7b --- /dev/null +++ b/tests/renderer/navigation/postCreation.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createAndFocusPost } from '../../../src/renderer/navigation/postCreation'; + +describe('postCreation', () => { + it('creates a post, focuses it, and optionally ensures posts sidebar', async () => { + const createPost = vi.fn().mockResolvedValue({ id: 'post-1' }); + const setSelectedPost = vi.fn(); + const ensurePostsSidebar = vi.fn(); + + const result = await createAndFocusPost({ + createPost, + setSelectedPost, + ensurePostsSidebar, + }); + + expect(createPost).toHaveBeenCalledWith({ + title: '', + content: '', + tags: [], + categories: [], + }); + expect(setSelectedPost).toHaveBeenCalledWith('post-1'); + expect(ensurePostsSidebar).toHaveBeenCalledTimes(1); + expect(result).toBe('post-1'); + }); + + it('returns null and does not focus when create returns nullish', async () => { + const createPost = vi.fn().mockResolvedValue(null); + const setSelectedPost = vi.fn(); + const ensurePostsSidebar = vi.fn(); + + const result = await createAndFocusPost({ + createPost, + setSelectedPost, + ensurePostsSidebar, + }); + + expect(setSelectedPost).not.toHaveBeenCalled(); + expect(ensurePostsSidebar).not.toHaveBeenCalled(); + expect(result).toBeNull(); + }); + + it('returns null and reports errors', async () => { + const createPost = vi.fn().mockRejectedValue(new Error('fail')); + const setSelectedPost = vi.fn(); + const onError = vi.fn(); + + const result = await createAndFocusPost({ + createPost, + setSelectedPost, + onError, + }); + + expect(setSelectedPost).not.toHaveBeenCalled(); + expect(onError).toHaveBeenCalledTimes(1); + expect(result).toBeNull(); + }); +});