diff --git a/scripts/externalChangeReload.spec.ts b/scripts/externalChangeReload.spec.ts index aa553d74..85d8779f 100644 --- a/scripts/externalChangeReload.spec.ts +++ b/scripts/externalChangeReload.spec.ts @@ -188,3 +188,48 @@ test('turning Live Mode on installs the watcher without reloading', () => { const body = sliceBetween(viewer, 'function toggleLiveMode', '\n\t}'); assert.doesNotMatch(body, /loadMarkdown/); }); + +// --- #692: the editor is where the reload has to land --- + +test('reloading a clean tab does not throw the user out of the editor', async () => { + // The gate on the Auto-Reload button used to hide it in edit mode, so this + // path was reachable only by enabling Live Mode in the preview and then + // pressing Edit. Now that editing is the case the button is FOR, a reload + // that dropped the tab back to preview would be the visible bug. + // + // `loadMarkdown(path)` with no options is exactly the call the + // `file-changed` listener makes for a `reload` outcome. + const session = makeSession(); + const tab = open('/notes/live.md', 'before'); + tab.isEditing = true; + + handleInvoke = (cmd) => { + if (cmd === 'canonicalize_path') return '/notes/live.md'; + // An editing pane always gets the whole file, never the 5MB preview + // slice — an editor bound to a partial buffer auto-saves it back over + // the document's tail. + if (cmd === 'read_file_content_checked') return ['after', false, 'UTF-8']; + throw new Error(`unexpected invoke: ${cmd}`); + }; + + await session.loadMarkdown('/notes/live.md'); + + assert.equal(tabManager.activeTab?.rawContent, 'after', 'the disk version did not arrive'); + assert.equal(tabManager.activeTab?.isEditing, true, 'the reload left the editor'); + assert.equal(tabManager.activeTab?.isDirty, false, 'the reloaded buffer is not an edit'); +}); + +test('entering split view no longer turns Live Mode off behind the user', () => { + // #692. Split used to kill live mode on the way in, with no comment and no + // way back — the setting was silently dropped and stayed dropped after + // leaving. That line arrived with the original split-view commit and reads + // as a consequence of the Auto-Reload button being hidden there (nothing + // left to turn it off with) rather than a decision that a split pane should + // not follow the file. The button and the chord are now offered in all + // three modes; a kill here would take the state straight back off. + // + // An absence claim about a component that cannot be imported, so it is + // matched as source text — the same reason as the four assertions above. + const body = sliceBetween(viewer, 'async function toggleSplitView', '\n\t}'); + assert.doesNotMatch(body, /toggleLiveMode|liveMode\s*=/); +}); diff --git a/scripts/titlebarToolbar.test.ts b/scripts/titlebarToolbar.test.ts index 3ebe75d5..1f422770 100644 --- a/scripts/titlebarToolbar.test.ts +++ b/scripts/titlebarToolbar.test.ts @@ -9,6 +9,7 @@ import { normalizeTitlebarToolbarHidden, normalizeTitlebarToolbarOrder, normalizeTitlebarToolbarPlacement, + visibleTitlebarActionIds, } from '../src/lib/utils/titlebarToolbar.js'; test('normalizeTitlebarToolbarOrder drops unknown ids, deduplicates, and appends defaults', () => { @@ -71,3 +72,94 @@ test('titlebar toolbar reorder helpers resolve drag and keyboard moves', () => { assert.equal(getTitlebarToolbarReorderMove(order, 'back', 'back'), null); assert.equal(getTitlebarToolbarAdjacentMove(order, 'back', 'up'), null); }); + +/* + * #692: editing a file in Markpad while VS Code writes the same file, and + * Markpad not noticing until the mode was toggled. + * + * Auto-Reload is what that user wanted and it already existed. Its two + * surfaces disagreed about where it existed, and their answers were exact + * complements: the button was drawn only in the preview (`!isEditing`, + * `!isSplit`), while `Mod+L` was only an `editorAction` in shortcuts.ts and + * therefore only on Monaco — which exists only in edit and split. Whether a + * user got the feature depended on which surface they happened to find, and + * pressing the chord in the editor armed the watcher with nothing on screen + * to show it had. + * + * The answer is one condition, in one place, with the chord routed through + * the same `toggleLiveMode` (`shortcutRegistry.test.ts` holds the chord end + * up, `externalChangeReload.spec.ts` holds the reload end). + */ + +const documentContext = { + hasActiveTab: true, + showHome: false, + currentFile: '/notes/a.md', + isSplit: false, + isEditing: false, +}; + +test('Auto-Reload is offered in every mode that has a file on disk', () => { + // An external writer can surprise all three equally: what varies between + // them is which pane is on screen, not whether the file can change. + for (const mode of [ + { label: 'preview' }, + { label: 'edit', isEditing: true }, + { label: 'split', isSplit: true }, + ]) { + assert.ok( + visibleTitlebarActionIds({ ...documentContext, ...mode }).includes('live'), + `Auto-Reload was missing in ${mode.label} mode`, + ); + } +}); + +test('Auto-Reload needs a file on disk to watch', () => { + assert.ok( + !visibleTitlebarActionIds({ ...documentContext, currentFile: '' }).includes('live'), + ); +}); + +test('the rest of the toolbar still answers to the mode it is in', () => { + const view = visibleTitlebarActionIds(documentContext); + const edit = visibleTitlebarActionIds({ ...documentContext, isEditing: true }); + const split = visibleTitlebarActionIds({ ...documentContext, isSplit: true }); + + // Find: Monaco owns Ctrl+F in pure edit mode, so the preview's Find hides + // there and comes back in split, where a preview is on screen again. + assert.deepEqual([view, edit, split].map((ids) => ids.includes('find')), [true, false, true]); + // The formatting toolbar is the mirror image: only where a pane can write. + assert.deepEqual([view, edit, split].map((ids) => ids.includes('editorToolbar')), [false, true, true]); + // Sync Scroll and Swap Panes both need two panes; Edit is meaningless once + // both are showing. + assert.deepEqual([view, edit, split].map((ids) => ids.includes('sync')), [false, false, true]); + assert.deepEqual([view, edit, split].map((ids) => ids.includes('swap')), [false, false, true]); + assert.deepEqual([view, edit, split].map((ids) => ids.includes('edit')), [true, true, false]); +}); + +test('a non-Markdown file gets none of the Markdown actions', () => { + // `.txt` would not do here: it is in MARKDOWN_LINK_EXTENSIONS. + const ids = visibleTitlebarActionIds({ ...documentContext, currentFile: '/notes/data.json' }); + for (const id of ['toc', 'fullWidth', 'live', 'split', 'edit', 'find']) { + assert.ok(!ids.includes(id), `${id} was offered for a .json file`); + } + // An unsaved buffer has no extension to read and is treated as Markdown. + assert.ok(visibleTitlebarActionIds({ ...documentContext, currentFile: '' }).includes('split')); +}); + +test('the home screen offers only the actions that are not about a document', () => { + assert.deepEqual(visibleTitlebarActionIds({ ...documentContext, showHome: true }), [ + 'theme', + 'settings', + ]); + assert.deepEqual(visibleTitlebarActionIds({ ...documentContext, hasActiveTab: false }), [ + 'theme', + 'settings', + ]); +}); + +test('Reset Zoom appears only away from 100%', () => { + assert.ok(!visibleTitlebarActionIds({ ...documentContext, zoomLevel: 100 }).includes('zoom')); + assert.ok(!visibleTitlebarActionIds(documentContext).includes('zoom')); + assert.ok(visibleTitlebarActionIds({ ...documentContext, zoomLevel: 125 }).includes('zoom')); +}); diff --git a/scripts/viewerKeymap.spec.ts b/scripts/viewerKeymap.spec.ts index 73f8e2dc..66ce043b 100644 --- a/scripts/viewerKeymap.spec.ts +++ b/scripts/viewerKeymap.spec.ts @@ -154,3 +154,23 @@ test('every command the dispatcher can name is one the component can run', () => assert.ok(reachable.size >= 10, `only ${reachable.size} commands were reached; the sweep is not running`); for (const command of reachable) assert.ok(table[command], `runViewerCommand has no case for ${command}`); }); + +test('Mod+L reaches Auto-Reload from every mode, including the preview', () => { + // #692. The chord used to exist only as an `editorAction`, so Monaco owned + // it and it did nothing in the preview — the exact inverse of where the + // Auto-Reload button was drawn, while the shortcut panel advertised it in + // all three modes regardless. The button now appears in all three; this is + // the other surface agreeing. + const modL = chord('l', 'KeyL', { ctrlKey: true }); + assert.equal(viewerCommandFor(modL, READING), 'toggle-live-mode'); + assert.equal( + viewerCommandFor(modL, { ...READING, isEditing: true, editorHasFocus: true }), + 'toggle-live-mode', + ); + assert.equal(viewerCommandFor(modL, { ...READING, isSplit: true }), 'toggle-live-mode'); + + // And it is Mod+L, not the cross product Mod+Shift+L / Mod+Alt+L. + assert.equal(viewerCommandFor(chord('l', 'KeyL', { ctrlKey: true, shiftKey: true }), READING), null); + assert.equal(viewerCommandFor(chord('l', 'KeyL', { ctrlKey: true, altKey: true }), READING), null); + assert.equal(viewerCommandFor(chord('l', 'KeyL'), READING), null); +}); diff --git a/src/lib/MarkdownViewer.svelte b/src/lib/MarkdownViewer.svelte index ca734c2f..15a8d639 100644 --- a/src/lib/MarkdownViewer.svelte +++ b/src/lib/MarkdownViewer.svelte @@ -2854,7 +2854,6 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu return; } tabManager.setSplitEnabled(tab.id, true); - if (liveMode) toggleLiveMode(); } else { // Closing split view is the same move as leaving edit mode, and it // gets the same treatment: the surviving pane renders the buffer, @@ -2921,6 +2920,8 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu case 'toggle-split-view': if (tabManager.activeTabId) toggleSplitView(tabManager.activeTabId); return; + case 'toggle-live-mode': + return void toggleLiveMode(); case 'toggle-edit-view': // The `silentSave` argument this used to pass meant "suppress the // unsaved-changes modal on the hotkey path". There is no modal on a diff --git a/src/lib/components/TitleBar.svelte b/src/lib/components/TitleBar.svelte index 8cf8b013..8fc0cccb 100644 --- a/src/lib/components/TitleBar.svelte +++ b/src/lib/components/TitleBar.svelte @@ -9,10 +9,9 @@ import { tabManager } from '../stores/tabs.svelte.js'; import { settings } from '../stores/settings.svelte.js'; import { t } from '../utils/i18n.js'; - import { getConfiguredTitlebarToolbarIds } from '../utils/titlebarToolbar.js'; + import { getConfiguredTitlebarToolbarIds, visibleTitlebarActionIds } from '../utils/titlebarToolbar.js'; import { modifierFor, shortcutLabel } from '../utils/shortcuts.js'; import { platformOf } from '../utils/platform.js'; - import { hasMarkdownLinkExtension } from '../utils/markdownLinks.js'; import { hasExportableDocument, hasRealFilePath } from '../utils/tabFileActions.js'; import { getVersion } from '@tauri-apps/api/app'; @@ -327,55 +326,16 @@ } }); - let visibleActionIds = $derived.by(() => { - const list: string[] = []; - - if (tabManager.activeTab && !showHome) { - list.push('back'); - list.push('forward'); - if (currentFile) list.push('reload'); - - // An unsaved buffer has no name to read an extension off, and is - // treated as Markdown — which is what the old inline default of `'md'` - // said, spelled as the condition it actually is. - const isMarkdown = currentFile ? hasMarkdownLinkExtension(currentFile) : true; - - if (isMarkdown) { - list.push('toc'); - list.push('fullWidth'); - if (!tabManager.activeTab?.isSplit && !isEditing && currentFile) { - list.push('live'); - } - if (tabManager.activeTab?.isSplit) { - list.push('sync'); - // Only a split has two panes to put in an order, so the - // control that orders them exists only there. - list.push('swap'); - } - list.push('split'); - } - if (isMarkdown && !tabManager.activeTab?.isSplit) { - list.push('edit'); - } - // Find in preview: only meaningful when a preview is actually - // visible (view mode or split). In pure edit mode Monaco's own - // Ctrl+F handles search, so we hide the entry there. - if (isMarkdown && (!isEditing || tabManager.activeTab?.isSplit)) { - list.push('find'); - } - if (isEditing || tabManager.activeTab?.isSplit) { - list.push('editorToolbar'); - } - list.push('zen'); - list.push('tabs'); - } - - if (zoomLevel && zoomLevel !== 100) list.push('zoom'); - list.push('theme'); - list.push('settings'); - - return list; - }); + let visibleActionIds = $derived.by(() => + visibleTitlebarActionIds({ + hasActiveTab: Boolean(tabManager.activeTab), + showHome, + currentFile, + isSplit: Boolean(tabManager.activeTab?.isSplit), + isEditing, + zoomLevel, + }), + ); let configuredActionIds = $derived.by(() => getConfiguredTitlebarToolbarIds( diff --git a/src/lib/utils/shortcuts.ts b/src/lib/utils/shortcuts.ts index 962e53ca..7cbf5349 100644 --- a/src/lib/utils/shortcuts.ts +++ b/src/lib/utils/shortcuts.ts @@ -348,7 +348,12 @@ export const SHORTCUTS: readonly ShortcutEntry[] = [ labelKey: 'menu.toggleLiveMode', chords: ['Mod+L'], group: 'view', + // Both halves, like Mod+E: Monaco's own Ctrl+L is `expandLineSelection`, + // so the editor action is what stops the chord selecting a line, and the + // document command is what makes it work in the preview, where there is + // no Monaco to register anything on. editorAction: true, + documentCommands: ['toggle-live-mode'], }, { id: 'view-toggle-split', diff --git a/src/lib/utils/titlebarToolbar.ts b/src/lib/utils/titlebarToolbar.ts index 8c260f78..4e37db2c 100644 --- a/src/lib/utils/titlebarToolbar.ts +++ b/src/lib/utils/titlebarToolbar.ts @@ -1,3 +1,5 @@ +import { hasMarkdownLinkExtension } from './markdownLinks.js'; + export type TitlebarToolbarPlacement = 'bar' | 'menu'; type TitlebarToolbarAction = { @@ -144,6 +146,87 @@ export function applyTitlebarToolbarMove(order: readonly string[], move: Titleba return next; } +export type TitlebarActionContext = { + hasActiveTab: boolean; + showHome: boolean; + currentFile: string; + isSplit: boolean; + isEditing: boolean; + zoomLevel?: number; +}; + +/** + * Which actions the current document offers, before the user's own order, + * hiding and bar/menu placement are applied to them. + * + * This used to be a `$derived.by` inside TitleBar.svelte, where nothing could + * call it — and one of its conditions was wrong for as long as that was true. + * The Auto-Reload button was drawn only in the preview (`!isEditing`, + * `!isSplit`) while its chord, Mod+L, was only an `editorAction` in + * shortcuts.ts and therefore only on Monaco, which exists only in the other + * two modes. The two surfaces of one feature never both applied, and the + * shortcut panel advertised the chord in all three regardless. Editing a file + * that another program also writes — the case auto-reload is for — got the + * chord and no indicator. + * + * Its output already fed `getConfiguredTitlebarToolbarIds` below, so this is + * the first half of a pipeline moving next to the second, not a new layer. + */ +export function visibleTitlebarActionIds(context: TitlebarActionContext): string[] { + const list: string[] = []; + + if (context.hasActiveTab && !context.showHome) { + list.push('back'); + list.push('forward'); + if (context.currentFile) list.push('reload'); + + // An unsaved buffer has no name to read an extension off, and is + // treated as Markdown — which is what the old inline default of `'md'` + // said, spelled as the condition it actually is. + const isMarkdown = context.currentFile + ? hasMarkdownLinkExtension(context.currentFile) + : true; + + if (isMarkdown) { + list.push('toc'); + list.push('fullWidth'); + // Every mode that has a file on disk, which is every mode an + // external writer can surprise. The chord answers the same + // question the same way — see the note above. + if (context.currentFile) { + list.push('live'); + } + if (context.isSplit) { + list.push('sync'); + // Only a split has two panes to put in an order, so the + // control that orders them exists only there. + list.push('swap'); + } + list.push('split'); + } + if (isMarkdown && !context.isSplit) { + list.push('edit'); + } + // Find in preview: only meaningful when a preview is actually + // visible (view mode or split). In pure edit mode Monaco's own + // Ctrl+F handles search, so we hide the entry there. + if (isMarkdown && (!context.isEditing || context.isSplit)) { + list.push('find'); + } + if (context.isEditing || context.isSplit) { + list.push('editorToolbar'); + } + list.push('zen'); + list.push('tabs'); + } + + if (context.zoomLevel && context.zoomLevel !== 100) list.push('zoom'); + list.push('theme'); + list.push('settings'); + + return list; +} + export function getConfiguredTitlebarToolbarIds( availableIds: readonly string[], order: readonly string[] | null | undefined, diff --git a/src/lib/utils/viewerKeymap.ts b/src/lib/utils/viewerKeymap.ts index 33346eac..2eed2a02 100644 --- a/src/lib/utils/viewerKeymap.ts +++ b/src/lib/utils/viewerKeymap.ts @@ -58,6 +58,7 @@ export type ViewerCommand = | 'close-window' | 'toggle-split-view' | 'toggle-edit-view' + | 'toggle-live-mode' | 'save-as' | 'save' | 'undo-close-tab' @@ -244,6 +245,11 @@ export function viewerCommandFor(e: KeyStroke, context: KeyContext): ViewerComma if (mod && key === 'q') return 'close-window'; if (mod && (code === 'Backslash' || code === 'IntlBackslash')) return 'toggle-split-view'; if (mod && key === 'e') return 'toggle-edit-view'; + // Mod+L had only the Monaco half (`editorAction` in shortcuts.ts), so it + // fired in the editor and in split view and did nothing in the preview — + // the exact inverse of where the Auto-Reload button was drawn (#692). The + // panel advertised the chord in every mode regardless. + if (mod && key === 'l') return 'toggle-live-mode'; // Save As. The app menu advertised this chord for as long as the menu has // existed, but nothing ever bound it: the branch below matched on // `cmdOrCtrl && key === 's'` with no Shift guard, so the advertised