diff --git a/.changeset/editor-action-bar.md b/.changeset/editor-action-bar.md new file mode 100644 index 00000000..99fe526c --- /dev/null +++ b/.changeset/editor-action-bar.md @@ -0,0 +1,8 @@ +--- +"@luzzle/web": minor +"@luzzle/web.theme": patch +--- + +Put Save, Preview, Publish and More in the editor frame. Keep saved-file publishing +available without stale index gating and preselect the current piece. Preserve +newer drafts during saves and give editor commands an explicit shared appearance. diff --git a/apps/web/src/lib/components/editor/MarkdownEditor.svelte b/apps/web/src/lib/components/editor/MarkdownEditor.svelte index 0fb15962..4a0af411 100644 --- a/apps/web/src/lib/components/editor/MarkdownEditor.svelte +++ b/apps/web/src/lib/components/editor/MarkdownEditor.svelte @@ -1,5 +1,5 @@ -
+
+ {#if toolbar} +
+ {@render toolbar()} +
+ {/if} +
+
diff --git a/apps/web/src/lib/components/editor/PieceToolbar.test.ts b/apps/web/src/lib/components/editor/PieceToolbar.test.ts new file mode 100644 index 00000000..ea915133 --- /dev/null +++ b/apps/web/src/lib/components/editor/PieceToolbar.test.ts @@ -0,0 +1,99 @@ +import { createRawSnippet, type ComponentProps } from 'svelte' +import { render } from 'svelte/server' +import { expect, test, vi } from 'vitest' +import PieceToolbar from './PieceToolbar.svelte' + +vi.mock('$app/navigation', () => ({ goto: vi.fn() })) + +const file = 'books/源 #?%.books.md' +const children = createRawSnippet(() => ({ render: () => 'More fixture' })) +const disabledAttribute = /(?:^|\s)disabled(?:\s|=|$)/ + +function html(props: Partial> = {}) { + return render(PieceToolbar, { + props: { + file, + saveFormId: 'save-piece', + isDirty: false, + isSaving: false, + hasPublishedVersion: true, + children, + ...props + } + }).body +} + +function buttonAttributes(output: string, label: string): string { + const buttons = [...output.matchAll(/]*)>([\s\S]*?)<\/button>/g)] + const button = buttons.find((match) => match[2].replace(//g, '').trim() === label) + if (!button) throw new Error(`Missing button: ${label}`) + return button[1] +} + +test.each([false, true])( + 'saved files keep Preview and Publish visible and available with published version=%s', + (hasPublishedVersion) => { + const output = html({ hasPublishedVersion }) + expect(buttonAttributes(output, 'Preview')).not.toMatch(disabledAttribute) + expect(buttonAttributes(output, 'Publish')).not.toMatch(disabledAttribute) + expect(buttonAttributes(output, 'Save')).toMatch(disabledAttribute) + expect(output).toContain('More fixture') + expect(output).toContain('Saved') + expect(output).toContain('aria-live="polite"') + } +) + +test('unsaved edits keep primary actions visible with a short status', () => { + const output = html({ isDirty: true }) + expect(buttonAttributes(output, 'Save')).not.toMatch(disabledAttribute) + expect(buttonAttributes(output, 'Preview')).toMatch(disabledAttribute) + expect(buttonAttributes(output, 'Publish')).toMatch(disabledAttribute) + expect(output).toContain('Unsaved changes') +}) + +test.each([false, true])('saving disables primary actions even when dirty=%s', (isDirty) => { + const output = html({ isDirty, isSaving: true }) + for (const label of ['Saving…', 'Preview', 'Publish']) { + expect(buttonAttributes(output, label)).toMatch(disabledAttribute) + } + expect(output).not.toContain('Unsaved changes') +}) + +test.each([ + { isDirty: false, isSaving: false }, + { isDirty: true, isSaving: false }, + { isDirty: false, isSaving: true }, + { isDirty: true, isSaving: true } +])('editor commands retain their explicit appearance with %o', (state) => { + const output = html(state) + for (const label of [state.isSaving ? 'Saving…' : 'Save', 'Preview', 'Publish']) { + expect(buttonAttributes(output, label)).toContain('data-variant="editor"') + } +}) + +test('Save submits the source form instead of navigating or implicitly publishing', () => { + const attributes = buttonAttributes(html({ isDirty: true }), 'Save') + expect(attributes).toContain('type="submit"') + expect(attributes).toContain('form="save-piece"') + for (const label of ['Preview', 'Publish']) { + expect(buttonAttributes(html(), label)).toContain('type="button"') + } +}) + +test('publication status is an encoded link only when a public version exists', () => { + const href = '/admin/piece/books/%E6%BA%90%20%23%3F%25.books.md/published' + expect(html()).toContain(`href="${href}"`) + const unpublished = html({ hasPublishedVersion: false }) + expect(unpublished).toContain('Not published') + expect(unpublished).not.toContain(href) +}) + +test('editor actions do not duplicate the surrounding archive navigation', () => { + const output = html() + expect(output).toContain('aria-label="Editor actions"') + expect(output).not.toContain('href="/admin"') + expect(output).not.toContain('Back to archive') + expect(output).not.toContain('Publish…') +}) + +// SSR proves rendering and disabled/form semantics, not click handling or navigation guards. diff --git a/apps/web/src/lib/components/editor/PieceView.svelte b/apps/web/src/lib/components/editor/PieceView.svelte deleted file mode 100644 index 8eaeaeef..00000000 --- a/apps/web/src/lib/components/editor/PieceView.svelte +++ /dev/null @@ -1,122 +0,0 @@ - - - - View - - - {#snippet child({ open, props, wrapperProps })} - {#if open} -
-
- goto(publishedHref(file))} - > - {#snippet child({ props })} -
- Published - {#if !availability.published} - - {#if isSaving}Saving…{:else}Not published yet{/if} - - {/if} -
- {/snippet} - - goto(previewHref(file))} - > - {#snippet child({ props })} -
- Preview - {#if !availability.preview} - Saving… - {/if} -
- {/snippet} - -
-
- {/if} - {/snippet} - - - - - diff --git a/apps/web/src/lib/components/editor/PieceView.test.ts b/apps/web/src/lib/components/editor/PieceView.test.ts deleted file mode 100644 index 2e1a4c1a..00000000 --- a/apps/web/src/lib/components/editor/PieceView.test.ts +++ /dev/null @@ -1,108 +0,0 @@ -import { readFileSync } from 'node:fs' -import { expect, test, vi } from 'vitest' -import { render } from 'svelte/server' -import PieceView from './PieceView.svelte' - -vi.mock('$app/navigation', () => ({ goto: vi.fn() })) -const source = readFileSync(new URL('./PieceView.svelte', import.meta.url), 'utf8') - -// Bits portals do not render menu contents in SSR. These tests establish the -// trigger and source contracts, not popup visibility, clicks, warnings or focus. -test.each([false, true])('SSR keeps View available while saving=%s', (isSaving) => { - const { body } = render(PieceView, { - props: { file: 'books/a #?.md', hasPublishedVersion: false, isSaving } - }) - expect(body.replace(//g, '')).toMatch( - /]*aria-haspopup="menu"[^>]*>View<\/button>/ - ) - expect(body).not.toMatch(/]*disabled/) -}) - -test('both literal items use native disabled state and current-prop navigation callbacks', () => { - expect(source).toContain('$derived(getViewAvailability(hasPublishedVersion, isSaving))') - expect(source).toMatch(/class="dropdown-content">\s*\s* goto(${helper}(file))}`) - } - expect(source).not.toMatch(/ { - const editor = readFileSync( - new URL('../../../routes/(auth)/admin/piece/[...path]/source/+page.svelte', import.meta.url), - 'utf8' - ) - expect(editor).toContain( - '' - ) - expect(editor).toContain( - '$derived(shouldOfferPublish(data.hasPublishedVersion, data.savedMarkdown))' - ) - expect(editor).toContain('$derived(editorHref(data.file))') - for (const action of ['save', 'delete']) - expect(editor).toContain(`action="{sourceUrl}?/${action}"`) - expect(editor).toContain('fetch(`${sourceUrl}?/attach`, {') - expect(editor).toContain( - "if (isDirty && !bypassWarning && navigation.type !== 'form' && navigation.to)" - ) - expect(editor).toContain('navigation.cancel()') - expect(editor).toContain('onclick={confirmDiscard}') -}) - -test('View menu source matches the original actions menu presentation', () => { - const original = readFileSync(new URL('./PieceActions.svelte', import.meta.url), 'utf8') - for (const pattern of [ - /\.dropdown-content\s*\{([^}]+)\}/, - /\.dropdown-item\s*\{([^}]+)\}/, - /\.dropdown-item\[data-disabled\]\s*\{([^}]+)\}/ - ]) { - const expected = original.match(pattern)?.[1] - const actual = source.match(pattern)?.[1] - expect(expected).toBeDefined() - expect(actual).toBeDefined() - expect(actual?.trim()).toBe(expected?.trim()) - } - const hover = original.match(/\.dropdown-item:hover\s*\{([^}]+)\}/)?.[1] - const highlight = source.match(/\.dropdown-item\[data-highlighted\]\s*\{([^}]+)\}/)?.[1] - expect(hover).toBeDefined() - expect(highlight).toBeDefined() - expect(highlight?.trim()).toBe(hover?.trim()) - expect(source).toMatch(/\.dropdown-item:focus-visible\s*\{\s*outline: auto;/) -}) - -test('menuitem source keeps labels and short reasons only while disabled', () => { - const markup = source.replace(/\s+/g, ' ') - expect(source).toContain('const id = $props.id()') - for (const [view, label] of [ - ['published', 'Published'], - ['preview', 'Preview'] - ]) { - expect(source).toContain(`${label}`) - expect(source).toContain(`aria-labelledby={\`\${id}-${view}-label\`}`) - expect(markup).toContain( - `aria-describedby={!availability.${view} ? \`\${id}-${view}-description\` : undefined}` - ) - expect(markup).toContain( - `{#if !availability.${view}} ` - ) - } - expect(source.match(/\{#snippet child\(\{ props \}\)\}\s* { + const content = reconcileSave(submitted, submitted, saved) + expect(content).toBe('A\n') + expect(content === saved.content).toBe(true) +}) + +test('typing B during Save A preserves B against the refreshed A baseline', () => { + const content = reconcileSave({ file, content: 'B' }, submitted, saved) + expect(content).toBe('B') + expect(content !== saved.content).toBe(true) + + // The retained draft can be submitted next, then adopt its own normalization. + const nextSubmission = { file, content } + const nextSaved = { type: 'success' as const, content: 'B\n' } + const nextContent = reconcileSave(nextSubmission, nextSubmission, nextSaved) + expect(nextContent).toBe('B\n') + expect(nextContent === nextSaved.content).toBe(true) +}) + +test('a failed Save echoing A does not replace newer B or mark it saved', () => { + const baseline = 'original source' + const content = reconcileSave({ file, content: 'B' }, submitted, { + type: 'failure', + content: 'A' + }) + expect(content).toBe('B') + expect(content !== baseline).toBe(true) +}) + +test('failure preserves an unchanged submitted draft too', () => { + expect(reconcileSave(submitted, submitted, { type: 'failure', content: 'error A' })).toBe('A') + expect(reconcileSave(submitted, submitted, { type: 'failure' })).toBe('A') +}) + +test('clearing the buffer while Save is pending remains an unsaved edit', () => { + const content = reconcileSave({ file, content: '' }, submitted, saved) + expect(content).toBe('') + expect(content !== saved.content).toBe(true) +}) + +test('an old file response cannot replace the newly loaded file, even with identical text', () => { + const current = { file: 'other.note.md', content: 'A' } + expect(reconcileSave(current, submitted, saved)).toBe(current.content) + expect(reconcileSave(current, submitted, { type: 'failure', content: 'error A' })).toBe( + current.content + ) +}) + +test('pristine same-file refreshes adopt each new saved baseline', () => { + const refreshed = { file, content: 'B' } + const content = reconcileSourceRefresh('A', submitted, refreshed, false) + expect(content).toBe('B') + expect(content).toBe(refreshed.content) + + const nextRefresh = { file, content: 'C' } + expect(reconcileSourceRefresh(content, refreshed, nextRefresh, false)).toBe('C') +}) + +test('same-file refreshes preserve dirty drafts against the latest loaded baseline', () => { + const refreshed = { file, content: 'B' } + const content = reconcileSourceRefresh('draft', submitted, refreshed, false) + expect(content).toBe('draft') + expect(content).not.toBe(refreshed.content) + + // Returning to historical A is still dirty against the now-loaded B baseline. + const nextRefresh = { file, content: 'C' } + expect(reconcileSourceRefresh('A', refreshed, nextRefresh, false)).toBe('A') +}) + +test('a pending Save defers even a pristine buffer refresh to save reconciliation', () => { + const refreshed = { file, content: saved.content } + const content = reconcileSourceRefresh('A', submitted, refreshed, true) + expect(content).toBe('A') + expect(reconcileSave({ file, content }, submitted, saved)).toBe(refreshed.content) +}) + +test('typing B survives refresh during Save A and either save outcome', () => { + const refreshed = { file, content: saved.content } + const content = reconcileSourceRefresh('B', submitted, refreshed, true) + expect(content).toBe('B') + expect(reconcileSave({ file, content }, submitted, saved)).toBe('B') + expect(reconcileSave({ file, content }, submitted, { type: 'failure', content: 'A' })).toBe('B') + expect(content).not.toBe(refreshed.content) +}) + +test('navigation always loads the new file despite a dirty buffer or pending Save', () => { + const refreshed = { file: 'other.note.md', content: 'other source' } + expect(reconcileSourceRefresh('draft', submitted, refreshed, false)).toBe('other source') + const content = reconcileSourceRefresh('draft', submitted, refreshed, true) + expect(content).toBe('other source') + expect(reconcileSave({ file: refreshed.file, content }, submitted, saved)).toBe(content) +}) diff --git a/apps/web/src/lib/components/editor/save-state.ts b/apps/web/src/lib/components/editor/save-state.ts new file mode 100644 index 00000000..1e183d6e --- /dev/null +++ b/apps/web/src/lib/components/editor/save-state.ts @@ -0,0 +1,30 @@ +export type SaveSnapshot = { file: string; content: string } + +export function reconcileSourceRefresh( + content: string, + previous: SaveSnapshot, + refreshed: SaveSnapshot, + savePending: boolean +): string { + if (refreshed.file !== previous.file || (!savePending && content === previous.content)) { + return refreshed.content + } + return content +} + +type SaveResult = { type: 'success'; content: string } | { type: 'failure'; content?: string } + +export function reconcileSave( + current: SaveSnapshot, + submitted: SaveSnapshot, + result: SaveResult +): string { + if ( + result.type === 'failure' || + current.file !== submitted.file || + current.content !== submitted.content + ) { + return current.content + } + return result.content +} diff --git a/apps/web/src/lib/components/editor/view-state.test.ts b/apps/web/src/lib/components/editor/view-state.test.ts deleted file mode 100644 index 6c8d532c..00000000 --- a/apps/web/src/lib/components/editor/view-state.test.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { expect, test } from 'vitest' -import { MarkdownPublicationStatus } from '$lib/pieces/types' -import { getViewAvailability, shouldOfferPublish } from './view-state' - -test.each([ - [false, false, { published: false, preview: true }], - [true, false, { published: true, preview: true }], - [false, true, { published: false, preview: false }], - [true, true, { published: false, preview: false }] -])('availability: published row=%s, saving=%s', (hasPublishedVersion, isSaving, expected) => { - expect(getViewAvailability(hasPublishedVersion, isSaving)).toEqual(expected) -}) - -test.each([ - [false, MarkdownPublicationStatus.MATCHING, true], - [false, MarkdownPublicationStatus.CHANGED, true], - [false, MarkdownPublicationStatus.UNKNOWN, true], - [true, MarkdownPublicationStatus.MATCHING, false], - [true, MarkdownPublicationStatus.CHANGED, true], - [true, MarkdownPublicationStatus.UNKNOWN, true] -])('publish offer: published row=%s, status=%s', (hasPublishedVersion, status, expected) => { - expect(shouldOfferPublish(hasPublishedVersion, status)).toBe(expected) -}) diff --git a/apps/web/src/lib/components/editor/view-state.ts b/apps/web/src/lib/components/editor/view-state.ts deleted file mode 100644 index 9007c4ac..00000000 --- a/apps/web/src/lib/components/editor/view-state.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { MarkdownPublicationStatus } from '$lib/pieces/types' - -export function getViewAvailability(hasPublishedVersion: boolean, isSaving: boolean) { - return { - published: hasPublishedVersion && !isSaving, - preview: !isSaving - } -} - -export function shouldOfferPublish( - hasPublishedVersion: boolean, - status: MarkdownPublicationStatus -): boolean { - return !hasPublishedVersion || status !== MarkdownPublicationStatus.MATCHING -} diff --git a/apps/web/src/lib/components/ui/Button.svelte b/apps/web/src/lib/components/ui/Button.svelte index 1f3b0e4f..d852bf6c 100644 --- a/apps/web/src/lib/components/ui/Button.svelte +++ b/apps/web/src/lib/components/ui/Button.svelte @@ -3,7 +3,7 @@ import type { Snippet } from 'svelte' type Props = { - variant?: 'primary' | 'secondary' | 'tertiary' | 'error' | 'outline' | 'link' + variant?: 'primary' | 'secondary' | 'tertiary' | 'error' | 'outline' | 'link' | 'editor' type?: 'button' | 'submit' | 'reset' form?: string onclick?: (e: MouseEvent) => void diff --git a/apps/web/src/lib/pieces/navigation.test.ts b/apps/web/src/lib/pieces/navigation.test.ts index db3b1eb2..4e187213 100644 --- a/apps/web/src/lib/pieces/navigation.test.ts +++ b/apps/web/src/lib/pieces/navigation.test.ts @@ -1,6 +1,6 @@ import { existsSync } from 'node:fs' import { expect, test, vi } from 'vitest' -import { editorHref, getPieceLinks, previewHref, publishedHref } from './navigation' +import { editorHref, getPieceLinks, previewHref, publishedHref, publishHref } from './navigation' const { resolve } = vi.hoisted(() => ({ resolve: vi.fn(() => { @@ -52,6 +52,29 @@ test.each([ } ) +test.each([ + 'books/源 #?%&"+=.books.md', + 'folder with spaces/a book.books.md', + 'nested/literal%2F.books.md', + 'books/📚.books.md', + 'nested/name?check=0&file=other#fragment.books.md' +])('publish navigation preselects the complete raw filename %s', (file) => { + const url = new URL(publishHref(file), 'https://app.test') + expect(url.pathname).toBe('/admin/publish') + expect(url.hash).toBe('') + expect([...url.searchParams.entries()]).toEqual([ + ['check', '1'], + ['file', file] + ]) + expect(resolve).not.toHaveBeenCalled() +}) + +test('publish navigation encodes a query value rather than a pathname', () => { + expect(publishHref('folder/a b+c.books.md')).toBe( + '/admin/publish?check=1&file=folder%2Fa+b%2Bc.books.md' + ) +}) + test.each( ['', '/garden'].flatMap((mount) => ['', '/page', '/icon', '/share'].map((suffix) => ({ mount, suffix })) diff --git a/apps/web/src/lib/pieces/navigation.ts b/apps/web/src/lib/pieces/navigation.ts index bc52746e..5128c6eb 100644 --- a/apps/web/src/lib/pieces/navigation.ts +++ b/apps/web/src/lib/pieces/navigation.ts @@ -10,6 +10,11 @@ export function publishedHref(file: string): string { return `/admin/piece/${piecePath(file)}/published` } +export function publishHref(file: string): string { + const query = new URLSearchParams({ check: '1', file }) + return `/admin/publish?${query}` +} + export function previewHref(file: string, job?: string): string { const href = `/admin/piece/${piecePath(file)}/preview` return job === undefined ? href : `${href}/${encodeURIComponent(job)}` diff --git a/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.server.ts b/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.server.ts index d25afc44..e30c3439 100644 --- a/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.server.ts +++ b/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.server.ts @@ -19,7 +19,6 @@ import { } from '@luzzle/core' import { Readable } from 'stream' import { normalizeLineEndings, normalizeMarkdown } from '$lib/server/markdown' -import { getMarkdownPublicationStatus } from '$lib/pieces/publication.server' async function resolveAssetUrls( piece: Piece, @@ -103,8 +102,7 @@ export const load: PageServerLoad = async ({ params }) => { schema: piece.schema, canGenerate: config.ai !== undefined, assetFields: assetPaths, - hasPublishedVersion: Boolean(webPiece), - savedMarkdown: await getMarkdownPublicationStatus(file, webPiece?.content_hash) + hasPublishedVersion: Boolean(webPiece) } } diff --git a/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.svelte b/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.svelte index 879a203d..253efc4f 100644 --- a/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.svelte +++ b/apps/web/src/routes/(auth)/admin/piece/[...path]/source/+page.svelte @@ -1,10 +1,15 @@