From 08ef72a802ce52ab5e2cb6b508eacb71075e5480 Mon Sep 17 00:00:00 2001 From: Georg Bauer Date: Wed, 4 Mar 2026 09:28:20 +0100 Subject: [PATCH] fix: unified handling of editor reloading (#32) Co-authored-by: hugo --- .../components/ChatPanel/ChatPanel.tsx | 49 ++-- src/renderer/components/Editor/Editor.tsx | 57 ++-- .../ImportAnalysisView/ImportAnalysisView.tsx | 52 ++-- .../MenuEditorView/MenuEditorView.tsx | 5 +- .../MetadataDiffPanel/MetadataDiffPanel.tsx | 5 +- .../components/ScriptsView/ScriptsView.tsx | 162 +++++------ src/renderer/components/TagsView/TagsView.tsx | 10 +- .../TemplatesView/TemplatesView.tsx | 92 +++---- src/renderer/navigation/useEntityEditor.ts | 108 ++++++++ .../components/EditorAutoSaveCursor.test.tsx | 1 + .../EditorDashboardTimeline.test.tsx | 1 + .../EditorMetadataCollapse.test.tsx | 1 + .../EditorVisualModePersistence.test.tsx | 1 + .../components/MenuEditorView.test.tsx | 5 + .../renderer/components/ScriptsView.test.tsx | 27 ++ tests/renderer/components/TagsView.test.tsx | 9 + .../components/TemplatesView.test.tsx | 27 ++ .../navigation/chatSurfaceUsageGuards.test.ts | 2 +- .../navigation/useEntityEditor.test.ts | 258 ++++++++++++++++++ 19 files changed, 633 insertions(+), 239 deletions(-) create mode 100644 src/renderer/navigation/useEntityEditor.ts create mode 100644 tests/renderer/navigation/useEntityEditor.test.ts diff --git a/src/renderer/components/ChatPanel/ChatPanel.tsx b/src/renderer/components/ChatPanel/ChatPanel.tsx index 7ff41a2..4380a77 100644 --- a/src/renderer/components/ChatPanel/ChatPanel.tsx +++ b/src/renderer/components/ChatPanel/ChatPanel.tsx @@ -7,6 +7,7 @@ import { dispatchAssistantAction } from '../../navigation/assistantActionDispatc import { useA2UISurface } from '../../a2ui/useA2UISurface'; import { useAppStore } from '../../store'; import { ChatTranscript } from '../ChatSurface'; +import { useEntityLoader } from '../../navigation/useEntityEditor'; import { useI18n } from '../../i18n'; import '../../styles/chatSurface.css'; import './ChatPanel.css'; @@ -89,31 +90,43 @@ export const ChatPanel: React.FC = ({ conversationId }) => { } }, []); - // Load conversation and messages - const loadData = useCallback(async () => { - try { + // Load conversation data via shared entity loader (handles activeProject + // guard, cancellation on ID change, and close-tab-on-not-found). + const fetchChatData = useCallback( + async (id: string) => { const [conv, msgs, modelsResult] = await Promise.all([ - window.electronAPI?.chat.getConversation(conversationId), - window.electronAPI?.chat.getHistory(conversationId), - window.electronAPI?.chat.getAvailableModels() + window.electronAPI?.chat.getConversation(id), + window.electronAPI?.chat.getHistory(id), + window.electronAPI?.chat.getAvailableModels(), ]); + if (!conv) return null; - if (conv) setConversation(conv); - if (msgs) { - setMessages(msgs); - replayFromMessages(msgs); + return { conversation: conv, messages: msgs, models: modelsResult?.models }; + }, + [], + ); + + useEntityLoader(conversationId, fetchChatData, { + onLoaded: (data) => { + setConversation(data.conversation); + if (data.messages) { + setMessages(data.messages); + replayFromMessages(data.messages); } - if (modelsResult?.models) setAvailableModels(modelsResult.models); - } catch (error) { - console.error('Failed to load chat data:', error); - } - }, [conversationId, replayFromMessages]); + if (data.models) setAvailableModels(data.models); + }, + onReset: () => { + setConversation(null); + }, + }); + // Check API key readiness useEffect(() => { checkReady(); - loadData(); + }, [checkReady]); - // Subscribe to stream events + // Subscribe to stream / tool / title / token events + useEffect(() => { const unsubDelta = window.electronAPI?.chat.onStreamDelta((data) => { if (data.conversationId === conversationId) { appendStreamDelta(data.delta); @@ -164,7 +177,7 @@ export const ChatPanel: React.FC = ({ conversationId }) => { unsubTitle?.(); unsubTokenUsage?.(); }; - }, [conversationId, loadData, scrollToBottom, checkReady, appendStreamDelta, recordToolCall, recordToolResult]); + }, [conversationId, scrollToBottom, appendStreamDelta, recordToolCall, recordToolResult]); // Scroll on new messages or streaming content useEffect(() => { diff --git a/src/renderer/components/Editor/Editor.tsx b/src/renderer/components/Editor/Editor.tsx index 44793d8..994406c 100644 --- a/src/renderer/components/Editor/Editor.tsx +++ b/src/renderer/components/Editor/Editor.tsx @@ -26,6 +26,7 @@ import { InsertModal } from '../InsertModal'; import { AISuggestionsModal, AISuggestions } from '../AISuggestionsModal/AISuggestionsModal'; import { openEntityTab } from '../../navigation/tabPolicy'; import { EditorRoute, resolveEditorRoute } from '../../navigation/editorRouting'; +import { useEntityLoader, useSaveShortcut } from '../../navigation/useEntityEditor'; import { useI18n } from '../../i18n'; import documentationContent from '../../../../DOCUMENTATION.md?raw'; import apiDocumentationContent from '../../../../API.md?raw'; @@ -169,29 +170,25 @@ export const PostEditor: React.FC = ({ postId }) => { } = useAppStore(); // Fetch full post data from backend + const fetchPost = useCallback( + (id: string) => window.electronAPI?.posts.get(id).then((p) => (p as PostData) || null) ?? Promise.resolve(null), + [], + ); + const [post, setPost] = useState(null); - const [isLoadingPost, setIsLoadingPost] = useState(true); // Track whether form state has been initialized from post data const [isInitialized, setIsInitialized] = useState(false); - - useEffect(() => { - let cancelled = false; - setIsLoadingPost(true); - setIsInitialized(false); - window.electronAPI?.posts.get(postId).then((fetchedPost) => { - if (cancelled) return; - if (fetchedPost) { - setPost(fetchedPost as PostData); - // Also update the store so other components have the full data - useAppStore.getState().updatePost(postId, fetchedPost as Partial); - } else { - // Post doesn't exist, close the tab - closeTab(postId); - } - setIsLoadingPost(false); - }); - return () => { cancelled = true; }; - }, [postId, closeTab]); + + const { isLoading: isLoadingPost } = useEntityLoader(postId, fetchPost, { + onLoaded: (loadedPost) => { + setPost(loadedPost); + useAppStore.getState().updatePost(postId, loadedPost as Partial); + }, + onReset: () => { + setPost(null); + setIsInitialized(false); + }, + }); const [title, setTitle] = useState(''); const [content, setContent] = useState(''); @@ -705,17 +702,7 @@ export const PostEditor: React.FC = ({ postId }) => { }; // Save on Ctrl+S - useEffect(() => { - const handleKeyDown = (e: KeyboardEvent) => { - if ((e.ctrlKey || e.metaKey) && e.key === 's') { - e.preventDefault(); - handleSave(); - } - }; - - window.addEventListener('keydown', handleKeyDown); - return () => window.removeEventListener('keydown', handleKeyDown); - }, [handleSave]); + useSaveShortcut(handleSave); // Listen for menu events useEffect(() => { @@ -1055,6 +1042,7 @@ export const PostEditor: React.FC = ({ postId }) => { const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { const { t: tr } = useI18n(); const { media, updateMedia, showErrorModal, showConfirmDeleteModal, openTab } = useAppStore(); + const activeProjectId = useAppStore((s) => s.activeProject?.id ?? null); const item = media.find(m => m.id === mediaId); const [title, setTitle] = useState(item?.title || ''); @@ -1081,12 +1069,13 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { // Load project language setting useEffect(() => { + if (!activeProjectId) return; window.electronAPI?.meta.getProjectMetadata().then(metadata => { if (metadata?.mainLanguage) { setProjectLanguage(metadata.mainLanguage); } }); - }, []); + }, [activeProjectId]); // Close quick actions menu when clicking outside useEffect(() => { @@ -1152,7 +1141,7 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { // Load linked posts for this media and fetch their titles useEffect(() => { const loadLinkedPosts = async () => { - if (!mediaId) return; + if (!mediaId || !activeProjectId) return; try { const links = await window.electronAPI?.postMedia.getForMedia(mediaId); if (links) { @@ -1172,7 +1161,7 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { } }; loadLinkedPosts(); - }, [mediaId]); + }, [mediaId, activeProjectId]); // Fetch posts for the picker when it opens useEffect(() => { diff --git a/src/renderer/components/ImportAnalysisView/ImportAnalysisView.tsx b/src/renderer/components/ImportAnalysisView/ImportAnalysisView.tsx index ef73a00..c2bbf88 100644 --- a/src/renderer/components/ImportAnalysisView/ImportAnalysisView.tsx +++ b/src/renderer/components/ImportAnalysisView/ImportAnalysisView.tsx @@ -1,5 +1,6 @@ import React, { useState, useCallback, useEffect, useRef } from 'react'; import type { ChatModel } from '../../types/electron'; +import { useEntityLoader } from '../../navigation/useEntityEditor'; import { useI18n } from '../../i18n'; import './ImportAnalysisView.css'; @@ -162,7 +163,6 @@ export const ImportAnalysisView: React.FC = ({ definiti const [wxrFilePath, setWxrFilePath] = useState(null); const [report, setReport] = useState(null); const [isLoading, setIsLoading] = useState(false); - const [isLoadingDefinition, setIsLoadingDefinition] = useState(true); const [expandedSections, setExpandedSections] = useState>({}); const [progressStep, setProgressStep] = useState(''); const [progressDetail, setProgressDetail] = useState(''); @@ -308,31 +308,33 @@ export const ImportAnalysisView: React.FC = ({ definiti await persistReport(updatedReport); }, [report, persistReport]); - // Load definition on mount - useEffect(() => { - const load = async () => { - setIsLoadingDefinition(true); - try { - const def = await window.electronAPI?.importDefinitions.get(definitionId); - if (def) { - setName(def.name); - if (def.uploadsFolderPath) setUploadsFolder(def.uploadsFolderPath); - if (def.wxrFilePath) setWxrFilePath(def.wxrFilePath); - if (def.lastAnalysisResult) { - const parsed = typeof def.lastAnalysisResult === 'string' - ? JSON.parse(def.lastAnalysisResult) - : def.lastAnalysisResult; - setReport(parsed as AnalysisReport); - } - } - } catch (error) { - console.error('Failed to load import definition:', error); - } finally { - setIsLoadingDefinition(false); + // Load definition via shared entity loader (handles activeProject guard, + // cancellation on ID change, and close-tab-on-not-found). + const fetchDefinition = useCallback( + (id: string) => window.electronAPI?.importDefinitions.get(id) ?? Promise.resolve(null), + [], + ); + const { isLoading: isLoadingDefinition } = useEntityLoader(definitionId, fetchDefinition, { + onLoaded: (def) => { + setName(def.name); + setUploadsFolder(def.uploadsFolderPath ?? null); + setWxrFilePath(def.wxrFilePath ?? null); + if (def.lastAnalysisResult) { + const parsed = typeof def.lastAnalysisResult === 'string' + ? JSON.parse(def.lastAnalysisResult) + : def.lastAnalysisResult; + setReport(parsed as AnalysisReport); + } else { + setReport(null); } - }; - load(); - }, [definitionId]); + }, + onReset: () => { + setName(t('importAnalysis.untitledImport')); + setUploadsFolder(null); + setWxrFilePath(null); + setReport(null); + }, + }); const handleNameBlur = useCallback(async () => { const trimmed = name.trim() || t('importAnalysis.untitledImport'); diff --git a/src/renderer/components/MenuEditorView/MenuEditorView.tsx b/src/renderer/components/MenuEditorView/MenuEditorView.tsx index c19a6de..2a7b045 100644 --- a/src/renderer/components/MenuEditorView/MenuEditorView.tsx +++ b/src/renderer/components/MenuEditorView/MenuEditorView.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { Tree } from 'react-arborist'; import { useI18n } from '../../i18n'; +import { useAppStore } from '../../store'; import { showToast } from '../Toast'; import type { MenuDocument, MenuItemData, PostData } from '../../../main/shared/electronApi'; import { PageInput } from '../PageInput'; @@ -185,6 +186,7 @@ function renderMenuKindIcon(kind: MenuItemData['kind']): React.ReactNode { export const MenuEditorView: React.FC = () => { const { t: tr } = useI18n(); + const activeProjectId = useAppStore((s) => s.activeProject?.id ?? null); const [items, setItems] = useState([]); const [selectedId, setSelectedId] = useState(null); const [isLoading, setIsLoading] = useState(true); @@ -217,6 +219,7 @@ export const MenuEditorView: React.FC = () => { }, []); useEffect(() => { + if (!activeProjectId) return; const load = async () => { setIsLoading(true); try { @@ -258,7 +261,7 @@ export const MenuEditorView: React.FC = () => { }; void load(); - }, [tr]); + }, [tr, activeProjectId]); useEffect(() => { return () => { diff --git a/src/renderer/components/MetadataDiffPanel/MetadataDiffPanel.tsx b/src/renderer/components/MetadataDiffPanel/MetadataDiffPanel.tsx index d86ad1b..164e5d5 100644 --- a/src/renderer/components/MetadataDiffPanel/MetadataDiffPanel.tsx +++ b/src/renderer/components/MetadataDiffPanel/MetadataDiffPanel.tsx @@ -1,4 +1,5 @@ import React, { useState, useEffect, useCallback } from 'react'; +import { useAppStore } from '../../store'; import { showToast } from '../Toast'; import { useI18n } from '../../i18n'; import './MetadataDiffPanel.css'; @@ -42,6 +43,7 @@ type ScanPhase = 'idle' | 'loading-stats' | 'scanning' | 'complete'; export const MetadataDiffPanel: React.FC = () => { const { t: tr } = useI18n(); + const activeProjectId = useAppStore((s) => s.activeProject?.id ?? null); const [stats, setStats] = useState(null); const [scanResult, setScanResult] = useState(null); const [scanPhase, setScanPhase] = useState('idle'); @@ -51,6 +53,7 @@ export const MetadataDiffPanel: React.FC = () => { // Load initial stats useEffect(() => { + if (!activeProjectId) return; const loadStats = async () => { setScanPhase('loading-stats'); try { @@ -65,7 +68,7 @@ export const MetadataDiffPanel: React.FC = () => { setScanPhase('idle'); }; loadStats(); - }, [tr]); + }, [tr, activeProjectId]); // Subscribe to task progress useEffect(() => { diff --git a/src/renderer/components/ScriptsView/ScriptsView.tsx b/src/renderer/components/ScriptsView/ScriptsView.tsx index 68cd6d4..35f1a11 100644 --- a/src/renderer/components/ScriptsView/ScriptsView.tsx +++ b/src/renderer/components/ScriptsView/ScriptsView.tsx @@ -4,6 +4,7 @@ import type { ScriptData } from '../../../main/shared/electronApi'; import { useAppStore } from '../../store'; import { BDS_EVENT_SCRIPTS_CHANGED, dispatchWindowEvent } from '../../utils'; import { getPythonRuntimeManager } from '../../python/runtimeManagerInstance'; +import { useEntityLoader, useSaveShortcut } from '../../navigation/useEntityEditor'; import { useI18n } from '../../i18n'; import { showToast } from '../Toast'; import './ScriptsView.css'; @@ -46,6 +47,12 @@ export const ScriptsView: React.FC = ({ scriptId }) => { const { t, language } = useI18n(); const appendPanelOutputEntry = useAppStore((state) => state.appendPanelOutputEntry); const closeTab = useAppStore((state) => state.closeTab); + + const fetchScript = useCallback( + (id: string) => window.electronAPI?.scripts.get(id) ?? Promise.resolve(null), + [], + ); + const [script, setScript] = useState(null); const [title, setTitle] = useState(''); const [slug, setSlug] = useState(''); @@ -63,6 +70,65 @@ export const ScriptsView: React.FC = ({ scriptId }) => { // Token incremented to signal Monaco to remount with fresh defaultValue. // Prevents controlled-mode cursor jumps during typing. const [monacoResetToken, setMonacoResetToken] = useState(0); + // Ref for entrypoint refresh cancellation — survives across renders + // without triggering the entityLoader effect. + const entrypointCancelRef = useRef(false); + + useEntityLoader(scriptId, fetchScript, { + onLoaded: (loadedScript) => { + setScript(loadedScript); + setTitle(loadedScript.title || ''); + setSlug(toFunctionSlug(loadedScript.slug || loadedScript.title || '')); + setKind(loadedScript.kind || 'utility'); + setEntrypoint(loadedScript.entrypoint || 'render'); + setEnabled(loadedScript.enabled ?? true); + setScriptContent(loadedScript.content || ''); + setMonacoResetToken(prev => prev + 1); + const normalizedExisting = toFunctionSlug(loadedScript.slug || loadedScript.title || ''); + setIsSlugManuallyEdited(normalizedExisting !== toFunctionSlug(loadedScript.title || '')); + + // Refresh entrypoints asynchronously + entrypointCancelRef.current = true; // cancel any pending refresh + const cancelToken = {}; + entrypointCancelRef.current = false; + const refreshEntrypoints = async () => { + try { + const runtimeManager = getPythonRuntimeManager(); + const discoveredEntrypoints = await runtimeManager.inspectEntrypoints( + loadedScript.content || '', + { cacheKey: buildCacheKey(loadedScript, loadedScript.content || '') }, + ); + const available = withMainEntrypoint(discoveredEntrypoints); + + if (entrypointCancelRef.current) return; + + setAvailableEntrypoints(available); + const preferredEntrypoint = available.includes(loadedScript.entrypoint) + ? loadedScript.entrypoint + : 'main'; + setEntrypoint(preferredEntrypoint); + } catch { + if (entrypointCancelRef.current) return; + setAvailableEntrypoints(['main']); + setEntrypoint('main'); + } + }; + void refreshEntrypoints(); + }, + onReset: () => { + entrypointCancelRef.current = true; + setScript(null); + setTitle(''); + setSlug(''); + setKind('utility'); + setEntrypoint('render'); + setAvailableEntrypoints(['main']); + setEnabled(true); + setScriptContent(''); + setMonacoResetToken(prev => prev + 1); + setIsSlugManuallyEdited(false); + }, + }); const buildCacheKey = (scriptMeta: Pick, content: string): string => { let hash = 0; @@ -166,86 +232,6 @@ export const ScriptsView: React.FC = ({ scriptId }) => { } }, [appendPanelOutputEntry, applySyntaxMarkers, isCheckingSyntax, script, scriptContent, t]); - useEffect(() => { - let cancelled = false; - - const refreshEntrypoints = async (content: string, scriptMeta: ScriptData) => { - try { - const runtimeManager = getPythonRuntimeManager(); - const discoveredEntrypoints = await runtimeManager.inspectEntrypoints(content, { - cacheKey: buildCacheKey(scriptMeta, content), - }); - const available = withMainEntrypoint(discoveredEntrypoints); - - if (cancelled) { - return; - } - - setAvailableEntrypoints(available); - - const preferredEntrypoint = available.includes(scriptMeta.entrypoint) - ? scriptMeta.entrypoint - : 'main'; - setEntrypoint(preferredEntrypoint); - } catch (error) { - if (cancelled) { - return; - } - setAvailableEntrypoints(['main']); - setEntrypoint('main'); - } - }; - - const loadScript = async () => { - if (!scriptId) { - setScript(null); - setTitle(''); - setSlug(''); - setKind('utility'); - setEntrypoint('render'); - setAvailableEntrypoints(['main']); - setEnabled(true); - setScriptContent(''); - setMonacoResetToken(prev => prev + 1); - setIsSlugManuallyEdited(false); - return; - } - - const item = await window.electronAPI?.scripts.get(scriptId); - if (cancelled || !item) { - setScript(null); - setTitle(''); - setSlug(''); - setKind('utility'); - setEntrypoint('render'); - setAvailableEntrypoints(['main']); - setEnabled(true); - setScriptContent(''); - setMonacoResetToken(prev => prev + 1); - setIsSlugManuallyEdited(false); - return; - } - - setScript(item); - setTitle(item.title || ''); - setSlug(toFunctionSlug(item.slug || item.title || '')); - setKind(item.kind || 'utility'); - setEntrypoint(item.entrypoint || 'render'); - setEnabled(item.enabled ?? true); - setScriptContent(item.content || ''); - setMonacoResetToken(prev => prev + 1); - const normalizedExisting = toFunctionSlug(item.slug || item.title || ''); - setIsSlugManuallyEdited(normalizedExisting !== toFunctionSlug(item.title || '')); - await refreshEntrypoints(item.content || '', item); - }; - - void loadScript(); - - return () => { - cancelled = true; - }; - }, [scriptId]); - const hasChanges = !!script && ( title !== script.title || slug !== script.slug || @@ -340,21 +326,7 @@ export const ScriptsView: React.FC = ({ scriptId }) => { } }; - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if ((event.ctrlKey || event.metaKey) && event.key === 's') { - event.preventDefault(); - void handleSaveScript(); - } - }; - - if (typeof window.addEventListener !== 'function' || typeof window.removeEventListener !== 'function') { - return; - } - - window.addEventListener('keydown', handleKeyDown); - return () => window.removeEventListener('keydown', handleKeyDown); - }, [handleSaveScript]); + useSaveShortcut(useCallback(() => { void handleSaveScript(); }, [handleSaveScript])); const handleRunScript = async () => { if (!script || isRunning) { diff --git a/src/renderer/components/TagsView/TagsView.tsx b/src/renderer/components/TagsView/TagsView.tsx index 870b673..e1f0d12 100644 --- a/src/renderer/components/TagsView/TagsView.tsx +++ b/src/renderer/components/TagsView/TagsView.tsx @@ -133,6 +133,7 @@ const SectionHeader: React.FC<{ export const TagsView: React.FC = () => { const { t } = useI18n(); const { showErrorModal } = useAppStore(); + const activeProjectId = useAppStore((s) => s.activeProject?.id ?? null); // State const [tagsWithCounts, setTagsWithCounts] = useState([]); @@ -181,22 +182,25 @@ export const TagsView: React.FC = () => { }, []); useEffect(() => { + if (!activeProjectId) return; loadTags(); - }, [loadTags]); + }, [loadTags, activeProjectId]); // Listen for tag events useEffect(() => { + if (!activeProjectId) return; return subscribeToTagEvents(window.electronAPI?.on, loadTags, { includeUpdated: true, }); - }, [loadTags]); + }, [loadTags, activeProjectId]); // Load post templates on mount useEffect(() => { + if (!activeProjectId) return; window.electronAPI?.templates.getEnabledByKind('post').then((templates) => { setPostTemplates(templates.map((t) => ({ slug: t.slug, title: t.title }))); }); - }, []); + }, [activeProjectId]); // Handle tag selection const handleTagSelect = (name: string) => { diff --git a/src/renderer/components/TemplatesView/TemplatesView.tsx b/src/renderer/components/TemplatesView/TemplatesView.tsx index 4f77d33..6982fb7 100644 --- a/src/renderer/components/TemplatesView/TemplatesView.tsx +++ b/src/renderer/components/TemplatesView/TemplatesView.tsx @@ -1,8 +1,9 @@ -import React, { useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import MonacoEditor from '@monaco-editor/react'; import type { TemplateData, TemplateKind } from '../../../main/shared/electronApi'; import { useAppStore } from '../../store'; import { BDS_EVENT_TEMPLATES_CHANGED, dispatchWindowEvent } from '../../utils'; +import { useEntityLoader, useSaveShortcut } from '../../navigation/useEntityEditor'; import { useI18n } from '../../i18n'; import { showToast } from '../Toast'; import './TemplatesView.css'; @@ -30,6 +31,12 @@ interface TemplatesViewProps { export const TemplatesView: React.FC = ({ templateId }) => { const { t, language } = useI18n(); const closeTab = useAppStore((state) => state.closeTab); + + const fetchTemplate = useCallback( + (id: string) => window.electronAPI?.templates.get(id) ?? Promise.resolve(null), + [], + ); + const [template, setTemplate] = useState(null); const [title, setTitle] = useState(''); const [slug, setSlug] = useState(''); @@ -41,52 +48,29 @@ export const TemplatesView: React.FC = ({ templateId }) => { const [isValidating, setIsValidating] = useState(false); const [monacoResetToken, setMonacoResetToken] = useState(0); - useEffect(() => { - let cancelled = false; - - const loadTemplate = async () => { - if (!templateId) { - setTemplate(null); - setTitle(''); - setSlug(''); - setKind('post'); - setEnabled(true); - setTemplateContent(''); - setMonacoResetToken((prev) => prev + 1); - setIsSlugManuallyEdited(false); - return; - } - - const item = await window.electronAPI?.templates.get(templateId); - if (cancelled || !item) { - setTemplate(null); - setTitle(''); - setSlug(''); - setKind('post'); - setEnabled(true); - setTemplateContent(''); - setMonacoResetToken((prev) => prev + 1); - setIsSlugManuallyEdited(false); - return; - } - - setTemplate(item); - setTitle(item.title || ''); - setSlug(item.slug || toTemplateSlug(item.title || '')); - setKind(item.kind || 'post'); - setEnabled(item.enabled ?? true); - setTemplateContent(item.content || ''); + useEntityLoader(templateId, fetchTemplate, { + onLoaded: (loadedTemplate) => { + setTemplate(loadedTemplate); + setTitle(loadedTemplate.title || ''); + setSlug(loadedTemplate.slug || toTemplateSlug(loadedTemplate.title || '')); + setKind(loadedTemplate.kind || 'post'); + setEnabled(loadedTemplate.enabled ?? true); + setTemplateContent(loadedTemplate.content || ''); setMonacoResetToken((prev) => prev + 1); - const normalizedExisting = toTemplateSlug(item.slug || item.title || ''); - setIsSlugManuallyEdited(normalizedExisting !== toTemplateSlug(item.title || '')); - }; - - void loadTemplate(); - - return () => { - cancelled = true; - }; - }, [templateId]); + const normalizedExisting = toTemplateSlug(loadedTemplate.slug || loadedTemplate.title || ''); + setIsSlugManuallyEdited(normalizedExisting !== toTemplateSlug(loadedTemplate.title || '')); + }, + onReset: () => { + setTemplate(null); + setTitle(''); + setSlug(''); + setKind('post'); + setEnabled(true); + setTemplateContent(''); + setMonacoResetToken((prev) => prev + 1); + setIsSlugManuallyEdited(false); + }, + }); const hasChanges = !!template && @@ -214,21 +198,7 @@ export const TemplatesView: React.FC = ({ templateId }) => { } }; - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if ((event.ctrlKey || event.metaKey) && event.key === 's') { - event.preventDefault(); - void handleSaveTemplate(); - } - }; - - if (typeof window.addEventListener !== 'function' || typeof window.removeEventListener !== 'function') { - return; - } - - window.addEventListener('keydown', handleKeyDown); - return () => window.removeEventListener('keydown', handleKeyDown); - }, [handleSaveTemplate]); + useSaveShortcut(useCallback(() => { void handleSaveTemplate(); }, [handleSaveTemplate])); return (
diff --git a/src/renderer/navigation/useEntityEditor.ts b/src/renderer/navigation/useEntityEditor.ts new file mode 100644 index 0000000..db1d10c --- /dev/null +++ b/src/renderer/navigation/useEntityEditor.ts @@ -0,0 +1,108 @@ +import { useCallback, useEffect, useState } from 'react'; +import { useAppStore } from '../store'; + +/** + * Shared hook for entity editors that load data by ID. + * + * Handles: + * - Deferring load until activeProject is available (startup race condition) + * - Cancellation of in-flight fetches on ID change / unmount + * - Closing the tab when the entity is not found (e.g. after project switch) + * - Providing a reload() helper for external-change refresh + * + * Instead of returning data that requires a separate sync effect, this hook + * calls `onLoaded` / `onReset` directly from within the fetch effect so + * that local state updates in the same render cycle as the load — matching + * the timing behaviour editors had when each hand-rolled its own load logic. + * + * @param entityId The entity primary key, or null for the "no entity" state. + * @param fetcher Async function that takes an ID and returns the entity or null. + * @param callbacks `onLoaded(data)` when entity arrives, `onReset()` when + * entityId is null or entity was not found. + */ +export function useEntityLoader( + entityId: string | null, + fetcher: (id: string) => Promise, + callbacks: { + onLoaded: (data: T) => void; + onReset: () => void; + }, +): { + isLoading: boolean; + reload: () => void; +} { + const activeProjectId = useAppStore((s) => s.activeProject?.id ?? null); + const closeTab = useAppStore((s) => s.closeTab); + + const [isLoading, setIsLoading] = useState(true); + const [reloadToken, setReloadToken] = useState(0); + + useEffect(() => { + if (!entityId) { + callbacks.onReset(); + setIsLoading(false); + return; + } + + if (!activeProjectId) { + // Project context not ready yet — stay in loading state, will + // re-run once activeProjectId appears. + callbacks.onReset(); + setIsLoading(true); + return; + } + + let cancelled = false; + setIsLoading(true); + + fetcher(entityId).then((result) => { + if (cancelled) return; + + if (result) { + callbacks.onLoaded(result); + } else { + // Entity doesn't exist in this project — close the orphaned tab. + callbacks.onReset(); + closeTab(entityId); + } + setIsLoading(false); + }); + + return () => { + cancelled = true; + }; + // callbacks intentionally excluded — they are inline objects whose + // identity changes each render; the real dependencies are the + // entityId, activeProjectId, and reloadToken. + }, [entityId, activeProjectId, reloadToken, fetcher, closeTab]); + + const reload = useCallback(() => { + setReloadToken((prev) => prev + 1); + }, []); + + return { isLoading, reload }; +} + +/** + * Bind Cmd/Ctrl+S to a save callback. + * + * Replaces the identical keydown listener that was copy-pasted across + * PostEditor, ScriptsView, and TemplatesView. + */ +export function useSaveShortcut(onSave: () => void): void { + useEffect(() => { + if (typeof window.addEventListener !== 'function' || typeof window.removeEventListener !== 'function') { + return; + } + + const handleKeyDown = (event: KeyboardEvent) => { + if ((event.ctrlKey || event.metaKey) && event.key === 's') { + event.preventDefault(); + onSave(); + } + }; + + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [onSave]); +} diff --git a/tests/renderer/components/EditorAutoSaveCursor.test.tsx b/tests/renderer/components/EditorAutoSaveCursor.test.tsx index b9a641c..0751afc 100644 --- a/tests/renderer/components/EditorAutoSaveCursor.test.tsx +++ b/tests/renderer/components/EditorAutoSaveCursor.test.tsx @@ -185,6 +185,7 @@ describe('Editor does not reset content on auto-save (cursor stability)', () => (window as any).electronAPI.meta.getCategories = vi.fn().mockReturnValue(neverSettles); useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, preferredEditorMode: 'markdown', posts: [], media: [], diff --git a/tests/renderer/components/EditorDashboardTimeline.test.tsx b/tests/renderer/components/EditorDashboardTimeline.test.tsx index c8ff8b1..65f1a63 100644 --- a/tests/renderer/components/EditorDashboardTimeline.test.tsx +++ b/tests/renderer/components/EditorDashboardTimeline.test.tsx @@ -62,6 +62,7 @@ describe('Editor dashboard timeline', () => { (window as any).electronAPI.app.setPreviewPostTarget = vi.fn().mockResolvedValue(undefined); useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, tabs: [], activeTabId: null, posts: [], diff --git a/tests/renderer/components/EditorMetadataCollapse.test.tsx b/tests/renderer/components/EditorMetadataCollapse.test.tsx index e04aa6f..a923f42 100644 --- a/tests/renderer/components/EditorMetadataCollapse.test.tsx +++ b/tests/renderer/components/EditorMetadataCollapse.test.tsx @@ -155,6 +155,7 @@ describe('Editor metadata collapse', () => { (window as any).electronAPI.meta.getCategories = vi.fn().mockReturnValue(neverSettles); useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, preferredEditorMode: 'wysiwyg', posts: [], media: [], diff --git a/tests/renderer/components/EditorVisualModePersistence.test.tsx b/tests/renderer/components/EditorVisualModePersistence.test.tsx index 5f3a848..ec1ce8b 100644 --- a/tests/renderer/components/EditorVisualModePersistence.test.tsx +++ b/tests/renderer/components/EditorVisualModePersistence.test.tsx @@ -165,6 +165,7 @@ describe('Editor visual mode persistence', () => { (window as any).electronAPI.meta.getCategories = vi.fn().mockReturnValue(neverSettles); useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, preferredEditorMode: 'wysiwyg', posts: [], media: [], diff --git a/tests/renderer/components/MenuEditorView.test.tsx b/tests/renderer/components/MenuEditorView.test.tsx index 1999e6e..be2a828 100644 --- a/tests/renderer/components/MenuEditorView.test.tsx +++ b/tests/renderer/components/MenuEditorView.test.tsx @@ -3,11 +3,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { fireEvent, render, screen } from '@testing-library/react'; import { MenuEditorView } from '../../../src/renderer/components/MenuEditorView/MenuEditorView'; +import { useAppStore } from '../../../src/renderer/store'; describe('MenuEditorView entry editor', () => { beforeEach(() => { vi.clearAllMocks(); + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + (window as any).electronAPI = { ...(window as any).electronAPI, menu: { diff --git a/tests/renderer/components/ScriptsView.test.tsx b/tests/renderer/components/ScriptsView.test.tsx index 5f703aa..0387950 100644 --- a/tests/renderer/components/ScriptsView.test.tsx +++ b/tests/renderer/components/ScriptsView.test.tsx @@ -86,6 +86,7 @@ describe('ScriptsView', () => { }; useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, panelVisible: false, panelActiveTab: 'tasks', panelOutputEntries: [], @@ -437,4 +438,30 @@ describe('ScriptsView', () => { expect(startTaskMock).not.toHaveBeenCalled(); }); }); + + it('defers loading until activeProject is set to avoid startup race condition', async () => { + useAppStore.setState({ activeProject: null }); + + const getMock = (window as any).electronAPI.scripts.get; + const { rerender } = render(); + + // Give the effect a chance to run + await vi.waitFor(() => { + expect(getMock).not.toHaveBeenCalled(); + }); + + // Now simulate project context becoming available + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + + // Re-render to pick up store change + rerender(); + + await vi.waitFor(() => { + expect(getMock).toHaveBeenCalledWith('script-1'); + const textarea = screen.getByLabelText('Script Content') as HTMLTextAreaElement; + expect(textarea.value).toContain('print("hello")'); + }); + }); }); diff --git a/tests/renderer/components/TagsView.test.tsx b/tests/renderer/components/TagsView.test.tsx index 57ee8ce..3cb4167 100644 --- a/tests/renderer/components/TagsView.test.tsx +++ b/tests/renderer/components/TagsView.test.tsx @@ -2,11 +2,16 @@ import React from 'react'; import { describe, it, expect, beforeEach, vi } from 'vitest'; import { render, act, screen, fireEvent } from '@testing-library/react'; import { TagsView } from '../../../src/renderer/components/TagsView/TagsView'; +import { useAppStore } from '../../../src/renderer/store'; describe('TagsView subscriptions', () => { beforeEach(() => { const onMock = vi.fn((_channel: string, _callback: (...args: unknown[]) => void) => vi.fn()); + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + (window as any).electronAPI = { ...(window as any).electronAPI, tags: { @@ -66,6 +71,10 @@ describe('TagsView template dropdown', () => { beforeEach(() => { const onMock = vi.fn((_channel: string, _callback: (...args: unknown[]) => void) => vi.fn()); + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + (window as any).electronAPI = { ...(window as any).electronAPI, tags: { diff --git a/tests/renderer/components/TemplatesView.test.tsx b/tests/renderer/components/TemplatesView.test.tsx index 121262e..ffb4f26 100644 --- a/tests/renderer/components/TemplatesView.test.tsx +++ b/tests/renderer/components/TemplatesView.test.tsx @@ -42,6 +42,10 @@ describe('TemplatesView', () => { beforeEach(() => { vi.clearAllMocks(); + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + (window as any).electronAPI = { ...(window as any).electronAPI, templates: { @@ -208,4 +212,27 @@ describe('TemplatesView', () => { expect(saveButton).toBeDisabled(); }); }); + + it('defers loading until activeProject is set to avoid startup race condition', async () => { + useAppStore.setState({ activeProject: null }); + + const getMock = (window as any).electronAPI.templates.get; + const { rerender } = render(); + + await vi.waitFor(() => { + expect(getMock).not.toHaveBeenCalled(); + }); + + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + + rerender(); + + await vi.waitFor(() => { + expect(getMock).toHaveBeenCalledWith('template-1'); + const titleInput = screen.getByLabelText('Title') as HTMLInputElement; + expect(titleInput.value).toBe('Custom Post'); + }); + }); }); diff --git a/tests/renderer/navigation/chatSurfaceUsageGuards.test.ts b/tests/renderer/navigation/chatSurfaceUsageGuards.test.ts index 71707f5..e4e52e9 100644 --- a/tests/renderer/navigation/chatSurfaceUsageGuards.test.ts +++ b/tests/renderer/navigation/chatSurfaceUsageGuards.test.ts @@ -11,7 +11,7 @@ describe('chat surface shared usage guards', () => { Element.prototype.scrollIntoView = vi.fn(); } - useAppStore.setState({ tabs: [], activeTabId: null, activeView: 'posts' }); + useAppStore.setState({ tabs: [], activeTabId: null, activeView: 'posts', activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any }); window.electronAPI.chat = { checkReady: vi.fn().mockResolvedValue({ ready: true }), diff --git a/tests/renderer/navigation/useEntityEditor.test.ts b/tests/renderer/navigation/useEntityEditor.test.ts new file mode 100644 index 0000000..5e9de55 --- /dev/null +++ b/tests/renderer/navigation/useEntityEditor.test.ts @@ -0,0 +1,258 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { useEntityLoader, useSaveShortcut } from '../../../src/renderer/navigation/useEntityEditor'; +import { useAppStore } from '../../../src/renderer/store'; + +describe('useEntityLoader', () => { + beforeEach(() => { + vi.clearAllMocks(); + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + tabs: [{ type: 'scripts', id: 'entity-1', isTransient: false }], + activeTabId: 'entity-1', + }); + }); + + it('defers loading until activeProject is set', async () => { + useAppStore.setState({ activeProject: null }); + + const fetcher = vi.fn().mockResolvedValue({ id: 'entity-1', title: 'Test' }); + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + const { result, rerender } = renderHook( + ({ id }) => useEntityLoader(id, fetcher, { onLoaded, onReset }), + { initialProps: { id: 'entity-1' } }, + ); + + expect(result.current.isLoading).toBe(true); + expect(fetcher).not.toHaveBeenCalled(); + // onReset is called when activeProject is missing (clear state) + expect(onReset).toHaveBeenCalled(); + + onReset.mockClear(); + + // Simulate project becoming available + act(() => { + useAppStore.setState({ + activeProject: { id: 'project-1', name: 'Test', path: '/tmp/test' } as any, + }); + }); + rerender({ id: 'entity-1' }); + + await vi.waitFor(() => { + expect(fetcher).toHaveBeenCalledWith('entity-1'); + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-1', title: 'Test' }); + expect(result.current.isLoading).toBe(false); + }); + }); + + it('loads entity data when activeProject is already set', async () => { + const fetcher = vi.fn().mockResolvedValue({ id: 'entity-1', title: 'Hello' }); + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + const { result } = renderHook(() => + useEntityLoader('entity-1', fetcher, { onLoaded, onReset }), + ); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-1', title: 'Hello' }); + expect(result.current.isLoading).toBe(false); + }); + }); + + it('closes tab when entity is not found', async () => { + const fetcher = vi.fn().mockResolvedValue(null); + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + renderHook(() => + useEntityLoader('entity-1', fetcher, { onLoaded, onReset }), + ); + + await vi.waitFor(() => { + expect(fetcher).toHaveBeenCalledWith('entity-1'); + expect(onLoaded).not.toHaveBeenCalled(); + expect(onReset).toHaveBeenCalled(); + const state = useAppStore.getState(); + expect(state.tabs.find(t => t.id === 'entity-1')).toBeUndefined(); + }); + }); + + it('resets and reloads when entity ID changes', async () => { + const fetcher = vi.fn() + .mockResolvedValueOnce({ id: 'entity-1', title: 'First' }) + .mockResolvedValueOnce({ id: 'entity-2', title: 'Second' }); + + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + useAppStore.setState({ + tabs: [ + { type: 'scripts', id: 'entity-1', isTransient: false }, + { type: 'scripts', id: 'entity-2', isTransient: false }, + ], + }); + + const { rerender } = renderHook( + ({ id }) => useEntityLoader(id, fetcher, { onLoaded, onReset }), + { initialProps: { id: 'entity-1' } }, + ); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-1', title: 'First' }); + }); + + onLoaded.mockClear(); + rerender({ id: 'entity-2' }); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-2', title: 'Second' }); + }); + }); + + it('cancels in-flight load when entity ID changes quickly', async () => { + let resolveFirst: (value: unknown) => void; + const firstPromise = new Promise((resolve) => { resolveFirst = resolve; }); + const fetcher = vi.fn() + .mockReturnValueOnce(firstPromise) + .mockResolvedValueOnce({ id: 'entity-2', title: 'Second' }); + + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + useAppStore.setState({ + tabs: [ + { type: 'scripts', id: 'entity-1', isTransient: false }, + { type: 'scripts', id: 'entity-2', isTransient: false }, + ], + }); + + const { rerender } = renderHook( + ({ id }) => useEntityLoader(id, fetcher, { onLoaded, onReset }), + { initialProps: { id: 'entity-1' } }, + ); + + // Switch before first resolves + rerender({ id: 'entity-2' }); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-2', title: 'Second' }); + }); + + // Late resolve of first should have no effect + resolveFirst!({ id: 'entity-1', title: 'First' }); + + // onLoaded should only have been called once (for entity-2) + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledTimes(1); + }); + }); + + it('calls onReset and isLoading=false when entityId is null', async () => { + const fetcher = vi.fn(); + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + const { result } = renderHook(() => + useEntityLoader(null, fetcher, { onLoaded, onReset }), + ); + + await vi.waitFor(() => { + expect(onReset).toHaveBeenCalled(); + expect(result.current.isLoading).toBe(false); + expect(fetcher).not.toHaveBeenCalled(); + }); + }); + + it('allows reload via returned reload function', async () => { + const fetcher = vi.fn() + .mockResolvedValueOnce({ id: 'entity-1', title: 'V1' }) + .mockResolvedValueOnce({ id: 'entity-1', title: 'V2' }); + + const onLoaded = vi.fn(); + const onReset = vi.fn(); + + const { result } = renderHook(() => + useEntityLoader('entity-1', fetcher, { onLoaded, onReset }), + ); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-1', title: 'V1' }); + }); + + await act(async () => { + result.current.reload(); + }); + + await vi.waitFor(() => { + expect(onLoaded).toHaveBeenCalledWith({ id: 'entity-1', title: 'V2' }); + expect(fetcher).toHaveBeenCalledTimes(2); + }); + }); +}); + +describe('useSaveShortcut', () => { + let eventTarget: EventTarget; + + beforeEach(() => { + eventTarget = new EventTarget(); + window.addEventListener = eventTarget.addEventListener.bind(eventTarget); + window.removeEventListener = eventTarget.removeEventListener.bind(eventTarget); + window.dispatchEvent = eventTarget.dispatchEvent.bind(eventTarget); + }); + + it('calls onSave when Cmd+S is pressed', () => { + const onSave = vi.fn(); + renderHook(() => useSaveShortcut(onSave)); + + const event = new KeyboardEvent('keydown', { + key: 's', + metaKey: true, + bubbles: true, + }); + window.dispatchEvent(event); + + expect(onSave).toHaveBeenCalledTimes(1); + }); + + it('calls onSave when Ctrl+S is pressed', () => { + const onSave = vi.fn(); + renderHook(() => useSaveShortcut(onSave)); + + const event = new KeyboardEvent('keydown', { + key: 's', + ctrlKey: true, + bubbles: true, + }); + window.dispatchEvent(event); + + expect(onSave).toHaveBeenCalledTimes(1); + }); + + it('does not call onSave for other key combinations', () => { + const onSave = vi.fn(); + renderHook(() => useSaveShortcut(onSave)); + + window.dispatchEvent(new KeyboardEvent('keydown', { key: 's', bubbles: true })); + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'a', metaKey: true, bubbles: true })); + + expect(onSave).not.toHaveBeenCalled(); + }); + + it('cleans up event listener on unmount', () => { + const onSave = vi.fn(); + const { unmount } = renderHook(() => useSaveShortcut(onSave)); + + unmount(); + + window.dispatchEvent(new KeyboardEvent('keydown', { + key: 's', + metaKey: true, + bubbles: true, + })); + + expect(onSave).not.toHaveBeenCalled(); + }); +});