From 0cbe271f6f9a14738aeafb6ede3c5f4d771b033c Mon Sep 17 00:00:00 2001 From: matevip Date: Fri, 8 May 2026 17:09:22 +0800 Subject: [PATCH] fix(ui/chat): mermaid streaming flicker + copy/download buttons --- .../src/components/chat/MessageBubble.vue | 108 +++++- .../src/composables/useMarkdownRenderer.ts | 26 +- .../src/composables/useMermaidRenderer.ts | 365 ++++++++++++++++-- mateclaw-ui/src/views/AgentContext.vue | 2 + mateclaw-ui/src/views/ChatConsole.vue | 6 +- 5 files changed, 474 insertions(+), 33 deletions(-) diff --git a/mateclaw-ui/src/components/chat/MessageBubble.vue b/mateclaw-ui/src/components/chat/MessageBubble.vue index 3647a396..15684c84 100644 --- a/mateclaw-ui/src/components/chat/MessageBubble.vue +++ b/mateclaw-ui/src/components/chat/MessageBubble.vue @@ -1958,25 +1958,125 @@ watch(isGenerating, (generating) => { /* ===== Mermaid block ===== */ .markdown-body :deep(.mermaid-block) { margin: 14px 0; - padding: 16px; border-radius: 12px; background: var(--mc-mermaid-bg, #f8fafc); border: 1px solid var(--mc-mermaid-border, #e2e8f0); + overflow: hidden; +} +.markdown-body :deep(.mermaid-block__header) { + display: flex; + align-items: center; + justify-content: space-between; + height: 38px; + padding: 0 14px; + background: var(--mc-code-header-bg); + border-bottom: 1px solid var(--mc-mermaid-border, #e2e8f0); + font-size: 12px; + line-height: 1; + color: var(--mc-code-lang-color); + /* Prevent the header label/buttons from being swept into a text selection + that starts in the surrounding markdown — the highlighted-grey selection + band would otherwise extend across the whole header row. */ + user-select: none; + -webkit-user-select: none; +} +.markdown-body :deep(.mermaid-block__lang) { + font-weight: 500; + letter-spacing: 0.02em; +} +.markdown-body :deep(.mermaid-block__actions) { + display: inline-flex; + align-items: center; + gap: 4px; +} +.markdown-body :deep(.mermaid-block__download) { + display: inline-flex; + align-items: center; + gap: 4px; + padding: 3px 8px; + background: transparent; + border: none; + border-radius: 6px; + color: var(--mc-code-copy-color); + font-size: 12px; + cursor: pointer; + transition: background 0.15s ease, color 0.15s ease; +} +.markdown-body :deep(.mermaid-block__download:hover) { + background: var(--mc-code-copy-hover-bg); + color: var(--mc-code-copy-hover-color); +} +/* Pin EVERY icon inside the header to 14×14. Without this, DOMPurify can + normalise away the `width="14" height="14"` attrs from the markdown HTML, + leaving the SVG to fall back to the UA default 300×150. The button is + inline-flex so it grows to fit the icon, and `:hover` then paints a + gigantic grey rectangle (which is what user issue #67's follow-up screen- + shot showed). Same defence-in-depth as `.code-block__header svg`. */ +.markdown-body :deep(.mermaid-block__header svg) { + width: 14px !important; + height: 14px !important; + flex-shrink: 0; + display: inline-block; + vertical-align: middle; +} +.markdown-body :deep(.mermaid-block__header > *) { + flex-shrink: 0; + min-width: 0; +} +.markdown-body :deep(.mermaid-block__body) { + padding: 16px; text-align: center; overflow-x: auto; + /* Reserve a stable height so the box doesn't collapse to 0px before the + SVG paints — keeps layout stable across the streaming cache-miss → + render cycle. */ + min-height: 96px; + display: flex; + align-items: center; + justify-content: center; } -.markdown-body :deep(.mermaid-block svg) { +.markdown-body :deep(.mermaid-block__body svg) { max-width: 100%; height: auto; } -.markdown-body :deep(.mermaid-block.mermaid-error) { +.markdown-body :deep(.mermaid-block.mermaid-error .mermaid-block__body) { background: #fef2f2; - border-color: #fecaca; color: #b91c1c; text-align: left; font-family: ui-monospace, SFMono-Regular, Menlo, monospace; font-size: 12px; white-space: pre-wrap; + display: block; +} +/* Streaming placeholder: three pulsing dots inside the empty body. The dots + render the same DOM string on every v-html update (stable innerHTML) so + the box stops "shaking" during streaming. Once the async render fires + after stream end, this gets replaced with the actual SVG. */ +.markdown-body :deep(.mermaid-block__loader) { + display: inline-flex; + gap: 6px; + align-items: center; +} +.markdown-body :deep(.mermaid-block__loader-dot) { + width: 6px; + height: 6px; + border-radius: 50%; + background: var(--mc-mermaid-border, #cbd5e1); + animation: mc-mermaid-pulse 1.4s ease-in-out infinite; +} +.markdown-body :deep(.mermaid-block__loader-dot:nth-child(2)) { + animation-delay: 0.2s; +} +.markdown-body :deep(.mermaid-block__loader-dot:nth-child(3)) { + animation-delay: 0.4s; +} +@keyframes mc-mermaid-pulse { + 0%, 80%, 100% { opacity: 0.3; transform: scale(0.85); } + 40% { opacity: 1; transform: scale(1); } +} +.markdown-body :deep(.mermaid-block__download.is-flash) { + background: var(--mc-warning-bg, rgba(255, 159, 67, 0.15)); + color: var(--mc-warning, #f59e0b); } /* ===== KaTeX inline / block ===== */ diff --git a/mateclaw-ui/src/composables/useMarkdownRenderer.ts b/mateclaw-ui/src/composables/useMarkdownRenderer.ts index a1bfc94d..4df38173 100644 --- a/mateclaw-ui/src/composables/useMarkdownRenderer.ts +++ b/mateclaw-ui/src/composables/useMarkdownRenderer.ts @@ -179,8 +179,29 @@ const customRenderer = { // Mermaid: ship raw source through a placeholder div for the // useMermaidRenderer post-mount step. Skip syntax highlighting entirely. + // The header (lang label + Copy + Download SVG) is part of the placeholder + // so users can copy the diagram source even before render completes; the + // composable paints the SVG into `.mermaid-block__body`. if (infoStr === 'mermaid') { - return `
` + const encoded = encodeURIComponent(rawCode) + const copySvg = `` + const downloadSvg = `` + return `
` + + `
` + + `Mermaid` + + `` + + `` + + `` + + `` + + `
` + + `
` + + `
` } // ECharts: same pattern, mounted by useEChartsRenderer. @@ -298,7 +319,8 @@ const purifyConfig = { ADD_ATTR: [ 'target', 'rel', 'class', 'data-code', 'data-echarts-option', 'data-wiki-title', 'data-slug', - 'data-tex', 'data-mermaid', + 'data-tex', 'data-mermaid', 'data-mermaid-download', + 'x1', 'y1', 'x2', 'y2', 'aria-label', 'open', 'type', 'viewBox', 'fill', 'stroke', 'stroke-width', 'd', 'x', 'y', 'width', 'height', 'rx', 'ry', 'points', diff --git a/mateclaw-ui/src/composables/useMermaidRenderer.ts b/mateclaw-ui/src/composables/useMermaidRenderer.ts index 7573b709..200fcef8 100644 --- a/mateclaw-ui/src/composables/useMermaidRenderer.ts +++ b/mateclaw-ui/src/composables/useMermaidRenderer.ts @@ -31,6 +31,50 @@ async function getMermaid(theme: 'dark' | 'default'): Promise { let renderCounter = 0 +// Module-level cache of rendered SVGs, keyed by raw mermaid source. This is +// the anti-flicker fix for STABLE content (history, theme toggles, scroll): +// streaming markdown updates use Vue's `v-html`, which destroys and recreates +// the entire subtree on every token — including stable `.mermaid-block` +// placeholders whose source hasn't changed. Element identity is therefore +// useless for dedup; the only stable key is the source string itself. On +// every MutationObserver tick we sync-paint from this cache before the +// browser repaints, so the user never sees an empty box for an already +// rendered diagram. +// +// For ACTIVELY STREAMING content the source itself changes every token, so +// the cache always misses. The streaming flicker is fixed separately by +// (a) skipping the async render path while the host message still has a +// `.with-cursor` ancestor, and (b) debouncing the async render so it only +// fires after content has been stable for STABLE_RENDER_DEBOUNCE_MS. +type SvgCacheEntry = { html: string; theme: 'dark' | 'default' } +const SVG_CACHE = new Map() +const SVG_CACHE_CAP = 64 +const STABLE_RENDER_DEBOUNCE_MS = 350 + +function cacheGet(src: string, theme: 'dark' | 'default'): string | null { + const entry = SVG_CACHE.get(src) + if (!entry || entry.theme !== theme) return null + // Refresh LRU position. + SVG_CACHE.delete(src) + SVG_CACHE.set(src, entry) + return entry.html +} + +function cacheSet(src: string, theme: 'dark' | 'default', html: string): void { + if (SVG_CACHE.size >= SVG_CACHE_CAP) { + const oldest = SVG_CACHE.keys().next().value + if (oldest !== undefined) SVG_CACHE.delete(oldest) + } + SVG_CACHE.set(src, { html, theme }) +} + +function isInsideStreaming(el: HTMLElement): boolean { + // `.with-cursor` is set on the assistant `.msg-content` while the message + // is generating (see MessageBubble.vue:141). When it's there, every token + // produces a fresh v-html update — rendering now would just flicker. + return !!el.closest('.msg-content.with-cursor') +} + /** * Composable that observes a container for `.mermaid-block[data-mermaid]` * placeholders (emitted by `useMarkdownRenderer.code()` for ```mermaid fenced @@ -46,17 +90,49 @@ export function useMermaidRenderer(containerRef: Ref) { const mounting = new Set() const tracked = new Set() let observer: MutationObserver | null = null + let asyncTimer: ReturnType | null = null + + function getBody(el: HTMLElement): HTMLElement { + // Renderers built before the header/body split fall back to the wrapper + // itself so cached chat history (rendered HTML in DB) still mounts. + return (el.querySelector('.mermaid-block__body')) || el + } + + function paintLoadingPlaceholder(el: HTMLElement) { + const body = getBody(el) + if (body.dataset.mcLoading === '1') return + // Use a stable inline-svg loader so the body has a non-empty paint that + // doesn't change between mutations — kills the visible "shake". + body.innerHTML = '' + body.dataset.mcLoading = '1' + } + + function tryMountFromCache(el: HTMLElement, src: string, theme: 'dark' | 'default'): boolean { + const cached = cacheGet(src, theme) + if (!cached) return false + const body = getBody(el) + body.innerHTML = cached + delete body.dataset.mcLoading + el.classList.remove('mermaid-error') + el.classList.add('mermaid-ready') + rendered.add(el) + tracked.add(el) + return true + } async function mountBlock(el: HTMLElement) { if (rendered.has(el) || mounting.has(el)) return - mounting.add(el) const src = decodeURIComponent(el.getAttribute('data-mermaid') || '') - if (!src.trim()) { - mounting.delete(el) - return - } + if (!src.trim()) return + + const theme: 'dark' | 'default' = themeStore.isDark ? 'dark' : 'default' + + mounting.add(el) try { - const theme = themeStore.isDark ? 'dark' : 'default' const mermaid = await getMermaid(theme) // Pre-parse so we can fall back gracefully on bad input. Without this, @@ -70,54 +146,146 @@ export function useMermaidRenderer(containerRef: Ref) { } const id = `mc-mermaid-${++renderCounter}` const { svg } = await mermaid.render(id, src) - el.innerHTML = svg - rendered.add(el) - tracked.add(el) + cacheSet(src, theme, svg) + // Re-check the element is still in the DOM — during streaming it may + // have been detached by a fresh v-html update before our async render + // resolved. The cache write above is what matters; future re-creations + // of this source will hit the sync fast path. + if (el.isConnected) { + const body = getBody(el) + body.innerHTML = svg + delete body.dataset.mcLoading + el.classList.add('mermaid-ready') + rendered.add(el) + tracked.add(el) + } } catch (e) { - // Streaming-aware: if the LLM is still emitting tokens the source may - // legitimately be incomplete on each pass. Mark this element done so - // we don't retry on every mutation; when streaming finishes Vue's v-html - // produces a fresh element (different identity) and we'll try again. - console.warn('[MermaidRenderer] failed to render — showing source:', e) - el.classList.add('mermaid-error') - el.textContent = src - rendered.add(el) + // Source either incomplete (still streaming) or genuinely broken. + // Don't mark this element rendered — leave the loading placeholder so + // the next stable scan can retry. We only show the source-as-text + // fallback if the parent message has finished generating, otherwise + // the user briefly sees raw mermaid syntax. + if (!isInsideStreaming(el)) { + console.warn('[MermaidRenderer] failed to render — showing source:', e) + el.classList.add('mermaid-error') + getBody(el).textContent = src + delete getBody(el).dataset.mcLoading + rendered.add(el) + } else { + paintLoadingPlaceholder(el) + } } finally { mounting.delete(el) } } - function scanAndMount() { + /** + * Synchronous pass: paint cache hits and loading placeholders. Runs on + * every MutationObserver tick — must stay cheap and side-effect-free + * besides DOM writes for the blocks it inspects. + */ + function syncPaint() { const container = containerRef.value if (!container) return const blocks = container.querySelectorAll( '.mermaid-block[data-mermaid]:not(.mermaid-error)', ) + const theme: 'dark' | 'default' = themeStore.isDark ? 'dark' : 'default' blocks.forEach((el) => { - if (!rendered.has(el) && !mounting.has(el) && !el.querySelector('svg')) { - mountBlock(el) + if (rendered.has(el) || mounting.has(el)) { + return } + const body = getBody(el) + if (body.querySelector('svg')) { + rendered.add(el) + tracked.add(el) + return + } + const src = decodeURIComponent(el.getAttribute('data-mermaid') || '') + if (!src.trim()) return + if (tryMountFromCache(el, src, theme)) return + // No cache: paint a stable loading placeholder so the empty box doesn't + // flicker between paints. This is what makes streaming feel calm — + // every v-html re-creation lands here and immediately writes the same + // loader markup, so the user sees a steady box, not a strobing one. + paintLoadingPlaceholder(el) }) } + /** + * Slow async pass: kick off mermaid renders for blocks that haven't been + * cached yet. Skipped while the host message is still streaming — running + * mermaid.render on a half-finished source wastes CPU AND visibly flashes + * partial diagrams as elements get destroyed mid-render. + */ + function asyncRenderPass() { + const container = containerRef.value + if (!container) return + const blocks = container.querySelectorAll( + '.mermaid-block[data-mermaid]:not(.mermaid-error):not(.mermaid-ready)', + ) + blocks.forEach((el) => { + if (rendered.has(el) || mounting.has(el)) return + const body = getBody(el) + if (body.querySelector('svg')) { + rendered.add(el) + tracked.add(el) + return + } + if (isInsideStreaming(el)) return + mountBlock(el) + }) + } + + function scheduleAsyncPass() { + if (asyncTimer) clearTimeout(asyncTimer) + asyncTimer = setTimeout(() => { + asyncTimer = null + asyncRenderPass() + }, STABLE_RENDER_DEBOUNCE_MS) + } + function rebuildAll() { // Theme switch: clear rendered SVGs and re-mount with the new theme. + // Don't drop the cache map itself — entries are theme-tagged and the next + // render will overwrite the stale one. tracked.forEach((el) => { - el.innerHTML = '' + const body = getBody(el) + body.innerHTML = '' + delete body.dataset.mcLoading + el.classList.remove('mermaid-ready') rendered.delete(el) }) tracked.clear() initializedTheme = null // force re-init with the new theme - scanAndMount() + syncPaint() + scheduleAsyncPass() } function attachObserver(container: HTMLElement) { observer?.disconnect() observer = new MutationObserver(() => { - nextTick(() => scanAndMount()) + nextTick(() => { + // Sync paint runs every tick — gives stable content cache-hit speed + // and gives streaming content a stable loader (not an empty box). + syncPaint() + // Async render is debounced so streaming tokens don't pile up + // concurrent mermaid.render() calls; kicks in ~350 ms after the last + // mutation, which is well after token cadence and well before a user + // would notice. + scheduleAsyncPass() + }) }) - observer.observe(container, { childList: true, subtree: true }) - scanAndMount() + observer.observe(container, { + childList: true, + subtree: true, + // Watch class changes so removing `.with-cursor` (stream end) triggers + // a re-scan even when no childList mutation accompanies it. + attributes: true, + attributeFilter: ['class'], + }) + syncPaint() + scheduleAsyncPass() } function startObserving() { @@ -144,8 +312,153 @@ export function useMermaidRenderer(containerRef: Ref) { stopThemeWatch() observer?.disconnect() observer = null + if (asyncTimer) clearTimeout(asyncTimer) + asyncTimer = null tracked.clear() } - return { startObserving, dispose, scanAndMount } + return { startObserving, dispose, scanAndMount: () => { syncPaint(); scheduleAsyncPass() } } +} + +/** + * Re-render mermaid source with `htmlLabels: false` so the resulting SVG is + * self-contained (uses `` nodes instead of `
`). + * The on-screen render keeps htmlLabels:true for nicer typography in the + * chat UI; but foreignObject HTML labels (a) depend on the host page's CSS + * variables, which are gone when the SVG is opened standalone, (b) aren't + * rendered at all by macOS Quick Look / Preview, and (c) occasionally trip + * XMLSerializer cross-namespace bugs that drop the inner xhtml xmlns. The + * net effect of all three is the "blank downloaded SVG" the user reports. + */ +async function renderSvgForExport(src: string, theme: 'dark' | 'default'): Promise { + try { + const mermaid = await getMermaid(theme) + const exportSrc = /^\s*%%\{\s*init\s*:/i.test(src) + ? src + : `%%{init: {"flowchart": {"htmlLabels": false}}}%%\n${src}` + const id = `mc-mermaid-export-${++renderCounter}` + const { svg } = await mermaid.render(id, exportSrc) + return svg + } catch (e) { + console.warn('[MermaidRenderer] export re-render failed; falling back to live SVG', e) + return null + } +} + +function ensureExportShape(svgEl: SVGSVGElement, fallbackBox: DOMRect | null) { + if (!svgEl.getAttribute('xmlns')) { + svgEl.setAttribute('xmlns', 'http://www.w3.org/2000/svg') + } + if (!svgEl.getAttribute('xmlns:xlink')) { + svgEl.setAttribute('xmlns:xlink', 'http://www.w3.org/1999/xlink') + } + // Prefer viewBox (intrinsic to the diagram) over the live bounding rect, + // which depends on the chat layout and can be 0 if the message is in a + // collapsed panel. + let width = 0 + let height = 0 + const vb = (svgEl.getAttribute('viewBox') || '').split(/\s+/).map(Number) + if (vb.length === 4 && vb[2] > 0 && vb[3] > 0) { + width = Math.round(vb[2]) + height = Math.round(vb[3]) + } + if ((!width || !height) && fallbackBox) { + width = Math.round(fallbackBox.width) + height = Math.round(fallbackBox.height) + } + if (!width || !height) { + width = 800 + height = 600 + } + svgEl.setAttribute('width', String(width)) + svgEl.setAttribute('height', String(height)) + // Strip mermaid's `style="max-width: 100%"` — fine inside a sized parent, + // collapses to 0 px in standalone viewers. + svgEl.style.removeProperty('max-width') + if (svgEl.getAttribute('style') === '') { + svgEl.removeAttribute('style') + } +} + +/** + * Click handler for `.mermaid-block__download` buttons. Synchronously claims + * the click (returning true so the caller can early-return) and kicks off + * the actual export work async — the export re-renders mermaid with + * htmlLabels:false to produce a portable SVG, hence the async path. + * + * Hosted in this module so callers (ChatConsole, AgentContext) only wire one + * line — matching the existing `handleCodeCopy` pattern. + */ +export function handleMermaidDownload(e: MouseEvent): boolean { + const btn = (e.target as HTMLElement | null)?.closest('.mermaid-block__download') as HTMLElement | null + if (!btn) return false + e.preventDefault() + e.stopPropagation() + void doMermaidDownload(btn) + return true +} + +async function doMermaidDownload(btn: HTMLElement) { + const block = btn.closest('.mermaid-block') as HTMLElement | null + if (!block) return + const liveSvg = block.querySelector('svg') as SVGSVGElement | null + const textEl = btn.querySelector('.mermaid-block__download-text') as HTMLElement | null + if (!liveSvg) { + flashButton(btn, textEl, 'Wait…') + return + } + const originalText = textEl?.textContent || 'SVG' + if (textEl) textEl.textContent = '...' + btn.setAttribute('disabled', 'true') + try { + const src = decodeURIComponent(block.getAttribute('data-mermaid') || '') + const theme: 'dark' | 'default' = initializedTheme + || (document.documentElement.classList.contains('dark') ? 'dark' : 'default') + + let exportSvg: SVGSVGElement | null = null + if (src) { + const svgString = await renderSvgForExport(src, theme) + if (svgString) { + const tmp = document.createElement('div') + tmp.innerHTML = svgString + exportSvg = tmp.querySelector('svg') as SVGSVGElement | null + } + } + // Fallback: clone the live SVG. Will still work for diagrams without + // foreignObject (sequence/class/state diagrams), and degrades gracefully + // for flowcharts (the file at least has the structure). + if (!exportSvg) { + exportSvg = liveSvg.cloneNode(true) as SVGSVGElement + } + + let liveBox: DOMRect | null = null + try { liveBox = liveSvg.getBoundingClientRect() } catch { /* ignore */ } + ensureExportShape(exportSvg, liveBox) + + const xml = new XMLSerializer().serializeToString(exportSvg) + const blob = new Blob([`\n${xml}`], { + type: 'image/svg+xml;charset=utf-8', + }) + const url = URL.createObjectURL(blob) + const a = document.createElement('a') + a.href = url + a.download = `mermaid-${Date.now()}.svg` + document.body.appendChild(a) + a.click() + document.body.removeChild(a) + setTimeout(() => URL.revokeObjectURL(url), 0) + } finally { + btn.removeAttribute('disabled') + if (textEl) textEl.textContent = originalText + } +} + +function flashButton(btn: HTMLElement, textEl: Element | null, text: string) { + const original = textEl?.textContent || '' + if (textEl) textEl.textContent = text + btn.classList.add('is-flash') + setTimeout(() => { + btn.classList.remove('is-flash') + if (textEl && original) textEl.textContent = original + }, 1200) } diff --git a/mateclaw-ui/src/views/AgentContext.vue b/mateclaw-ui/src/views/AgentContext.vue index d69dac7c..ae7349dc 100644 --- a/mateclaw-ui/src/views/AgentContext.vue +++ b/mateclaw-ui/src/views/AgentContext.vue @@ -211,6 +211,7 @@ import { agentApi, agentContextApi } from '@/api/index' import { copyToClipboard } from '@/utils/clipboard' import type { Agent, WorkspaceFile } from '@/types/index' import { useMarkdownRenderer } from '@/composables/useMarkdownRenderer' +import { handleMermaidDownload } from '@/composables/useMermaidRenderer' import { plainTextIcon } from '@/composables/usePixelarticons' const { renderMarkdown } = useMarkdownRenderer() @@ -321,6 +322,7 @@ async function fetchPromptFiles() { } function handlePreviewClick(e: MouseEvent) { + if (handleMermaidDownload(e)) return const btn = (e.target as HTMLElement).closest('.code-block__copy') as HTMLElement | null if (!btn) return // Same as ChatConsole: the copy button now lives inside
diff --git a/mateclaw-ui/src/views/ChatConsole.vue b/mateclaw-ui/src/views/ChatConsole.vue index 65ae2387..cb0cf9ee 100644 --- a/mateclaw-ui/src/views/ChatConsole.vue +++ b/mateclaw-ui/src/views/ChatConsole.vue @@ -334,7 +334,7 @@ import TalkMode from '@/components/chat/TalkMode.vue' import ModelSelector from '@/components/chat/ModelSelector.vue' import { useEChartsRenderer } from '@/composables/useEChartsRenderer' import { useKatexRenderer } from '@/composables/useKatexRenderer' -import { useMermaidRenderer } from '@/composables/useMermaidRenderer' +import { useMermaidRenderer, handleMermaidDownload } from '@/composables/useMermaidRenderer' // ============ Talk Mode ============ const showTalkMode = ref(false) @@ -1731,6 +1731,10 @@ function formatConversationTime(time?: string) { } function handleCodeCopy(e: MouseEvent) { + // Mermaid download button shares the same global click delegation. Handle + // it first so the SVG export beats the copy-button selector below if the + // user happens to click in an area where both ancestors are reachable. + if (handleMermaidDownload(e)) return const btn = (e.target as HTMLElement).closest('.code-block__copy') as HTMLElement | null if (!btn) return // The copy button now sits inside
for collapsible code