Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions scripts/externalChangeReload.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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*=/);
});
92 changes: 92 additions & 0 deletions scripts/titlebarToolbar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
normalizeTitlebarToolbarHidden,
normalizeTitlebarToolbarOrder,
normalizeTitlebarToolbarPlacement,
visibleTitlebarActionIds,
} from '../src/lib/utils/titlebarToolbar.js';

test('normalizeTitlebarToolbarOrder drops unknown ids, deduplicates, and appends defaults', () => {
Expand Down Expand Up @@ -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'));
});
20 changes: 20 additions & 0 deletions scripts/viewerKeymap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
3 changes: 2 additions & 1 deletion src/lib/MarkdownViewer.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
62 changes: 11 additions & 51 deletions src/lib/components/TitleBar.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions src/lib/utils/shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
83 changes: 83 additions & 0 deletions src/lib/utils/titlebarToolbar.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { hasMarkdownLinkExtension } from './markdownLinks.js';

export type TitlebarToolbarPlacement = 'bar' | 'menu';

type TitlebarToolbarAction = {
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions src/lib/utils/viewerKeymap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type ViewerCommand =
| 'close-window'
| 'toggle-split-view'
| 'toggle-edit-view'
| 'toggle-live-mode'
| 'save-as'
| 'save'
| 'undo-close-tab'
Expand Down Expand Up @@ -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
Expand Down
Loading