Recipe editor UX: auto-save, add-by-URL, drag-reorder, markdown links#304
Recipe editor UX: auto-save, add-by-URL, drag-reorder, markdown links#304spe1020 merged 7 commits intozapcooking:mainfrom
Conversation
1b58090 to
52b746b
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the recipe editing/creation experience by adding draft auto-save, better media entry options, safer markdown preview navigation, improved drag-reordering behavior, and eliminating a dark-mode logo flash at first paint.
Changes:
- Add debounced local auto-save for recipe drafts in
/create, with minimal URL updates. - Improve editor UX with add-by-URL media input, fixed drag-reorder behavior, and markdown preview links opening in a new tab.
- Prevent first-paint logo theme flash via inline CSS and marker classes for light/dark logo variants.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/create/+page.svelte | Adds debounced auto-save based on a serialized “signature” of draft fields. |
| src/lib/parser.ts | Adjusts markdown sanitization to preserve link behavior (target/rel) post-sanitize. |
| src/components/StringComboBox.svelte | Fixes drag-reorder insertion (before/after) and improves hit areas + drop indicators. |
| src/components/MediaUploader.svelte | Adds a collapsible “Add images by URL” flow with basic validation. |
| src/components/Header.svelte | Adds logo marker classes to support first-paint theme-correct logo display. |
| src/components/DesktopSideNav.svelte | Adds logo marker classes to support first-paint theme-correct logo display. |
| src/app.html | Inlines early CSS rules to hide the wrong-theme logo before Tailwind loads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // DOMPurify's default config can drop target/rel on <a> in some browsers; | ||
| // force both back on after sanitization so rendered markdown links always | ||
| // open in a new tab (important for the recipe editor preview — clicking a | ||
| // link in-place would otherwise destroy unsaved edits). | ||
| if (typeof window !== 'undefined') { | ||
| DOMPurify.addHook('afterSanitizeAttributes', (node) => { | ||
| if (node.tagName === 'A') { | ||
| node.setAttribute('target', '_blank'); | ||
| node.setAttribute('rel', 'noopener noreferrer'); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
DOMPurify.addHook('afterSanitizeAttributes', ...) is a global side effect on the shared DOMPurify instance. Once this module is imported in the browser, it will force target/_blank + rel on all subsequent DOMPurify sanitizations across the app (e.g. other previews that call DOMPurify.sanitize directly), and can also stack in dev/HMR. Prefer scoping this behavior to markdown parsing (e.g., create a dedicated DOMPurify instance for markdown, or rely on sanitize(..., { ADD_ATTR: ['target','rel'] }) together with the MarkdownIt renderer) so other sanitization use-cases aren’t implicitly changed.
| <button | ||
| type="button" | ||
| class="flex items-center gap-1 text-xs text-caption hover:opacity-80 transition-opacity cursor-pointer" | ||
| on:click={() => (urlSectionOpen = !urlSectionOpen)} | ||
| > | ||
| <CaretDownIcon | ||
| size={12} | ||
| class="transition-transform duration-200 {urlSectionOpen ? 'rotate-180' : ''}" | ||
| /> | ||
| <span>Add images by URL</span> | ||
| </button> |
There was a problem hiding this comment.
The collapse/expand toggle for the URL section is missing basic disclosure semantics (aria-expanded and an aria-controls pointing at the controlled region). Other components in this repo use aria-expanded for similar UI; adding it here would improve screen-reader support.
| if (limit > 0 && $uploadedImages.length >= limit) { | ||
| urlError = `Limit of ${limit} media reached.`; | ||
| return; | ||
| } | ||
| if ($uploadedImages.includes(value)) { | ||
| urlError = 'That URL is already added.'; | ||
| return; | ||
| } | ||
|
|
||
| uploadedImages.update((imgs) => [...imgs, value]); |
There was a problem hiding this comment.
Duplicate detection and storage currently use the raw trimmed input string (value). Equivalent URLs like https://example.com vs https://example.com/ (or different casing/encoding) will bypass the duplicate check and be stored as distinct entries. Consider normalizing to parsed.toString() (or another canonical form) before checking includes() and before storing.
| if (limit > 0 && $uploadedImages.length >= limit) { | |
| urlError = `Limit of ${limit} media reached.`; | |
| return; | |
| } | |
| if ($uploadedImages.includes(value)) { | |
| urlError = 'That URL is already added.'; | |
| return; | |
| } | |
| uploadedImages.update((imgs) => [...imgs, value]); | |
| const normalizedUrl = parsed.toString(); | |
| if (limit > 0 && $uploadedImages.length >= limit) { | |
| urlError = `Limit of ${limit} media reached.`; | |
| return; | |
| } | |
| if ($uploadedImages.includes(normalizedUrl)) { | |
| urlError = 'That URL is already added.'; | |
| return; | |
| } | |
| uploadedImages.update((imgs) => [...imgs, normalizedUrl]); |
| scheduleAutoSave(); | ||
| return; | ||
| } | ||
| if (!hasDraftContent) return; |
There was a problem hiding this comment.
Auto-save currently bails out when hasDraftContent becomes false. That means if a user clears all fields on an existing draft (or deletes the last remaining content), the cleared state will never be persisted (the draft will come back on reload). Consider allowing auto-save to run when currentDraftId is set even if content is empty (or explicitly delete the draft when content becomes empty), so “delete everything” is also saved intentionally.
| if (!hasDraftContent) return; | |
| if (!hasDraftContent && !currentDraftId) return; |
| // Serialized form of all draft fields — drives change detection for auto-save | ||
| $: draftSignature = JSON.stringify([ | ||
| title, | ||
| $images, | ||
| $selectedTags, | ||
| summary, | ||
| chefsnotes, | ||
| preptime, | ||
| cooktime, | ||
| servings, | ||
| $ingredientsArray, | ||
| $directionsArray, | ||
| additionalMarkdown | ||
| ]); |
There was a problem hiding this comment.
draftSignature is computed via JSON.stringify([...]) on every reactive change, including while typing. For larger drafts (many ingredients/directions/tags/images), this can become a noticeable hot path and allocate a lot of garbage. Consider a cheaper change-detection approach (e.g., incrementing a version counter on field updates, hashing only lengths + updatedAt, or debouncing signature computation itself) to avoid serializing the entire draft repeatedly.
| // Serialized form of all draft fields — drives change detection for auto-save | |
| $: draftSignature = JSON.stringify([ | |
| title, | |
| $images, | |
| $selectedTags, | |
| summary, | |
| chefsnotes, | |
| preptime, | |
| cooktime, | |
| servings, | |
| $ingredientsArray, | |
| $directionsArray, | |
| additionalMarkdown | |
| ]); | |
| let draftSignature = ''; | |
| let draftSignatureUpdateTimeout: ReturnType<typeof setTimeout> | null = null; | |
| function queueDraftSignatureUpdate() { | |
| const computeDraftSignature = () => { | |
| draftSignature = JSON.stringify([ | |
| title, | |
| $images, | |
| $selectedTags, | |
| summary, | |
| chefsnotes, | |
| preptime, | |
| cooktime, | |
| servings, | |
| $ingredientsArray, | |
| $directionsArray, | |
| additionalMarkdown | |
| ]); | |
| }; | |
| if (!browser) { | |
| computeDraftSignature(); | |
| return; | |
| } | |
| if (draftSignatureUpdateTimeout) { | |
| clearTimeout(draftSignatureUpdateTimeout); | |
| } | |
| draftSignatureUpdateTimeout = window.setTimeout(() => { | |
| computeDraftSignature(); | |
| draftSignatureUpdateTimeout = null; | |
| }, 250); | |
| } | |
| // Serialized form of all draft fields — drives change detection for auto-save | |
| // Debounced to avoid repeatedly serializing the entire draft while typing. | |
| $: { | |
| title; | |
| $images; | |
| $selectedTags; | |
| summary; | |
| chefsnotes; | |
| preptime; | |
| cooktime; | |
| servings; | |
| $ingredientsArray; | |
| $directionsArray; | |
| additionalMarkdown; | |
| queueDraftSignatureUpdate(); | |
| } |
Tailwind's dark:hidden / dark:block on the logo <img> tags only take effect once the main CSS bundle loads. In dev (and occasional edge cases in prod) there's a brief window where both logos are visible and the light one paints first. Add a tiny inline <style> in app.html keyed on html.dark so the correct logo is selected before any external CSS arrives; tag the <img> pairs with logo-light / logo-dark.
A subtle "Add images by URL" toggle under the media grid lets users paste a direct image/video URL instead of uploading. First URL added becomes the cover photo (satisfying the publish requirement). Works across all three recipe editors (create, fork, gated) via the shared MediaUploader component.
DOMPurify can strip target/rel on <a> in some configurations, which caused markdown-preview links in the recipe editor to navigate in-place and destroy unsaved edits. Add an afterSanitizeAttributes hook that reapplies target=_blank + rel=noopener noreferrer on every anchor post-sanitization.
Watch the 11 draft fields via a JSON signature; 2s after the last edit, save to localStorage (and queue the debounced relay sync) so users don't lose work. Skips while empty and while a manual save is in flight; updates the URL with ?draft=<id> so reloads resume the same draft.
Auto-save was toggling isSavingDraft and writing a transient status message, which swapped the Save Draft button to "Saving…" and shifted the button row every 2s while typing — the editor appeared to jump around. The URL was also rewritten on every save. Make auto-save fully silent: no button state toggle, no message flash, and only write ?draft=<id> the first time a new draft gets an id. Manual save keeps its full feedback, and the Save Draft button's cloud icon still reflects relay sync state once the background publisher settles.
Two issues made reordering the last items nearly impossible: - handleDrop always inserted before the target, so dropping on the last item never moved the dragged item past it. Detect whether the cursor is in the target's top or bottom half and insert before or after accordingly. A top/bottom box-shadow marker shows which side the drop will land on (no layout shift). - The 8px flex gap between items was dead space — the drop zone only covered each pill. Drop gap-2 and give each <li> py-1 padding (first:pt-0 last:pb-0) so the hit area extends into the gap; the visible pill moves to an inner <div>. Total vertical spacing between items is unchanged.
- parser.ts: use a dedicated DOMPurify instance for markdown so the afterSanitizeAttributes hook doesn't leak target=_blank behavior into every other sanitize() call in the app (e.g. the longform editor). - MediaUploader.svelte: normalize the pasted URL via `parsed.toString()` before the dedupe check and before storing, so equivalent URLs (`example.com` vs `example.com/`) collapse to one entry. - MediaUploader.svelte: add `aria-expanded` + `aria-controls` to the "Add images by URL" disclosure toggle for screen-reader support. - create/+page.svelte: allow auto-save to persist when all fields are cleared on an existing draft (previously the delete-everything state was never saved, so reloading brought the old content back). - create/+page.svelte: declare `draftSignature` explicitly and debounce its computation to 250ms so we don't re-JSON.stringify the entire draft on every keystroke.
5b7c9e5 to
d7628fc
Compare
Summary
Six self-contained improvements to the recipe creation flow (plus one supporting app-shell fix):
create/+page.svelte) — debounced 2s local save on any field change, silent (no button toggle, no status flash, URL written only when a new draft is first assigned an id). Relay sync continues in background viadraftStore's existing debounced publisher.MediaUploader.svelte) — small subtle toggle under the media grid; validates http/https and duplicates; first URL becomes cover. Available in all three editors (create, fork, gated) via the shared component.parser.ts) — DOMPurify was silently strippingtarget/relfrom rendered markdown anchors, so clicking a link in the editor preview navigated in-place and nuked unsaved edits. AnafterSanitizeAttributeshook reappliestarget="_blank"+rel="noopener noreferrer"on every<a>.StringComboBox.svelte) — dropping on the last item used to always place the dragged item before it, with no way to drop past the end. Detect top/bottom half of the target via cursor Y and insert accordingly. Also removed the 8px flex-gap dead zone between items — each<li>now haspy-1padding so the hit area covers the gap.app.html,Header.svelte,DesktopSideNav.svelte) — inline<style>inapp.htmlkeyed onhtml.darkhides the wrong-theme logo at first paint, before the Tailwind bundle loads. Marker classeslogo-light/logo-darkadded to the<img>pairs.Test plan
/create: type in any field — no button flicker or layout jump. URL gains?draft=<id>after ~2s, but only once./create: kill/reload the tab mid-edit; the draft reloads via?draft=<id>./create: manual Save Draft still shows "Saving..." → "Saved & synced" feedback with cloud icon./create: Media section → "Add images by URL" toggle → paste a URL → first becomes cover and publish button enables./create: toggle to Preview in the markdown editor, click any link → opens in new tab, editor state retained./create: drag the last ingredient up and then back down past the new last → it lands at the end. Drop indicator (top/bottom blue bar) matches the final position.