Skip to content

feat: add export to PDF#6

Open
ranjan-codewalnut wants to merge 1 commit intomainfrom
feat/export-to-pdf
Open

feat: add export to PDF#6
ranjan-codewalnut wants to merge 1 commit intomainfrom
feat/export-to-pdf

Conversation

@ranjan-codewalnut
Copy link
Copy Markdown
Collaborator

Summary

  • Add client-side PDF export using html2pdf.js library
  • New exportToPdf utility extracts editor HTML, applies matching CSS styles (fonts, lists, links, images), and generates a US Letter PDF with proper margins (0.5" top/bottom, 1" left/right)
  • Add download icon button to the toolbar for triggering the export

Test plan

  • Open the editor and type content with various formatting (headings, bold, italic, lists, links, images)
  • Click the export (download) button in the toolbar
  • Verify a document.pdf file is downloaded
  • Open the PDF and confirm styling matches the editor content

🤖 Generated with Claude Code

Add a toolbar button to export the editor content as a PDF file with
matching styles, US Letter format, and proper margins.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mergemitra
Copy link
Copy Markdown

mergemitra Bot commented Apr 30, 2026

PR Review

Scores

PR Quality Correctness Code Quality Test Quality Enterprise Quality
PR Quality Notes

PR Description & Scope

  • ✅ Title follows conventional format and clearly states the PDF export feature.
  • ✅ Summary matches the diff: html2pdf.js, toolbar button, PDF styling, and the test plan are all covered.

Commit Messages & History

  • ✅ The single commit uses conventional format and describes the PDF export clearly.

PR Size & Focus

  • ✅ Small, focused PR at 81 added lines across 4 files, centered on PDF export.
  • ✅ The change stays tight around package.json, Toolbar.tsx, and exportToPdf.ts.

Self-Review & Polish

  • ✅ No debug statements, conflict markers, or accidental files appear in the diff.

TL;DR

This PR adds client-side PDF export by wiring a new toolbar button to a html2pdf.js-backed exportToPdf utility. The biggest risks are that the exported PDF is generated from a fresh DOM instead of the editor’s paginated view, can lose external images or fail on longer documents, and introduces startup/accessibility regressions through eager loading and rasterized output. The repo also has no automated test harness yet, so the new export path ships without regression coverage.

Merge Recommendation: Approve with suggestions — the feature is directionally useful, but the current implementation has material fidelity, accessibility, and reliability gaps.

Focus Areas for Architect Review
  • The chosen html2pdf.js pipeline is inherently raster-based. Even when it succeeds, it produces image-backed PDFs, so text is not searchable/selectable and fidelity is bounded by html2canvas’s CSS/rendering limits.
  • Reference docs used: html2pdf.js, html2canvas overview, html2canvas FAQ, html2canvas configuration
  • Toolbar.tsx is continuing to grow by adding one-off button blocks inline. If more commands are coming, this component is close to the point where section extraction or a declarative action config would improve scanability and reduce repetition.
  • PDF fidelity is currently maintained by hand in a TS string. If export formats are going to expand, the editor likely needs a shared styling contract for rendered content rather than per-output restyling.
  • This dependency adds a sizeable browser-side export stack. If client-side PDF export stays, add bundle-budget checks and dependency/vulnerability monitoring for this path.
  • The app has no shared notification/progress pattern today. A small toast or status mechanism would make future long-running client actions like export/import/upload observable instead of failing silently.
  • If accessible or compliance-grade PDFs are a product requirement, html2canvas is likely the wrong foundation; plan for a renderer that preserves a text layer and document semantics.
  • package.json has no test script or test dependencies, and the repo tree contains no *.test.* / *.spec.* files, so there is no automated test harness available for this PR yet.
  • The new user-facing export path in src/utils/exportToPdf.ts and the toolbar wiring in src/components/Toolbar.tsx would benefit from coverage once a harness exists, especially around the export trigger and the html2pdf.js options passed through.

🟠 9 major (See below inline comments)

💬 Minor / Nitpicks (3)
  • [Code Quality] 💬 [nitpick] — src/utils/exportToPdf.ts:45-67 is a reusable export helper with several non-obvious assumptions, but there’s no JSDoc or inline note explaining them. A short docblock covering the letter-size output, margin order, rasterization choice, and useCORS dependency would make later adjustments much easier to reason about. (src/utils/exportToPdf.ts:45-67)
  • [Code Quality] 💬 [nitpick] — src/utils/exportToPdf.ts:59-64 packs a lot of format decisions into anonymous literals and an undocumented tuple assertion. Named constants for the margins, filename, scale, and paper format would make the configuration self-describing and avoid having to mentally decode [0.5, 1, 0.5, 1] every time someone changes the export settings. (src/utils/exportToPdf.ts:59-64)
  • [Code Quality] 💬 [nitpick] — src/utils/exportToPdf.ts:4 names the CSS blob EDITOR_STYLES, but the selectors are specific to the PDF export container rather than the editor generally. That name is broad enough to suggest it is a shared editor style contract, which makes the utility easier to misread and accidentally reuse incorrectly. (src/utils/exportToPdf.ts:4)

Based on 226390e...ea0d3a1

Comment thread src/utils/exportToPdf.ts
Comment on lines +1 to +2
import type { Editor } from "@tiptap/react";
import html2pdf from "html2pdf.js";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Enterprise Quality] 🟠 html2pdf.js is imported at module scope and Toolbar pulls that module into the initial editor render, so every editor session pays the download/parse cost of the PDF stack even if export is never used. For a client-only editor this is avoidable startup regression on all users; move the export path behind a dynamic import() so the chunk is fetched only when the button is pressed.

Related: package.json:26, src/components/Toolbar.tsx:30

Comment thread src/utils/exportToPdf.ts
Comment on lines +4 to +41
const EDITOR_STYLES = `
.pdf-export-container {
font-family: Arial, sans-serif;
font-size: 11pt;
line-height: 1.5;
color: #000;
}
.pdf-export-container ul {
list-style-type: disc;
padding-left: 1.5em;
margin: 0.5em 0;
}
.pdf-export-container ol {
list-style-type: decimal;
padding-left: 1.5em;
margin: 0.5em 0;
}
.pdf-export-container ul ul {
list-style-type: circle;
}
.pdf-export-container ul ul ul {
list-style-type: square;
}
.pdf-export-container li {
margin: 0.2em 0;
}
.pdf-export-container a {
color: #1a73e8;
text-decoration: underline;
}
.pdf-export-container img {
max-width: 100%;
height: auto;
}
.pdf-export-container hr {
border: none;
border-top: 1px solid #dadce0;
margin: 1em 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Code Quality] 🟠 src/utils/exportToPdf.ts:4-41 adds a second hardcoded source of truth for editor presentation (Arial, 11pt, link color, rule color, list spacing), while the toolbar already carries some of those defaults in src/components/Toolbar.tsx:112-114. That duplication will drift the first time editor styling changes, and PDF parity will become a manual sync problem instead of a shared configuration.

Related: src/components/Toolbar.tsx:112-114

Comment thread src/utils/exportToPdf.ts
Comment on lines +45 to +67
export function exportToPdf(editor: Editor): void {
const editorHtml = editor.getHTML();

const wrapper = document.createElement("div");

const styleElement = document.createElement("style");
styleElement.textContent = EDITOR_STYLES;
wrapper.appendChild(styleElement);

const content = document.createElement("div");
content.className = "pdf-export-container";
content.innerHTML = editorHtml;
wrapper.appendChild(content);

const options = {
margin: [0.5, 1, 0.5, 1] as [number, number, number, number],
filename: "document.pdf",
image: { type: "jpeg" as const, quality: 0.98 },
html2canvas: { scale: 2, useCORS: true },
jsPDF: { unit: "in", format: "letter", orientation: "portrait" as const },
};

html2pdf().set(options).from(wrapper).save();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Correctness] 🟠 src/utils/exportToPdf.ts:45-67 rebuilds the PDF from editor.getHTML() and lets html2pdf paginate that fresh DOM, instead of exporting the already-paginated page view the user is editing. In this editor, page-boundary avoidance is applied at render time, so any heading/paragraph that was visually pushed to the next page can still be split or land on a different page in the PDF. This shows up as soon as a document spans multiple pages or has a block near the bottom of a page.


[Code Quality] 🟠 src/utils/exportToPdf.ts:45-67 couples two responsibilities in one utility: it reaches into Tiptap to extract HTML and also owns the PDF rendering/saving flow. That makes the module harder to reuse outside this toolbar path and raises the cost of testing or adding other export entry points, because callers have to supply a full Editor instead of plain exportable content.

Related: src/components/Toolbar.tsx:530


[Enterprise Quality] 🟠 exportToPdf is fire-and-forget: it kicks off html2pdf().save() and the toolbar discards the returned async chain. When rasterization fails because of large documents, memory pressure, or cross-origin images without permissive CORS, the user gets no feedback and the app cannot disable/retry the button to prevent overlapping exports; return a promise, await it in the toolbar, and surface a visible failure state.

Related: src/components/Toolbar.tsx:528-530


[Correctness] 🟠 src/utils/exportToPdf.ts:62-67 assumes useCORS: true is enough for images, but there is no proxy/fallback path. The existing image flow accepts arbitrary third-party URLs, and html2canvas skips cross-origin images that are not CORS-enabled, so documents that preview correctly in the editor can export with missing images whenever the user inserted a normal external image URL.


[Correctness] 🟠 src/utils/exportToPdf.ts:62-67 renders the entire document into a single canvas at scale: 2. With this editor’s ~960px content height per page, Chrome’s 32,767px canvas-height limit is hit at roughly 17 pages, after which html2canvas/html2pdf can return blank or truncated output. Long documents are therefore exportable in the editor UI but not reliably exportable as PDF.


[Enterprise Quality] 🟠 The PDF output is rasterized through html2canvas into JPEG-backed pages, so exported documents will not contain selectable/searchable text, semantic headings, or accessible links. That is a material accessibility gap for anyone consuming the PDF with screen readers or needing copy/search, and it usually requires a print/DOM-based or server-side renderer rather than a canvas snapshot to fix.

Comment on lines +528 to +530
<ToolbarButton
title="Export to PDF"
onClick={() => exportToPdf(editor)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Enterprise Quality] 🟠 The new export control inherits mouse-only activation from ToolbarButton: the action is bound to onMouseDown, but keyboard activation of a native button dispatches click, not mousedown. That means Enter/Space cannot trigger export, and the icon-only button still relies on title instead of a stable programmatic label; wire activation through onClick and add aria-label={title} while keeping onMouseDown only for focus preservation.

ℹ️ Original location: src/components/Toolbar.tsx:64-85 — moved to nearest diff line for GitHub compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant