Skip to content

Conversation

@duckduckhero
Copy link
Contributor

@duckduckhero duckduckhero commented Aug 20, 2025

Summary by cubic

Upgrade PDF export with selectable themes, improved list rendering, and a cleaner layout. Files now save directly to the system Downloads folder with a confirmation message.

  • New Features
    • Theme picker in Share > PDF with 12 built-in themes (colors + fonts).
    • Better content formatting: H1–H3 headers, styled metadata, clickable Hyprnote link.
    • Accurate lists: nested ordered/unordered, alphabetic/roman markers, vector bullets.
    • Themed page background applied on every page.
    • Exports now saved to the OS Downloads folder; toast shows the saved path.
    • Updated Tauri permissions to allow read/write in $DOWNLOAD.

@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

📝 Walkthrough

Walkthrough

Adds PDF theming and Downloads directory support to the desktop app’s export flow. Introduces a PDF themes module, updates the PDF exporter to accept a theme and save to the Downloads folder, wires a theme selector into the Share button, and expands Tauri capabilities to allow open/write in Downloads.

Changes

Cohort / File(s) Summary
Tauri capabilities (Downloads access)
apps/desktop/src-tauri/capabilities/default.json
Extend allow lists for opener:allow-open-path and fs:allow-write-file to include $DOWNLOAD/* and $DOWNLOAD/** alongside existing $APPDATA/* and $APPDATA/**.
Share button UI and handlers (theme selection)
apps/desktop/src/components/toolbar/buttons/share-button.tsx
Add theme picker for PDF export; track selectedPdfTheme; update exportHandlers.pdf to accept ThemeName and pass to exportToPDF; show success message with saved path; import ThemeName and getAvailableThemes.
PDF export pipeline (theming, lists, Downloads path)
apps/desktop/src/components/toolbar/utils/pdf-export.ts
exportToPDF now accepts ThemeName (default "default"); re-export theme APIs; switch save location to downloadDir; add theming (colors/background), page utilities, improved metadata layout; refactor HTML processing to support nested lists, numbering, bullets, and vector bullets.
PDF themes module (new)
apps/desktop/src/components/toolbar/utils/pdf-themes.ts
New module exporting ThemeName union, PDFTheme interface, getPDFTheme, getAvailableThemes, and getThemePreview; defines 12 themes with fonts and color palettes.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant UI as ShareButton UI
  participant EH as exportHandlers
  participant PDF as pdf-export (exportToPDF)
  participant FS as FileSystem (Downloads)
  participant Msg as Message/Toast

  User->>UI: Open Share > PDF
  UI->>UI: Select theme (default if none)
  UI->>EH: pdf(session, theme)
  EH->>PDF: exportToPDF(session, theme)
  PDF->>FS: Save PDF to downloadDir
  FS-->>PDF: Path to saved file
  PDF-->>EH: Return path
  EH-->>UI: ExportResult { type: pdf, path }
  UI->>Msg: "Saved to Downloads" with path
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pdf-upgrade

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@duckduckhero duckduckhero deleted the pdf-upgrade branch August 20, 2025 18:11
@duckduckhero duckduckhero restored the pdf-upgrade branch August 20, 2025 18:12
@duckduckhero duckduckhero deleted the pdf-upgrade branch August 20, 2025 18:13
@duckduckhero duckduckhero restored the pdf-upgrade branch August 20, 2025 18:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)

506-510: Cross-platform path join — avoid mixed separators on Windows

Manual "/" concatenation can produce mixed separators on Windows (e.g., C:\...\Downloads/filename). Use Tauri’s join to build the path.

-import { downloadDir } from "@tauri-apps/api/path";
+import { downloadDir, join } from "@tauri-apps/api/path";
...
-  const downloadsPath = await downloadDir();
-  const filePath = downloadsPath.endsWith("/")
-    ? `${downloadsPath}${filename}`
-    : `${downloadsPath}/${filename}`;
+  const downloadsPath = await downloadDir();
+  const filePath = await join(downloadsPath, filename);
🧹 Nitpick comments (8)
apps/desktop/src-tauri/capabilities/default.json (1)

62-66: Minor: consider removing redundant APPDATA single-level patterns

Having both $APPDATA/* and $APPDATA/** is redundant; ** already includes single-level paths. Dropping the * entries tightens the config slightly without reducing access.

-        { "path": "$APPDATA/*" },
         { "path": "$APPDATA/**" },
         { "path": "$DOWNLOAD/*" },
         { "path": "$DOWNLOAD/**" }

Also applies to: 82-86

apps/desktop/src/components/toolbar/utils/pdf-themes.ts (4)

218-235: Descriptions don’t match defined fonts for a few themes

The descriptions for these themes reference different fonts than the actual theme config:

  • forest: description says “with Courier” but font is "helvetica"
  • cyberpunk: description says “with Courier” but font is "helvetica"
  • summer: description says “with Courier” but font is "helvetica"

Update the descriptions (or the fonts) to keep them consistent.

   forest: "Forest greens on mint background with Courier",
-  cyberpunk: "Matrix green on space black with Courier",
+  cyberpunk: "Matrix green on space black with Helvetica",
   retro: "Gold text on dark brown with Courier",
   spring: "Fresh greens on yellow-green with Courier",
-  summer: "Bright reds on yellow with Courier",
+  summer: "Bright reds on yellow with Helvetica",
-  winter: "Deep blues on icy background with Times",
+  winter: "Deep blues on icy background with Times",

If “forest” truly uses Helvetica as defined above, also change:

-  forest: "Forest greens on mint background with Courier",
+  forest: "Forest greens on mint background with Helvetica",

28-186: DRY opportunity: centralize theme definitions and derive the list from keys

getAvailableThemes duplicates the union order. Consider hoisting the themes map to a top-level const THEMES and deriving getAvailableThemes() from its keys, so adding/removing a theme stays single-sourced.

Example:

// top-level
export const THEMES = { /* current themes map */ } as const satisfies Record<ThemeName, PDFTheme>;

export const getPDFTheme = (name: ThemeName) => THEMES[name];

export const getAvailableThemes = (): ThemeName[] => Object.keys(THEMES) as ThemeName[];

Also applies to: 190-205


187-188: Unnecessary fallback for typed key

themes[themeName] || themes.default is unreachable given themeName: ThemeName. Using themes[themeName] keeps types tight and avoids masking missing keys during refactors.

-  return themes[themeName] || themes.default;
+  return themes[themeName];

31-53: Comments are “what”, not “why”

Inline color comments are largely “what” and repeat the literal. If you keep comments, prefer brief “why” context (e.g., “contrast target” or “brand-aligned”). Otherwise, consider trimming to reduce noise.

Also applies to: 69-93, 108-132, 134-158, 160-184

apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)

335-356: Theme selector UX is straightforward; tighten typing to avoid casts

The dropdown renders all available themes and capitalizes labels. To avoid the (value as ThemeName) cast, you can type the onValueChange callback if your Select component supports generics, or wrap it:

- onValueChange={(value) => setSelectedPdfTheme(value as ThemeName)}
+ onValueChange={(value) => setSelectedPdfTheme(value as ThemeName)} // If Select can't be generic
+ // Alternative if Select supports generics:
+ // <Select<ThemeName> value={selectedPdfTheme} onValueChange={setSelectedPdfTheme}>
apps/desktop/src/components/toolbar/utils/pdf-export.ts (2)

459-469: Ordered list continuation lines should align under the marker

Currently ordered items set bulletSpace = 0, so wrapped lines start under the marker instead of aligning to the text. Compute bulletSpace from the marker width.

-      bulletSpace = segment.listType === "ordered" ? 0 : 6;
+      if (segment.listType === "ordered" && typeof segment.listItemNumber === "number") {
+        const marker = `${getOrderedListMarker(segment.listItemNumber, segment.listLevel ?? 0)} `;
+        bulletSpace = pdf.getTextWidth(marker);
+      } else {
+        bulletSpace = 6;
+      }

This keeps wrapped lines aligned with the first line’s text for ordered lists.

Also applies to: 492-494


31-41: Alphabetic ordered markers overflow after ‘z’

String.fromCharCode(96 + counter) will go past ‘z’ for counter > 26. If deep lists or long lists are possible, mod the alphabet index to avoid odd characters.

-    case 1:
-      return `${String.fromCharCode(96 + counter)}.`;
+    case 1: {
+      const idx = ((counter - 1) % 26) + 1;
+      return `${String.fromCharCode(96 + idx)}.`;
+    }
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 22ab193 and d3bf8f2.

📒 Files selected for processing (4)
  • apps/desktop/src-tauri/capabilities/default.json (2 hunks)
  • apps/desktop/src/components/toolbar/buttons/share-button.tsx (5 hunks)
  • apps/desktop/src/components/toolbar/utils/pdf-export.ts (8 hunks)
  • apps/desktop/src/components/toolbar/utils/pdf-themes.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}

⚙️ CodeRabbit Configuration File

**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".

Files:

  • apps/desktop/src/components/toolbar/utils/pdf-themes.ts
  • apps/desktop/src/components/toolbar/utils/pdf-export.ts
  • apps/desktop/src/components/toolbar/buttons/share-button.tsx
🧬 Code Graph Analysis (2)
apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)
apps/desktop/src/components/toolbar/utils/pdf-themes.ts (2)
  • ThemeName (1-13)
  • getPDFTheme (28-188)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (2)
apps/desktop/src/components/toolbar/utils/pdf-themes.ts (2)
  • ThemeName (1-13)
  • getAvailableThemes (190-205)
apps/desktop/src/components/toolbar/utils/pdf-export.ts (3)
  • ThemeName (8-8)
  • getAvailableThemes (8-8)
  • exportToPDF (304-513)
🪛 ast-grep (0.38.6)
apps/desktop/src/components/toolbar/utils/pdf-export.ts

[warning] 67-67: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = cleanedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 67-67: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = cleanedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)

🪛 Biome (2.1.2)
apps/desktop/src/components/toolbar/utils/pdf-export.ts

[error] 276-276: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 287-287: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: ci (windows, windows-latest)
  • GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (8)
apps/desktop/src-tauri/capabilities/default.json (3)

63-66: DOWNLOAD path allowlist added — aligns with new export target

Adding $DOWNLOAD/* and $DOWNLOAD/** to opener:allow-open-path is correct for opening exported PDFs from the Downloads directory.


83-86: WRITE permission extended to Downloads — needed for saving PDFs

Allowing fs:allow-write-file for $DOWNLOAD/* and $DOWNLOAD/** is necessary for writeFile to succeed when saving the PDF.


63-66: Confirmed correct Tauri download placeholder

The $DOWNLOAD variable is the officially supported path placeholder for the Downloads directory in Tauri v2 (see beta.tauri.app). No changes are required.

apps/desktop/src/components/toolbar/buttons/share-button.tsx (3)

26-26: Importing theming API from pdf-export keeps the public surface tight

Importing exportToPDF, getAvailableThemes, and ThemeName from the pdf utilities is consistent with the new API design. Good.


41-41: Theme selection state wired with a safe default

selectedPdfTheme with default "default" is sensible and typed as ThemeName.


165-166: PDF export now passes the selected theme

Handing selectedPdfTheme into exportHandlers.pdf(session, theme) correctly propagates theming to the exporter.

apps/desktop/src/components/toolbar/utils/pdf-export.ts (2)

256-263: jsPDF color getters may not exist across versions

getFillColor()/getDrawColor() aren’t consistently available in all jsPDF builds. If you observe runtime undefined, drop the save/restore and just set the desired colors explicitly before drawing bullets (you already set text/draw colors elsewhere).

Would you like me to produce a fallback that avoids these getters while preserving behavior?

Also applies to: 300-302


324-339: Nice touch: background fill only when non-white + applied on new pages

This avoids unnecessary ops for white themes and keeps pages consistent.

Comment on lines +61 to 69
const cleanedHtml = html
.replace(/<\/?strong>/gi, "")
.replace(/<\/?b>/gi, "")
.replace(/<\/?em>/gi, "")
.replace(/<\/?i>/gi, "");

const tempDiv = document.createElement("div");
tempDiv.innerHTML = html;
tempDiv.innerHTML = cleanedHtml;

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid innerHTML assignment; parse HTML safely off-DOM

tempDiv.innerHTML = ... triggers XSS linters and is unnecessary here. Use DOMParser to parse the HTML and traverse the resulting document. This avoids assigning HTML to a live element and quiets security tooling.

-  const cleanedHtml = html
+  const cleanedHtml = html
     .replace(/<\/?strong>/gi, "")
     .replace(/<\/?b>/gi, "")
     .replace(/<\/?em>/gi, "")
     .replace(/<\/?i>/gi, "");
 
-  const tempDiv = document.createElement("div");
-  tempDiv.innerHTML = cleanedHtml;
+  const parser = new DOMParser();
+  const doc = parser.parseFromString(cleanedHtml, "text/html");
...
-  Array.from(tempDiv.childNodes).forEach(processNode);
+  Array.from(doc.body.childNodes).forEach(processNode);

Note: If you want stricter sanitation (e.g., drop script/style entirely), add a small pre-pass to remove them before parsing:

const cleanedHtml = html
  .replace(/<script[\s\S]*?>[\s\S]*?<\/script>/gi, "")
  .replace(/<style[\s\S]*?>[\s\S]*?<\/style>/gi, "")
  // existing replacements...

Also applies to: 207-207

🧰 Tools
🪛 ast-grep (0.38.6)

[warning] 67-67: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = cleanedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 67-67: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = cleanedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)

🤖 Prompt for AI Agents
In apps/desktop/src/components/toolbar/utils/pdf-export.ts around lines 61-69
(and similarly at 207), avoid assigning to element.innerHTML; instead parse the
cleanedHtml using DOMParser (or parseFromString) to obtain an off-DOM Document,
then traverse that document to extract and manipulate nodes; if stricter
sanitation is desired, pre-strip <script> and <style> blocks from the input
string before parsing to ensure those tags are removed prior to DOMParser
processing.

Comment on lines +266 to +299
case "filled-circle":
pdf.circle(x, bulletY, size * 0.85, "F");
break;

case "hollow-circle":
pdf.circle(x, bulletY, size * 0.85, "S");
break;

case "square":
const squareSize = size * 1.4;
pdf.rect(
x - squareSize / 2,
bulletY - squareSize / 2,
squareSize,
squareSize,
"F",
);
break;

case "triangle":
const triangleSize = size * 1.15;
pdf.triangle(
x,
bulletY - triangleSize / 2, // top point
x - triangleSize / 2,
bulletY + triangleSize / 2, // bottom left
x + triangleSize / 2,
bulletY + triangleSize / 2, // bottom right
"F",
);
break;
}

pdf.setFillColor(currentFillColor);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Biome error: declarations inside switch cases need blocks

const squareSize and const triangleSize are declared directly in case clauses. Wrap them in blocks to satisfy noSwitchDeclarations and avoid leakage across cases.

-    case "square":
-      const squareSize = size * 1.4;
+    case "square": {
+      const squareSize = size * 1.4;
       pdf.rect(
         x - squareSize / 2,
         bulletY - squareSize / 2,
         squareSize,
         squareSize,
         "F",
       );
-      break;
+      break;
+    }
...
-    case "triangle":
-      const triangleSize = size * 1.15;
+    case "triangle": {
+      const triangleSize = size * 1.15;
       pdf.triangle(
         x,
         bulletY - triangleSize / 2, // top point
         x - triangleSize / 2,
         bulletY + triangleSize / 2, // bottom left
         x + triangleSize / 2,
         bulletY + triangleSize / 2, // bottom right
         "F",
       );
-      break;
+      break;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
switch (bulletType) {
case "filled-circle":
pdf.circle(x, bulletY, size * 0.85, "F");
break;
case "hollow-circle":
pdf.circle(x, bulletY, size * 0.85, "S");
break;
case "square":
const squareSize = size * 1.4;
pdf.rect(
x - squareSize / 2,
bulletY - squareSize / 2,
squareSize,
squareSize,
"F",
);
break;
case "triangle":
const triangleSize = size * 1.15;
pdf.triangle(
x,
bulletY - triangleSize / 2, // top point
x - triangleSize / 2,
bulletY + triangleSize / 2, // bottom left
x + triangleSize / 2,
bulletY + triangleSize / 2, // bottom right
"F",
);
break;
}
switch (bulletType) {
case "filled-circle":
pdf.circle(x, bulletY, size * 0.85, "F");
break;
case "hollow-circle":
pdf.circle(x, bulletY, size * 0.85, "S");
break;
case "square": {
const squareSize = size * 1.4;
pdf.rect(
x - squareSize / 2,
bulletY - squareSize / 2,
squareSize,
squareSize,
"F",
);
break;
}
case "triangle": {
const triangleSize = size * 1.15;
pdf.triangle(
x,
bulletY - triangleSize / 2, // top point
x - triangleSize / 2,
bulletY + triangleSize / 2, // bottom left
x + triangleSize / 2,
bulletY + triangleSize / 2, // bottom right
"F",
);
break;
}
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 276-276: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 287-287: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🤖 Prompt for AI Agents
In apps/desktop/src/components/toolbar/utils/pdf-export.ts around lines 266 to
299, the switch cases declare const variables directly which violates
noSwitchDeclarations; wrap the 'square' and 'triangle' case bodies in their own
block braces ({ ... }) so the const squareSize and const triangleSize are scoped
to their cases and cannot leak into other cases, leaving the existing
pdf.rect/pdf.triangle calls inside those newly added blocks.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

7 issues found across 4 files

React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.

cyberpunk: "Matrix green on space black with Courier",
retro: "Gold text on dark brown with Courier",
spring: "Fresh greens on yellow-green with Courier",
summer: "Bright reds on yellow with Courier",
Copy link

Choose a reason for hiding this comment

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

Align the summer theme description with the Helvetica font used in the definition

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-themes.ts at line 230:

<comment>Align the summer theme description with the Helvetica font used in the definition</comment>

<file context>
@@ -0,0 +1,235 @@
+export type ThemeName =
+  | &quot;default&quot;
+  | &quot;light&quot;
+  | &quot;dark&quot;
+  | &quot;corporate&quot;
+  | &quot;ocean&quot;
+  | &quot;sunset&quot;
+  | &quot;forest&quot;
+  | &quot;cyberpunk&quot;
</file context>

ocean: "Ocean blues on light cyan with Helvetica",
sunset: "Warm browns and oranges on cream with Times",
forest: "Forest greens on mint background with Courier",
cyberpunk: "Matrix green on space black with Courier",
Copy link

Choose a reason for hiding this comment

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

Synchronize the cyberpunk theme description with the actual font

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-themes.ts at line 227:

<comment>Synchronize the cyberpunk theme description with the actual font</comment>

<file context>
@@ -0,0 +1,235 @@
+export type ThemeName =
+  | &quot;default&quot;
+  | &quot;light&quot;
+  | &quot;dark&quot;
+  | &quot;corporate&quot;
+  | &quot;ocean&quot;
+  | &quot;sunset&quot;
+  | &quot;forest&quot;
+  | &quot;cyberpunk&quot;
</file context>

corporate: "Professional navy on white with Times",
ocean: "Ocean blues on light cyan with Helvetica",
sunset: "Warm browns and oranges on cream with Times",
forest: "Forest greens on mint background with Courier",
Copy link

Choose a reason for hiding this comment

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

Update the forest theme description to match the actual font setting

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-themes.ts at line 226:

<comment>Update the forest theme description to match the actual font setting</comment>

<file context>
@@ -0,0 +1,235 @@
+export type ThemeName =
+  | &quot;default&quot;
+  | &quot;light&quot;
+  | &quot;dark&quot;
+  | &quot;corporate&quot;
+  | &quot;ocean&quot;
+  | &quot;sunset&quot;
+  | &quot;forest&quot;
+  | &quot;cyberpunk&quot;
</file context>

};

export const getAvailableThemes = (): ThemeName[] => {
return [
Copy link

Choose a reason for hiding this comment

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

Avoid duplicating the theme name list; derive it from a single authoritative source to prevent drift

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-themes.ts at line 191:

<comment>Avoid duplicating the theme name list; derive it from a single authoritative source to prevent drift</comment>

<file context>
@@ -0,0 +1,235 @@
+export type ThemeName =
+  | &quot;default&quot;
+  | &quot;light&quot;
+  | &quot;dark&quot;
+  | &quot;corporate&quot;
+  | &quot;ocean&quot;
+  | &quot;sunset&quot;
+  | &quot;forest&quot;
+  | &quot;cyberpunk&quot;
</file context>

}

export const getPDFTheme = (themeName: ThemeName): PDFTheme => {
const themes: Record<ThemeName, PDFTheme> = {
Copy link

Choose a reason for hiding this comment

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

Recreate the static themes map only once instead of on every getPDFTheme call

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-themes.ts at line 29:

<comment>Recreate the static themes map only once instead of on every getPDFTheme call</comment>

<file context>
@@ -0,0 +1,235 @@
+export type ThemeName =
+  | &quot;default&quot;
+  | &quot;light&quot;
+  | &quot;dark&quot;
+  | &quot;corporate&quot;
+  | &quot;ocean&quot;
+  | &quot;sunset&quot;
+  | &quot;forest&quot;
+  | &quot;cyberpunk&quot;
</file context>

}

const cleanedHtml = html
.replace(/<\/?strong>/gi, "")
Copy link

Choose a reason for hiding this comment

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

Removing formatting tags without inserting whitespace can merge adjacent words and corrupt output

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-export.ts at line 62:

<comment>Removing formatting tags without inserting whitespace can merge adjacent words and corrupt output</comment>

<file context>
@@ -11,26 +14,61 @@ export type SessionData = Session &amp; {
 
 interface TextSegment {
   text: string;
-  bold?: boolean;
-  italic?: boolean;
-  isHeader?: number; // 1, 2, 3 for h1, h2, h3
+  isHeader?: number;
   isListItem?: boolean;
+  listType?: &quot;ordered&quot; | &quot;unordered&quot;;
</file context>

return `${counter}.`;
case 1:
return `${String.fromCharCode(96 + counter)}.`;
default:
Copy link

Choose a reason for hiding this comment

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

getOrderedListMarker fails for counters above 26, resulting in invalid characters for large ordered lists

Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-export.ts at line 36:

<comment>getOrderedListMarker fails for counters above 26, resulting in invalid characters for large ordered lists</comment>

<file context>
@@ -11,26 +14,61 @@ export type SessionData = Session &amp; {
 
 interface TextSegment {
   text: string;
-  bold?: boolean;
-  italic?: boolean;
-  isHeader?: number; // 1, 2, 3 for h1, h2, h3
+  isHeader?: number;
   isListItem?: boolean;
+  listType?: &quot;ordered&quot; | &quot;unordered&quot;;
</file context>

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.

2 participants