diff --git a/web/src/lib/components/editor/Editor.svelte b/web/src/lib/components/editor/Editor.svelte index a3e6c8d23..fae0a6c24 100644 --- a/web/src/lib/components/editor/Editor.svelte +++ b/web/src/lib/components/editor/Editor.svelte @@ -26,17 +26,46 @@ import CodeBlock from '@tiptap/extension-code-block'; import Placeholder from '@tiptap/extension-placeholder'; import { copyToClipboard } from '$lib/utils/clipboard'; + import { + mermaidThemeForMode, + collectMermaidRerenders, + type MermaidTheme, + } from './mermaidTheme'; // Serialized mermaid render queue — mermaid can't handle concurrent renders let mermaidMod: typeof import('mermaid') | null = null; let renderQueue: Promise = Promise.resolve(); + // BUG-3106: the mode read and the redraw set live in mermaidTheme.ts so + // both are reachable from a test; the comments there carry the reasoning. + function currentMermaidTheme(): MermaidTheme { + if (typeof document === 'undefined') return 'dark'; + return mermaidThemeForMode(document.documentElement); + } + + // Watches 's data-theme; armed in onMount, disconnected in onDestroy. + let themeObserver: MutationObserver | null = null; + + // The theme the loaded module was last initialized with. mermaid caches + // its config, so a theme change needs a fresh initialize() AND a re-render + // — the SVG already in the DOM carries the old palette baked in. + let appliedMermaidTheme: MermaidTheme | null = null; + async function initMermaid() { if (!mermaidMod) { mermaidMod = await import('mermaid'); + } + const theme = currentMermaidTheme(); + if (theme !== appliedMermaidTheme) { mermaidMod.default.initialize({ startOnLoad: false, - theme: 'dark', + // BUG-3106: follows the app's mode. This was an unconditional + // 'dark', which drew every diagram on the dark palette in light + // mode. The TASK-3090 comment below called it "OUR value, not a + // default" and kept it through the 12 bump for continuity — + // true of the BUMP, but it was never a deliberate choice to + // ignore the app's mode, just a value nobody had revisited. + theme, securityLevel: 'strict', fontFamily: 'inherit', // `layout` is the load-bearing one. Read from the shipped @@ -46,9 +75,7 @@ // on an upgrade nobody asked them about. `look` is a defensive // pin, not a fix: both versions already default to "classic", // and it is written down so a later default change cannot move - // our diagrams silently. (`theme` stays "dark" above for the - // same continuity reason — it is OUR value, not a default, and - // both versions default to "default".) + // our diagrams silently. // This is the site default, not a ceiling: a diagram that opts // into ELK in its own frontmatter still gets ELK. // @@ -61,10 +88,20 @@ layout: 'dagre', look: 'classic', }); + appliedMermaidTheme = theme; } return mermaidMod; } + // Re-render every diagram currently in this editor on the new palette. + // The SVG already in the DOM has the old palette baked in, so a fresh + // initialize() alone changes nothing that is already on screen. + function rerenderMermaidForTheme(root: HTMLElement) { + for (const { source, target } of collectMermaidRerenders(root)) { + queueMermaidRender(source, target); + } + } + function queueMermaidRender(source: string, target: HTMLElement) { renderQueue = renderQueue.then(async () => { try { @@ -883,6 +920,23 @@ onMount(() => { if (!element) return; + // BUG-3106: watch the app's mode so diagrams follow it. + // + // A MutationObserver on 's data-theme rather than a hook in the + // togglers, because there are FOUR of them (+layout's initializer, + // TopBar, Sidebar, YouSheet) and they share no store — only the + // attribute. Observing the attribute covers all four, and covers the + // next one without it having to know diagrams exist. + const themeRoot = document.documentElement; + themeObserver = new MutationObserver(() => { + if (currentMermaidTheme() === appliedMermaidTheme) return; + if (element) rerenderMermaidForTheme(element); + }); + themeObserver.observe(themeRoot, { + attributes: true, + attributeFilter: ['data-theme'], + }); + // Resolve the workspace slug at mount time. The Editor lives inside // a route that has page.params.workspace set; falling back to the // workspace store covers code paths where the editor is rendered @@ -1249,6 +1303,8 @@ onDestroy(() => { editor?.destroy(); + themeObserver?.disconnect(); + themeObserver = null; // onDestroy (unlike onMount) also runs during SSR, where `window` is // undefined — guard with typeof before touching it (TASK-2109). if (typeof window !== 'undefined' && window.visualViewport && visualViewportHandler) { diff --git a/web/src/lib/components/editor/mermaidTheme.svelte.test.ts b/web/src/lib/components/editor/mermaidTheme.svelte.test.ts new file mode 100644 index 000000000..3f8436054 --- /dev/null +++ b/web/src/lib/components/editor/mermaidTheme.svelte.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect } from 'vitest'; +import { mermaidThemeForMode, collectMermaidRerenders } from './mermaidTheme'; + +// BUG-3106: initMermaid passed theme:'dark' unconditionally, so every diagram +// was drawn on the dark palette in light mode. + +function html(attr: string | null): Element { + const el = document.createElement('html'); + if (attr !== null) el.setAttribute('data-theme', attr); + return el; +} + +describe('mermaidThemeForMode', () => { + // The bug itself: this is the case that was wrong, and it is the one + // assertion that fails against the unfixed code. + it('light mode asks for the light palette', () => { + expect(mermaidThemeForMode(html('light'))).toBe('default'); + }); + + it('explicit dark mode asks for the dark palette', () => { + expect(mermaidThemeForMode(html('dark'))).toBe('dark'); + }); + + // The asymmetry worth pinning. +layout only ever SETS data-theme — for a + // light preference, or from a saved value — and leaves it OFF for the + // default, which is dark. A `=== 'dark'` predicate would read absent as + // light and break the common case on a fresh browser, so this is the leg + // that discriminates between the correct predicate and the plausible + // wrong one. + it('treats an ABSENT attribute as dark, not as light', () => { + expect(mermaidThemeForMode(html(null))).toBe('dark'); + }); + + // An unrecognised value is the app's default mode, not a third palette. + it('treats an unrecognised value as dark', () => { + expect(mermaidThemeForMode(html('solarized'))).toBe('dark'); + expect(mermaidThemeForMode(html(''))).toBe('dark'); + }); + + it('survives being called with no root (SSR)', () => { + expect(mermaidThemeForMode(null)).toBe('dark'); + expect(mermaidThemeForMode(undefined)).toBe('dark'); + }); +}); + +describe('collectMermaidRerenders', () => { + function wrapper(opts: { source: string; error?: boolean }): string { + return ` +
+ 
${opts.source}
+
+
`; + } + + function root(inner: string): HTMLElement { + const el = document.createElement('div'); + el.innerHTML = inner; + return el; + } + + it('returns each renderable diagram with its own source', () => { + const r = root(wrapper({ source: 'graph TD; A-->B;' }) + wrapper({ source: 'sequenceDiagram' })); + const got = collectMermaidRerenders(r); + expect(got.map((g) => g.source)).toEqual(['graph TD; A-->B;', 'sequenceDiagram']); + // Each target is the diagram node from its OWN wrapper — a pairing bug + // would redraw one diagram's source into another's node. + const wrappers = r.querySelectorAll('.mermaid-wrapper'); + expect(got[0].target).toBe(wrappers[0].querySelector('.mermaid-diagram')); + expect(got[1].target).toBe(wrappers[1].querySelector('.mermaid-diagram')); + }); + + it('skips a diagram showing a syntax error', () => { + const r = root(wrapper({ source: 'not a diagram', error: true })); + expect(collectMermaidRerenders(r)).toEqual([]); + }); + + it('skips an empty or blank source', () => { + const r = root(wrapper({ source: '' }) + wrapper({ source: ' \n ' })); + expect(collectMermaidRerenders(r)).toEqual([]); + }); + + // The error leg must skip for the RIGHT reason: a wrapper identical except + // for the error class is collected, so the empty result above is the class + // and not the fixture. + it('collects the same wrapper once the error class is gone', () => { + const r = root(wrapper({ source: 'graph TD; A-->B;', error: true })); + expect(collectMermaidRerenders(r)).toEqual([]); + r.querySelector('.mermaid-diagram')!.classList.remove('mermaid-error'); + expect(collectMermaidRerenders(r)).toHaveLength(1); + }); + + it('ignores non-mermaid code blocks', () => { + const r = root(` + 
func main() {}
`); + expect(collectMermaidRerenders(r)).toEqual([]); + }); +}); diff --git a/web/src/lib/components/editor/mermaidTheme.ts b/web/src/lib/components/editor/mermaidTheme.ts new file mode 100644 index 000000000..cd7558497 --- /dev/null +++ b/web/src/lib/components/editor/mermaidTheme.ts @@ -0,0 +1,58 @@ +// BUG-3106: mermaid's palette follows the app's light/dark mode. +// +// Extracted from Editor.svelte so the two load-bearing decisions — which +// theme the current mode calls for, and which diagrams a mode change has to +// redraw — are reachable from a test. Editor.svelte imports both. + +export type MermaidTheme = 'dark' | 'default'; + +/** + * Which mermaid theme the app's CURRENT mode calls for. + * + * There is no theme store to read. Four components (+layout's initializer, + * TopBar, Sidebar, YouSheet) each toggle `data-theme` on and mirror it + * to localStorage['pad-theme']; the ATTRIBUTE is the one thing all four agree + * on, so it is what this reads. + * + * ABSENT MEANS DARK, and that asymmetry is the app's rather than ours: + * +layout only ever SETS the attribute — to 'light' for a light preference, + * or to the saved value when there is one — and leaves it off entirely for + * the default. So the predicate has to be `!== 'light'`. Writing `=== 'dark'` + * would send the default mode, which is the common case on a fresh browser, + * down the light branch and draw every diagram on the wrong palette. + */ +export function mermaidThemeForMode(root: Element | null | undefined): MermaidTheme { + if (!root) return 'dark'; + return root.getAttribute('data-theme') === 'light' ? 'default' : 'dark'; +} + +/** + * The diagrams a theme change has to redraw, within one editor. + * + * Each mermaid code block's NodeView builds the same DOM shape: a + * `.mermaid-wrapper` holding the source `code.language-mermaid` and the + * `.mermaid-diagram` the SVG lands in. That shape is the only handle on the + * live set — ProseMirror creates the NodeViews and nothing enumerates them. + * + * Two kinds of wrapper are SKIPPED, both deliberately: + * - one whose diagram carries `mermaid-error`, because it is showing the + * "Invalid Mermaid syntax" message and a re-render would replace that + * message with the identical message, flickering an error the user is + * already reading; + * - one whose source is empty or blank, which has nothing to draw. + */ +export function collectMermaidRerenders( + root: Element, +): Array<{ source: string; target: HTMLElement }> { + const out: Array<{ source: string; target: HTMLElement }> = []; + for (const wrapper of root.querySelectorAll('.mermaid-wrapper')) { + const diagram = wrapper.querySelector('.mermaid-diagram'); + const code = wrapper.querySelector('code.language-mermaid'); + if (!(diagram instanceof HTMLElement) || !code) continue; + if (diagram.classList.contains('mermaid-error')) continue; + const source = code.textContent ?? ''; + if (source.trim() === '') continue; + out.push({ source, target: diagram }); + } + return out; +}