From bc62a4e40a39e3899c28a452c4e2a29456ddab22 Mon Sep 17 00:00:00 2001 From: gum798 <33922655+gum798@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:28:43 -0700 Subject: [PATCH] fix(editor): keep preview Add-note controls out of PDF export Exporting Markdown to PDF from Preview printed an Add-note + button above every block. Preview exports the .markdown-body subtree, and its per-block annotation control renders inside that subtree, so the clone-scrub pass never removed it. Mark the controls container with data-orca-export-hide at the source, add the explicit class to UI_ONLY_SELECTORS (attr-strip fallback; generic attr covers renames), and hide it in EXPORT_CSS as a belt-and-suspenders backstop. Review note bodies and open composer drafts are transient review state and are intentionally excluded from the document PDF. Fixes #21198 / STA-7761 Attribution: diagnosis and core scrub entry by @gum798 (PR #21199, closed in favor of this PR) --- .../src/components/editor/export-css.ts | 7 +- .../editor/markdown-export-extract.test.ts | 91 +++++++++++++++++++ .../editor/markdown-export-extract.ts | 10 +- .../editor/markdown-export-html.test.ts | 7 ++ ...-markdown-preview-annotation-renderers.tsx | 6 +- 5 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/components/editor/export-css.ts b/src/renderer/src/components/editor/export-css.ts index b19fe732134..858c45b5a6c 100644 --- a/src/renderer/src/components/editor/export-css.ts +++ b/src/renderer/src/components/editor/export-css.ts @@ -125,12 +125,13 @@ html, body { /* Why: the export subtree selection already excludes the big chrome (toolbar, search bar, etc.), but in-document affordances like the code-copy button - can still leak. Hide the well-known offenders as a belt-and-suspenders - defense on top of DOM scrubbing. */ + and preview annotation controls can still leak. Hide the well-known + offenders as a belt-and-suspenders defense on top of DOM scrubbing. */ .code-block-copy-btn, .markdown-preview-search, +.markdown-annotation-controls, .rich-markdown-toolbar, -[data-orca-export-hide="true"] { +[data-orca-export-hide] { display: none !important; } diff --git a/src/renderer/src/components/editor/markdown-export-extract.test.ts b/src/renderer/src/components/editor/markdown-export-extract.test.ts index f10a8289c44..a1c7a4ad31b 100644 --- a/src/renderer/src/components/editor/markdown-export-extract.test.ts +++ b/src/renderer/src/components/editor/markdown-export-extract.test.ts @@ -66,4 +66,95 @@ describe('getActiveMarkdownExportPayload', () => { }) ).rejects.toThrow('Failed to inline image for PDF export') }) + + it('strips preview annotation controls so Add note buttons never reach the PDF', async () => { + await mockPreviewOpenFile() + const root = document.createElement('div') + // Why: class-only fixture proves the explicit selector scrubs even when + // the data attr is absent; two blocks prove every block is scrubbed. + root.innerHTML = ` +
+
+

Title

+
+ +
+
saved note body
+
+
+
+

Body text

+
+ +
+
+ 
graph TD;
+
` + const payload = await getActiveMarkdownExportPayload({ + fileId: '/repo/docs/readme.md', + root + }) + const exported = parseExportedHtml(payload?.html) + expect(exported.querySelector('h1')?.textContent).toBe('Title') + expect(exported.querySelector('p')?.textContent).toBe('Body text') + expect(exported.querySelector('pre code')?.textContent).toContain('graph TD;') + expect(exported.querySelector('.markdown-annotation-controls')).toBeNull() + expect(exported.querySelector('.markdown-annotation-add')).toBeNull() + expect(exported.querySelector('.markdown-annotation-composer')).toBeNull() + expect(exported.querySelector('.markdown-annotation-note-stack')).toBeNull() + expect(exported.textContent).not.toContain('draft note') + expect(exported.textContent).not.toContain('saved note body') + // Why: scrub runs on a clone; the live preview keeps its controls. + expect(root.querySelector('.markdown-annotation-controls')).not.toBeNull() + }) + + it('strips list-block annotation controls while preserving list text', async () => { + await mockPreviewOpenFile() + const root = document.createElement('div') + // Why: attr-only fixture (renamed class) proves the generic + // data-orca-export-hide rule scrubs even after a class rename. + root.innerHTML = ` +
+ +
` + const payload = await getActiveMarkdownExportPayload({ + fileId: '/repo/docs/readme.md', + root + }) + const exported = parseExportedHtml(payload?.html) + expect(exported.querySelector('li')?.textContent).toContain('List item') + expect(exported.querySelector('[data-orca-export-hide]')).toBeNull() + expect(exported.querySelector('.markdown-annotation-add')).toBeNull() + expect(root.querySelector('[data-orca-export-hide]')).not.toBeNull() + }) }) + +async function mockPreviewOpenFile(): Promise { + const { useAppStore } = await import('@/store') + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test mock provides only openFiles, the sole store slice getActiveMarkdownExportPayload reads. + vi.mocked(useAppStore.getState).mockReturnValue({ + openFiles: [ + { + id: '/repo/docs/readme.md', + filePath: '/repo/docs/readme.md', + relativePath: 'docs/readme.md', + mode: 'markdown-preview' + } + ] + } as never) +} + +function parseExportedHtml(html: string | undefined): HTMLElement { + const container = document.createElement('div') + container.innerHTML = html ?? '' + return container +} diff --git a/src/renderer/src/components/editor/markdown-export-extract.ts b/src/renderer/src/components/editor/markdown-export-extract.ts index a0ec5ebb2fe..acf53973921 100644 --- a/src/renderer/src/components/editor/markdown-export-extract.ts +++ b/src/renderer/src/components/editor/markdown-export-extract.ts @@ -17,12 +17,18 @@ const DOCUMENT_SUBTREE_SELECTOR = '.ProseMirror, .markdown-body' // Why: even after picking the smallest subtree, a few in-document UI leaks // can remain. The design doc lists these by name and treats the cloned-scrub // pass as a belt-and-suspenders defense so PDF output never shows copy -// buttons, per-block search highlights, or other transient affordances. +// buttons, per-block search highlights, annotation controls, or other +// transient affordances. const UI_ONLY_SELECTORS = [ '.code-block-copy-btn', '.markdown-preview-search', '[class*="rich-markdown-search"]', - '[data-orca-export-hide="true"]' + // Why: preview annotation controls (add-note button, composer, note stack) + // render inside `.markdown-body`. The source also carries + // `data-orca-export-hide`, so the generic rule below covers renames; this + // explicit entry covers an attr-strip regression. + '.markdown-annotation-controls', + '[data-orca-export-hide]' ] function basenameWithoutExt(filePath: string): string { diff --git a/src/renderer/src/components/editor/markdown-export-html.test.ts b/src/renderer/src/components/editor/markdown-export-html.test.ts index 66bce46fba6..9d98aa61502 100644 --- a/src/renderer/src/components/editor/markdown-export-html.test.ts +++ b/src/renderer/src/components/editor/markdown-export-html.test.ts @@ -28,4 +28,11 @@ describe('buildMarkdownExportHtml', () => { const html = buildMarkdownExportHtml({ title: '', renderedHtml: '

x

' }) expect(html).toContain('Untitled') }) + + it('hides preview annotation controls even if DOM scrubbing misses them', () => { + const html = buildMarkdownExportHtml({ title: 'Notes', renderedHtml: '

x

' }) + expect(html).toContain('.markdown-annotation-controls') + expect(html).toContain('[data-orca-export-hide') + expect(html).toContain('display: none') + }) }) diff --git a/src/renderer/src/components/editor/use-markdown-preview-annotation-renderers.tsx b/src/renderer/src/components/editor/use-markdown-preview-annotation-renderers.tsx index 4e45e337a10..ab0ffeb1146 100644 --- a/src/renderer/src/components/editor/use-markdown-preview-annotation-renderers.tsx +++ b/src/renderer/src/components/editor/use-markdown-preview-annotation-renderers.tsx @@ -88,7 +88,11 @@ export function useMarkdownPreviewAnnotationRenderers({ } return ( -
+ // Why: annotation controls (add-note button, composer, saved note + // stack) are transient review state, not document content. They render + // inside `.markdown-body`, so mark the container for PDF export + // exclusion — the extract scrub and export CSS both honor this. +