From fc77ce6e117bc42b1c0066061e4f9c3e0cf0775e Mon Sep 17 00:00:00 2001 From: lucaronin Date: Wed, 18 Mar 2026 13:32:07 +0100 Subject: [PATCH] fix: sync raw editor + suppress toast overwrite on trash/archive actions Two bugs fixed: 1. CodeMirror raw editor didn't reflect frontmatter changes (e.g. Trashed: true) after trash/archive because useCodeMirror only used content as initial state. Added content-sync effect with external-sync guard to prevent infinite loops. 2. Entry actions (trash/archive/restore/unarchive) showed "Property updated" toast instead of contextual message because runFrontmatterAndApply overwrote it. Added silent option to suppress toast when caller manages its own feedback. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/hooks/frontmatterOps.ts | 9 +- src/hooks/useCodeMirror.test.ts | 34 +++++++ src/hooks/useCodeMirror.ts | 15 ++- src/hooks/useEntryActions.test.ts | 37 +++++-- src/hooks/useEntryActions.ts | 17 ++-- src/hooks/useNoteActions.ts | 12 +-- .../trash-archive-ui-consistency.spec.ts | 98 +++++++++++++++++++ 7 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 tests/smoke/trash-archive-ui-consistency.spec.ts diff --git a/src/hooks/frontmatterOps.ts b/src/hooks/frontmatterOps.ts index 77be5ecb..33fd3ad4 100644 --- a/src/hooks/frontmatterOps.ts +++ b/src/hooks/frontmatterOps.ts @@ -60,18 +60,25 @@ async function executeFrontmatterOp(op: 'update' | 'delete', path: string, key: return isTauri() ? invokeFrontmatter('delete_frontmatter_property', { path, key }) : applyMockFrontmatterDelete(path, key) } +export interface FrontmatterOpOptions { + /** Suppress toast feedback (caller manages its own toast). */ + silent?: boolean +} + /** Run a frontmatter update/delete and apply the result to state. */ export async function runFrontmatterAndApply( op: 'update' | 'delete', path: string, key: string, value: FrontmatterValue | undefined, callbacks: { updateTab: (p: string, c: string) => void; updateEntry: (p: string, patch: Partial) => void; toast: (m: string | null) => void }, + options?: FrontmatterOpOptions, ): Promise { try { callbacks.updateTab(path, await executeFrontmatterOp(op, path, key, value)) const patch = frontmatterToEntryPatch(op, key, value) if (Object.keys(patch).length > 0) callbacks.updateEntry(path, patch) - callbacks.toast(op === 'update' ? 'Property updated' : 'Property deleted') + if (!options?.silent) callbacks.toast(op === 'update' ? 'Property updated' : 'Property deleted') } catch (err) { console.error(`Failed to ${op} frontmatter:`, err) + if (options?.silent) throw err callbacks.toast(`Failed to ${op} property`) } } diff --git a/src/hooks/useCodeMirror.test.ts b/src/hooks/useCodeMirror.test.ts index 64e9397a..9105862c 100644 --- a/src/hooks/useCodeMirror.test.ts +++ b/src/hooks/useCodeMirror.test.ts @@ -67,6 +67,40 @@ describe('useCodeMirror', () => { spy.mockRestore() }) + it('syncs content prop changes to the editor', () => { + const ref = { current: container } + const onDocChange = vi.fn() + const callbacks = { ...noopCallbacks, onDocChange } + const { result, rerender } = renderHook( + ({ content }) => useCodeMirror(ref, content, false, callbacks), + { initialProps: { content: '---\ntitle: Hello\n---\nBody' } }, + ) + const view = result.current.current! + expect(view.state.doc.toString()).toBe('---\ntitle: Hello\n---\nBody') + + // Simulate external content update (e.g. frontmatter written to disk) + rerender({ content: '---\ntitle: Hello\nTrashed: true\n---\nBody' }) + + expect(view.state.doc.toString()).toBe('---\ntitle: Hello\nTrashed: true\n---\nBody') + // External sync should NOT trigger onDocChange (would cause infinite loop) + expect(onDocChange).not.toHaveBeenCalled() + }) + + it('does not sync when content matches current editor state', () => { + const ref = { current: container } + const { result, rerender } = renderHook( + ({ content }) => useCodeMirror(ref, content, false, noopCallbacks), + { initialProps: { content: 'hello' } }, + ) + const view = result.current.current! + const spy = vi.spyOn(view, 'dispatch') + + // Re-render with same content — no dispatch needed + rerender({ content: 'hello' }) + expect(spy).not.toHaveBeenCalled() + spy.mockRestore() + }) + it('installs zoomCursorFix that overrides posAtCoords on the view instance', () => { const ref = { current: container } const { result } = renderHook(() => diff --git a/src/hooks/useCodeMirror.ts b/src/hooks/useCodeMirror.ts index 6696db4f..1ad764ee 100644 --- a/src/hooks/useCodeMirror.ts +++ b/src/hooks/useCodeMirror.ts @@ -82,6 +82,19 @@ export function useCodeMirror( const viewRef = useRef(null) const callbacksRef = useRef(callbacks) callbacksRef.current = callbacks + // Track whether we're dispatching an external sync so the updateListener skips it + const externalSyncRef = useRef(false) + + // Sync content prop changes to the editor (e.g. after frontmatter update on disk) + useEffect(() => { + const view = viewRef.current + if (!view) return + const current = view.state.doc.toString() + if (current === content) return + externalSyncRef.current = true + view.dispatch({ changes: { from: 0, to: current.length, insert: content } }) + externalSyncRef.current = false + }, [content]) useEffect(() => { const parent = containerRef.current @@ -101,7 +114,7 @@ export function useCodeMirror( frontmatterHighlightPlugin, zoomCursorFix(), EditorView.updateListener.of((update) => { - if (update.docChanged) { + if (update.docChanged && !externalSyncRef.current) { callbacksRef.current.onDocChange(update.state.doc.toString()) } if (update.selectionSet || update.docChanged) { diff --git a/src/hooks/useEntryActions.test.ts b/src/hooks/useEntryActions.test.ts index 9b44e6a9..b4d1bd51 100644 --- a/src/hooks/useEntryActions.test.ts +++ b/src/hooks/useEntryActions.test.ts @@ -67,8 +67,8 @@ describe('useEntryActions', () => { await result.current.handleTrashNote('/vault/note/test.md') }) - expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed', true) - expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed at', expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/)) + expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed', true, { silent: true }) + expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed at', expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/), { silent: true }) expect(updateEntry).toHaveBeenCalledWith('/vault/note/test.md', { trashed: true, trashedAt: expect.any(Number), @@ -76,6 +76,19 @@ describe('useEntryActions', () => { expect(setToastMessage).toHaveBeenCalledWith('Note moved to trash') expect(onFrontmatterPersisted).toHaveBeenCalledTimes(1) }) + + it('final toast is contextual, not "Property updated"', async () => { + const { result } = setup() + const toastCalls: (string | null)[] = [] + setToastMessage.mockImplementation((msg: string | null) => toastCalls.push(msg)) + + await act(async () => { + await result.current.handleTrashNote('/vault/note/test.md') + }) + + // The only toast should be "Note moved to trash", never "Property updated" + expect(toastCalls).toEqual(['Note moved to trash']) + }) }) describe('handleRestoreNote', () => { @@ -86,8 +99,8 @@ describe('useEntryActions', () => { await result.current.handleRestoreNote('/vault/note/test.md') }) - expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed', false) - expect(handleDeleteProperty).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed at') + expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed', false, { silent: true }) + expect(handleDeleteProperty).toHaveBeenCalledWith('/vault/note/test.md', 'Trashed at', { silent: true }) expect(updateEntry).toHaveBeenCalledWith('/vault/note/test.md', { trashed: false, trashedAt: null, @@ -105,11 +118,23 @@ describe('useEntryActions', () => { await result.current.handleArchiveNote('/vault/note/test.md') }) - expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'archived', true) + expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'archived', true, { silent: true }) expect(updateEntry).toHaveBeenCalledWith('/vault/note/test.md', { archived: true }) expect(setToastMessage).toHaveBeenCalledWith('Note archived') expect(onFrontmatterPersisted).toHaveBeenCalledTimes(1) }) + + it('final toast is contextual, not "Property updated"', async () => { + const { result } = setup() + const toastCalls: (string | null)[] = [] + setToastMessage.mockImplementation((msg: string | null) => toastCalls.push(msg)) + + await act(async () => { + await result.current.handleArchiveNote('/vault/note/test.md') + }) + + expect(toastCalls).toEqual(['Note archived']) + }) }) describe('handleUnarchiveNote', () => { @@ -120,7 +145,7 @@ describe('useEntryActions', () => { await result.current.handleUnarchiveNote('/vault/note/test.md') }) - expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'archived', false) + expect(handleUpdateFrontmatter).toHaveBeenCalledWith('/vault/note/test.md', 'archived', false, { silent: true }) expect(updateEntry).toHaveBeenCalledWith('/vault/note/test.md', { archived: false }) expect(setToastMessage).toHaveBeenCalledWith('Note unarchived') expect(onFrontmatterPersisted).toHaveBeenCalledTimes(1) diff --git a/src/hooks/useEntryActions.ts b/src/hooks/useEntryActions.ts index ec296c9c..4f074699 100644 --- a/src/hooks/useEntryActions.ts +++ b/src/hooks/useEntryActions.ts @@ -1,11 +1,12 @@ import { useCallback } from 'react' import type { VaultEntry } from '../types' +import type { FrontmatterOpOptions } from './frontmatterOps' interface EntryActionsConfig { entries: VaultEntry[] updateEntry: (path: string, updates: Partial) => void - handleUpdateFrontmatter: (path: string, key: string, value: string | number | boolean | string[]) => Promise - handleDeleteProperty: (path: string, key: string) => Promise + handleUpdateFrontmatter: (path: string, key: string, value: string | number | boolean | string[], options?: FrontmatterOpOptions) => Promise + handleDeleteProperty: (path: string, key: string, options?: FrontmatterOpOptions) => Promise setToastMessage: (msg: string | null) => void createTypeEntry: (typeName: string) => Promise onFrontmatterPersisted?: () => void @@ -34,8 +35,8 @@ export function useEntryActions({ setToastMessage('Note moved to trash') const now = new Date().toISOString().slice(0, 10) try { - await handleUpdateFrontmatter(path, 'Trashed', true) - await handleUpdateFrontmatter(path, 'Trashed at', now) + await handleUpdateFrontmatter(path, 'Trashed', true, { silent: true }) + await handleUpdateFrontmatter(path, 'Trashed at', now, { silent: true }) onFrontmatterPersisted?.() } catch (err) { updateEntry(path, { trashed: false, trashedAt: null }) @@ -49,8 +50,8 @@ export function useEntryActions({ updateEntry(path, { trashed: false, trashedAt: null }) setToastMessage('Note restored from trash') try { - await handleUpdateFrontmatter(path, 'Trashed', false) - await handleDeleteProperty(path, 'Trashed at') + await handleUpdateFrontmatter(path, 'Trashed', false, { silent: true }) + await handleDeleteProperty(path, 'Trashed at', { silent: true }) onFrontmatterPersisted?.() } catch (err) { updateEntry(path, { trashed: true, trashedAt: Date.now() / 1000 }) @@ -65,7 +66,7 @@ export function useEntryActions({ updateEntry(path, { archived: true }) setToastMessage('Note archived') try { - await handleUpdateFrontmatter(path, 'archived', true) + await handleUpdateFrontmatter(path, 'archived', true, { silent: true }) onFrontmatterPersisted?.() } catch (err) { updateEntry(path, { archived: false }) @@ -79,7 +80,7 @@ export function useEntryActions({ updateEntry(path, { archived: false }) setToastMessage('Note unarchived') try { - await handleUpdateFrontmatter(path, 'archived', false) + await handleUpdateFrontmatter(path, 'archived', false, { silent: true }) onFrontmatterPersisted?.() } catch (err) { updateEntry(path, { archived: true }) diff --git a/src/hooks/useNoteActions.ts b/src/hooks/useNoteActions.ts index 3593c9df..83940523 100644 --- a/src/hooks/useNoteActions.ts +++ b/src/hooks/useNoteActions.ts @@ -10,7 +10,7 @@ import { useNoteRename, performRename, loadNoteContent, renameToastMessage, reloadTabsAfterRename, } from './useNoteRename' -import { runFrontmatterAndApply } from './frontmatterOps' +import { runFrontmatterAndApply, type FrontmatterOpOptions } from './frontmatterOps' export interface NoteActionsConfig { addEntry: (entry: VaultEntry) => void @@ -114,8 +114,8 @@ export function useNoteActions(config: NoteActionsConfig) { ) const runFrontmatterOp = useCallback( - (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue) => - runFrontmatterAndApply(op, path, key, value, { updateTab: updateTabContent, updateEntry, toast: setToastMessage }), + (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue, options?: FrontmatterOpOptions) => + runFrontmatterAndApply(op, path, key, value, { updateTab: updateTabContent, updateEntry, toast: setToastMessage }, options), [updateTabContent, updateEntry, setToastMessage], ) @@ -130,8 +130,8 @@ export function useNoteActions(config: NoteActionsConfig) { handleOpenDailyNote: creation.handleOpenDailyNote, handleCreateType: creation.handleCreateType, createTypeEntrySilent: creation.createTypeEntrySilent, - handleUpdateFrontmatter: useCallback(async (path: string, key: string, value: FrontmatterValue) => { - await runFrontmatterOp('update', path, key, value) + handleUpdateFrontmatter: useCallback(async (path: string, key: string, value: FrontmatterValue, options?: FrontmatterOpOptions) => { + await runFrontmatterOp('update', path, key, value, options) if (shouldRenameOnTitleUpdate(key, value)) { try { await renameAfterTitleChange(path, value, { @@ -143,7 +143,7 @@ export function useNoteActions(config: NoteActionsConfig) { } } }, [runFrontmatterOp, config.vaultPath, config.replaceEntry, rename.tabsRef, setTabs, activeTabPathRef, handleSwitchTab, setToastMessage, updateTabContent]), - handleDeleteProperty: useCallback((path: string, key: string) => runFrontmatterOp('delete', path, key), [runFrontmatterOp]), + handleDeleteProperty: useCallback((path: string, key: string, options?: FrontmatterOpOptions) => runFrontmatterOp('delete', path, key, undefined, options), [runFrontmatterOp]), handleAddProperty: useCallback((path: string, key: string, value: FrontmatterValue) => runFrontmatterOp('update', path, key, value), [runFrontmatterOp]), handleRenameNote: rename.handleRenameNote, } diff --git a/tests/smoke/trash-archive-ui-consistency.spec.ts b/tests/smoke/trash-archive-ui-consistency.spec.ts new file mode 100644 index 00000000..821d8d2d --- /dev/null +++ b/tests/smoke/trash-archive-ui-consistency.spec.ts @@ -0,0 +1,98 @@ +import { test, expect } from '@playwright/test' +import { openCommandPalette, executeCommand } from './helpers' + +const NOTE_ITEM = '[data-testid="note-list-container"] .cursor-pointer' + +test.describe('Trash/archive state consistency across UI', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/') + await page.waitForLoadState('networkidle') + }) + + test('trashing a note shows TrashedNoteBanner and updates Properties', async ({ page }) => { + // Select first note in the list + const firstNote = page.locator(NOTE_ITEM).first() + await firstNote.click() + await page.waitForTimeout(500) + + // Verify no trash banner initially + await expect(page.locator('[data-testid="trashed-note-banner"]')).not.toBeVisible() + + // Trash via command palette + await openCommandPalette(page) + await executeCommand(page, 'Trash Note') + await page.waitForTimeout(500) + + // Banner should appear + await expect(page.locator('[data-testid="trashed-note-banner"]')).toBeVisible({ timeout: 3000 }) + + // Toast should say "Note moved to trash", NOT "Property updated" + const toast = page.locator('.fixed.bottom-8') + await expect(toast).toContainText(/moved to trash/i, { timeout: 3000 }) + }) + + test('archiving a note shows ArchivedNoteBanner', async ({ page }) => { + // Select first note in the list + const firstNote = page.locator(NOTE_ITEM).first() + await firstNote.click() + await page.waitForTimeout(500) + + // Verify no archive banner initially + await expect(page.locator('[data-testid="archived-note-banner"]')).not.toBeVisible() + + // Archive via command palette + await openCommandPalette(page) + await executeCommand(page, 'Archive Note') + await page.waitForTimeout(500) + + // Banner should appear + await expect(page.locator('[data-testid="archived-note-banner"]')).toBeVisible({ timeout: 3000 }) + }) + + test('raw editor shows updated frontmatter after trash', async ({ page }) => { + // Select first note + const firstNote = page.locator(NOTE_ITEM).first() + await firstNote.click() + await page.waitForTimeout(500) + + // Switch to raw editor mode + await openCommandPalette(page) + await executeCommand(page, 'Toggle Raw Editor') + await page.waitForTimeout(300) + + const rawEditor = page.locator('[data-testid="raw-editor-codemirror"]') + await expect(rawEditor).toBeVisible({ timeout: 3000 }) + + // Trash the note + await openCommandPalette(page) + await executeCommand(page, 'Trash Note') + await page.waitForTimeout(500) + + // Raw editor should show Trashed: true in frontmatter + const editorText = await rawEditor.textContent() + expect(editorText).toContain('Trashed') + }) + + test('Changes list updates after trashing a note', async ({ page }) => { + const sidebar = page.locator('.app__sidebar') + const changesRow = sidebar.locator('div', { hasText: /^Changes/ }).first() + await changesRow.waitFor({ timeout: 5000 }) + + const badge = changesRow.locator('span').last() + const initialCount = Number(await badge.textContent()) + + // Select and trash a note + const firstNote = page.locator(NOTE_ITEM).first() + await firstNote.click() + await page.waitForTimeout(300) + + await openCommandPalette(page) + await executeCommand(page, 'Trash Note') + + // Badge count should increase + await expect(async () => { + const text = await badge.textContent() + expect(Number(text)).toBeGreaterThan(initialCount) + }).toPass({ timeout: 5000 }) + }) +})