mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 00:02:29 +00:00
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)
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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 = `
|
||||
<div class="markdown-body">
|
||||
<div class="markdown-annotation-block" data-source-line="1" data-source-end-line="1">
|
||||
<h1>Title</h1>
|
||||
<div class="markdown-annotation-controls">
|
||||
<button type="button" class="markdown-annotation-add" aria-label="Add note"><svg></svg></button>
|
||||
<div class="markdown-annotation-composer"><textarea>draft note</textarea></div>
|
||||
<div class="markdown-annotation-note-stack"><div class="markdown-annotation-card">saved note body</div></div>
|
||||
</div>
|
||||
</div>
|
||||
<div class="markdown-annotation-block" data-source-line="2" data-source-end-line="2">
|
||||
<p>Body text</p>
|
||||
<div class="markdown-annotation-controls">
|
||||
<button type="button" class="markdown-annotation-add" aria-label="Add note"><svg></svg></button>
|
||||
</div>
|
||||
</div>
|
||||
<pre><code class="language-mermaid">graph TD;</code></pre>
|
||||
</div>`
|
||||
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 = `
|
||||
<div class="markdown-body">
|
||||
<ul>
|
||||
<li>
|
||||
<div class="markdown-annotation-list-block" data-source-line="2" data-source-end-line="2">
|
||||
<span class="markdown-annotation-list-content">List item</span>
|
||||
<div class="markdown-annotation-controls-renamed" data-orca-export-hide="true">
|
||||
<button type="button" class="markdown-annotation-add" aria-label="Add note"><svg></svg></button>
|
||||
</div>
|
||||
</div>
|
||||
</li>
|
||||
</ul>
|
||||
</div>`
|
||||
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<void> {
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -28,4 +28,11 @@ describe('buildMarkdownExportHtml', () => {
|
||||
const html = buildMarkdownExportHtml({ title: '', renderedHtml: '<p>x</p>' })
|
||||
expect(html).toContain('<title>Untitled</title>')
|
||||
})
|
||||
|
||||
it('hides preview annotation controls even if DOM scrubbing misses them', () => {
|
||||
const html = buildMarkdownExportHtml({ title: 'Notes', renderedHtml: '<p>x</p>' })
|
||||
expect(html).toContain('.markdown-annotation-controls')
|
||||
expect(html).toContain('[data-orca-export-hide')
|
||||
expect(html).toContain('display: none')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -88,7 +88,11 @@ export function useMarkdownPreviewAnnotationRenderers({
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="markdown-annotation-controls">
|
||||
// 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.
|
||||
<div className="markdown-annotation-controls" data-orca-export-hide="true">
|
||||
<button
|
||||
type="button"
|
||||
className="markdown-annotation-add"
|
||||
|
||||
Reference in New Issue
Block a user