feat(preview-server): dark mode switcher #3
Conversation
…1749) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: gabriel miranda <gabrielmfern@outlook.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Bu Kinoshita <6929565+bukinoshita@users.noreply.github.com>
Co-authored-by: gabriel miranda <gabrielmfern@outlook.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Bu Kinoshita <6929565+bukinoshita@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This reverts commit f98a84a.
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
1 similar comment
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
![cubic-dev-local[bot]](https://avatars.githubusercontent.com/u/189440881?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 9 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:30">
invertColor emits LCH color strings that Firefox treats as invalid, so dark-mode styles break
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The violation claims that Firefox treats LCH color strings as invalid, breaking dark-mode styles. However, documentation confirms that Firefox has supported CSS lch() and oklch() color functions since version 113 (released in 2023). Given the current date (2025), all modern Firefox versions support these functions. The Color.js library's toString() method outputs valid CSS lch() strings when in the 'lch' space, which are parsable by supporting browsers. The repro step assumes a current Firefox, where the styles should apply correctly. This makes the violation factually incorrect for current browser versions. Although email clients may have limited support, the preview server renders in a browser iframe, so browser support is relevant. Filter out due to technical inaccuracy.
• **Exa queries**: "colorjs.io Color.toString() output when color space is lch latest version", "CSS lch() and oklch() color functions browser support Firefox latest version", "caniuse CSS lch() oklch() color functions Firefox support", "caniuse CSS lch() color function browser support Firefox"</violation>
<violation number="2" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:36">
The color inversion logic for dark mode emulation only targets inline styles, completely ignoring CSS rules defined in `<style>` blocks. Since other packages in the project, like the Tailwind integration, generate non-inline styles, this will lead to an inconsistent and partially applied dark mode.
• **Exa queries**: "React key prop behavior when changed dynamic value unmount remount"</violation>
<violation number="3" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:134">
Using a dynamic `key` on the `<iframe>` to toggle dark mode forces a full remount of the component on every theme change. This is an inefficient architectural choice that leads to a poor user experience, including content flicker and loss of scroll position.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Although changing the key causes the iframe to remount, this is an intentional design choice to easily reset the color inversions when switching themes, as noted in the code comment. While it may cause minor UX issues like brief flicker or loss of scroll position, the impact is low for short email previews, and the code is functional. React documentation confirms key changes trigger remount, but optimization is not critical here.
• **Exa queries**: "React key prop behavior when changed dynamic value unmount remount"</violation>
</file>
<file name="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx">
<violation number="1" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:18">
`Toggle.Root` never receives the controlled `pressed` state, so its internal state drifts out of sync with the `enabled` prop—screen readers will hear the opposite of the selected theme. Pass `pressed={enabled}` (and optionally switch to `onPressedChange`) so the toggle mirrors the actual dark-mode state.
• **Exa queries**: "Radix UI React Toggle component API documentation pressed prop onPressedChange accessibility aria-pressed"</violation>
</file>
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
`@types/color` provides declarations for the `color` package, but this project imports `colorjs.io`, which already includes its own types. Keeping this mismatch adds an unused dependency and still leaves `colorjs.io` untyped. Please drop this line (or supply the correct module declarations).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| {...rest} | ||
| // This key makes sure that the iframe itself remounts to the DOM when theme changes, so | ||
| // that the color changes in dark mode can be easily undone when switching to light mode. | ||
| key={darkMode ? 'iframe-inverted-colors' : 'iframe-normal-colors'} |
There was a problem hiding this comment.
Using a dynamic key on the <iframe> to toggle dark mode forces a full remount of the component on every theme change. This is an inefficient architectural choice that leads to a poor user experience, including content flicker and loss of scroll position.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Although changing the key causes the iframe to remount, this is an intentional design choice to easily reset the color inversions when switching themes, as noted in the code comment. While it may cause minor UX issues like brief flicker or loss of scroll position, the impact is low for short email previews, and the code is functional. React documentation confirms key changes trigger remount, but optimization is not critical here.
• Exa queries: "React key prop behavior when changed dynamic value unmount remount"
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 134:
<comment>Using a dynamic `key` on the `<iframe>` to toggle dark mode forces a full remount of the component on every theme change. This is an inefficient architectural choice that leads to a poor user experience, including content flicker and loss of scroll position.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Although changing the key causes the iframe to remount, this is an intentional design choice to easily reset the color inversions when switching themes, as noted in the code comment. While it may cause minor UX issues like brief flicker or loss of scroll position, the impact is low for short email previews, and the code is functional. React documentation confirms key changes trigger remount, but optimization is not critical here.
• **Exa queries**: "React key prop behavior when changed dynamic value unmount remount"</comment>
<file context>
@@ -0,0 +1,138 @@
+ {...rest}
+ // This key makes sure that the iframe itself remounts to the DOM when theme changes, so
+ // that the color changes in dark mode can be easily undone when switching to light mode.
+ key={darkMode ? 'iframe-inverted-colors' : 'iframe-normal-colors'}
+ />
+ </Slot>
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| const colorRegex = | ||
| /#[0-9a-fA-F]{3,4}|#[0-9a-fA-F]{6,8}|rgba?\(.*?\)|hsl\(.*?\)|hsv\(.*?\)|oklab\(.*?\)|oklch\(.*?\)/g; | ||
|
|
||
| function applyColorInversion(iframe: HTMLIFrameElement) { |
There was a problem hiding this comment.
The color inversion logic for dark mode emulation only targets inline styles, completely ignoring CSS rules defined in <style> blocks. Since other packages in the project, like the Tailwind integration, generate non-inline styles, this will lead to an inconsistent and partially applied dark mode.
• Exa queries: "React key prop behavior when changed dynamic value unmount remount"
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 36:
<comment>The color inversion logic for dark mode emulation only targets inline styles, completely ignoring CSS rules defined in `<style>` blocks. Since other packages in the project, like the Tailwind integration, generate non-inline styles, this will lead to an inconsistent and partially applied dark mode.
• **Exa queries**: "React key prop behavior when changed dynamic value unmount remount"</comment>
<file context>
@@ -0,0 +1,138 @@
+const colorRegex =
+ /#[0-9a-fA-F]{3,4}|#[0-9a-fA-F]{6,8}|rgba?\(.*?\)|hsl\(.*?\)|hsv\(.*?\)|oklab\(.*?\)|oklch\(.*?\)/g;
+
+function applyColorInversion(iframe: HTMLIFrameElement) {
+ const { contentDocument, contentWindow } = iframe;
+ if (!contentDocument || !contentWindow) return;
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| return ( | ||
| <Tooltip> | ||
| <Tooltip.Trigger asChild> | ||
| <Toggle.Root |
There was a problem hiding this comment.
Toggle.Root never receives the controlled pressed state, so its internal state drifts out of sync with the enabled prop—screen readers will hear the opposite of the selected theme. Pass pressed={enabled} (and optionally switch to onPressedChange) so the toggle mirrors the actual dark-mode state.
• Exa queries: "Radix UI React Toggle component API documentation pressed prop onPressedChange accessibility aria-pressed"
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 18:
<comment>`Toggle.Root` never receives the controlled `pressed` state, so its internal state drifts out of sync with the `enabled` prop—screen readers will hear the opposite of the selected theme. Pass `pressed={enabled}` (and optionally switch to `onPressedChange`) so the toggle mirrors the actual dark-mode state.
• **Exa queries**: "Radix UI React Toggle component API documentation pressed prop onPressedChange accessibility aria-pressed"</comment>
<file context>
@@ -0,0 +1,38 @@
+ return (
+ <Tooltip>
+ <Tooltip.Trigger asChild>
+ <Toggle.Root
+ value="dark"
+ className={cn(
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
@types/color provides declarations for the color package, but this project imports colorjs.io, which already includes its own types. Keeping this mismatch adds an unused dependency and still leaves colorjs.io untyped. Please drop this line (or supply the correct module declarations).
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>`@types/color` provides declarations for the `color` package, but this project imports `colorjs.io`, which already includes its own types. Keeping this mismatch adds an unused dependency and still leaves `colorjs.io` untyped. Please drop this line (or supply the correct module declarations).</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
|
||
| color.lch.c! *= 0.8; | ||
|
|
||
| return color.toString(); |
There was a problem hiding this comment.
invertColor emits LCH color strings that Firefox treats as invalid, so dark-mode styles break
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The violation claims that Firefox treats LCH color strings as invalid, breaking dark-mode styles. However, documentation confirms that Firefox has supported CSS lch() and oklch() color functions since version 113 (released in 2023). Given the current date (2025), all modern Firefox versions support these functions. The Color.js library's toString() method outputs valid CSS lch() strings when in the 'lch' space, which are parsable by supporting browsers. The repro step assumes a current Firefox, where the styles should apply correctly. This makes the violation factually incorrect for current browser versions. Although email clients may have limited support, the preview server renders in a browser iframe, so browser support is relevant. Filter out due to technical inaccuracy.
• Exa queries: "colorjs.io Color.toString() output when color space is lch latest version", "CSS lch() and oklch() color functions browser support Firefox latest version", "caniuse CSS lch() oklch() color functions Firefox support", "caniuse CSS lch() color function browser support Firefox"
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 30:
<comment>invertColor emits LCH color strings that Firefox treats as invalid, so dark-mode styles break
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The violation claims that Firefox treats LCH color strings as invalid, breaking dark-mode styles. However, documentation confirms that Firefox has supported CSS lch() and oklch() color functions since version 113 (released in 2023). Given the current date (2025), all modern Firefox versions support these functions. The Color.js library's toString() method outputs valid CSS lch() strings when in the 'lch' space, which are parsable by supporting browsers. The repro step assumes a current Firefox, where the styles should apply correctly. This makes the violation factually incorrect for current browser versions. Although email clients may have limited support, the preview server renders in a browser iframe, so browser support is relevant. Filter out due to technical inaccuracy.
• **Exa queries**: "colorjs.io Color.toString() output when color space is lch latest version", "CSS lch() and oklch() color functions browser support Firefox latest version", "caniuse CSS lch() oklch() color functions Firefox support", "caniuse CSS lch() color function browser support Firefox"</comment>
<file context>
@@ -0,0 +1,138 @@
+
+ color.lch.c! *= 0.8;
+
+ return color.toString();
+}
+
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: Functional Bugs Agent
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
`@types/color` provides typings for the `color` package, but the new code imports `Color` from `colorjs.io`. This mismatch leaves `colorjs.io` without typings and makes the new dependency misleading; please remove it or add the correct type declarations for `colorjs.io`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: `colorjs.io` already ships official TypeScript declarations (for example, the package’s published `types/` directory contains numerous `.d.ts` files such as `index.d.ts` and `color.d.ts`, e.g. https://www.bolt.magwiji.com/node_modules/colorjs.io/types/src/). Because the dependency itself includes typings, adding `@types/color` neither fixes nor relates to the new `colorjs.io` import. The reported issue that `colorjs.io` is left without typings is therefore incorrect.
• **Exa queries**: "colorjs.io TypeScript definitions package 0.5.2", ""colorjs.io" TypeScript definitions", ""colorjs.io" "index.d.ts""</violation>
</file>
<file name="packages/preview-server/src/app/preview/[...slug]/preview.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/preview.tsx:187">
EmailFrame does not forward refs, so makeIframeDocumentBubbleEvents is no longer attached and resizing breaks when the cursor enters the iframe.
• **Exa queries**: "React function components ref without forwardRef does ref work latest version", "react docs function components cannot be given refs forwardRef warning", "react forwardRef docs function components cannot be given refs warning"</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
@types/color provides typings for the color package, but the new code imports Color from colorjs.io. This mismatch leaves colorjs.io without typings and makes the new dependency misleading; please remove it or add the correct type declarations for colorjs.io.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: colorjs.io already ships official TypeScript declarations (for example, the package’s published types/ directory contains numerous .d.ts files such as index.d.ts and color.d.ts, e.g. https://www.bolt.magwiji.com/node_modules/colorjs.io/types/src/). Because the dependency itself includes typings, adding @types/color neither fixes nor relates to the new colorjs.io import. The reported issue that colorjs.io is left without typings is therefore incorrect.
• Exa queries: "colorjs.io TypeScript definitions package 0.5.2", ""colorjs.io" TypeScript definitions", ""colorjs.io" "index.d.ts""
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>`@types/color` provides typings for the `color` package, but the new code imports `Color` from `colorjs.io`. This mismatch leaves `colorjs.io` without typings and makes the new dependency misleading; please remove it or add the correct type declarations for `colorjs.io`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: `colorjs.io` already ships official TypeScript declarations (for example, the package’s published `types/` directory contains numerous `.d.ts` files such as `index.d.ts` and `color.d.ts`, e.g. https://www.bolt.magwiji.com/node_modules/colorjs.io/types/src/). Because the dependency itself includes typings, adding `@types/color` neither fixes nor relates to the new `colorjs.io` import. The reported issue that `colorjs.io` is left without typings is therefore incorrect.
• **Exa queries**: "colorjs.io TypeScript definitions package 0.5.2", ""colorjs.io" TypeScript definitions", ""colorjs.io" "index.d.ts""</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| width={width} | ||
| > | ||
| <iframe | ||
| <EmailFrame |
There was a problem hiding this comment.
EmailFrame does not forward refs, so makeIframeDocumentBubbleEvents is no longer attached and resizing breaks when the cursor enters the iframe.
• Exa queries: "React function components ref without forwardRef does ref work latest version", "react docs function components cannot be given refs forwardRef warning", "react forwardRef docs function components cannot be given refs warning"
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/preview.tsx at line 187:
<comment>EmailFrame does not forward refs, so makeIframeDocumentBubbleEvents is no longer attached and resizing breaks when the cursor enters the iframe.
• **Exa queries**: "React function components ref without forwardRef does ref work latest version", "react docs function components cannot be given refs forwardRef warning", "react forwardRef docs function components cannot be given refs warning"</comment>
<file context>
@@ -165,19 +184,18 @@ const Preview = ({ emailTitle, className, ...props }: PreviewProps) => {
width={width}
>
- <iframe
+ <EmailFrame
className="max-h-full rounded-lg bg-white [color-scheme:auto]"
+ darkMode={isDarkModeEnabled}
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
There was a problem hiding this comment.
6 issues found across 9 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="packages/preview-server/src/app/preview/[...slug]/preview.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/preview.tsx:187">
`EmailFrame` is a plain function component, so the `ref` you pass here never reaches the underlying `<iframe>`. As a result `makeIframeDocumentBubbleEvents` no longer runs and iframe events stop bubbling to the shell. Please forward the ref inside `EmailFrame` (e.g. with `React.forwardRef`).</violation>
</file>
<file name="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx">
<violation number="1" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:18">
This toggle renders only an icon, so it needs an accessible label. Add an `aria-label` (or similar) on `<Toggle.Root>` so screen readers can announce what the button does.</violation>
<violation number="2" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:27">
`<Toggle.Root>` is treated as a controlled toggle (the `enabled` prop drives styling), but the component never binds the Radix `pressed` state. When `enabled` is set externally (e.g., on initial load), the toggle's internal state and `aria-pressed` stay `false`, so assistive tech sees the wrong state. Bind `pressed={enabled}` and use `onPressedChange={onChange}` instead of the `onClick` handler.</violation>
</file>
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
Incorrect TypeScript types are installed for the new `colorjs.io` dependency. The `package.json` adds types for the `color` package, but the implementation uses `colorjs.io`. This will lead to type errors and developer confusion as the types do not match the library being used.</violation>
</file>
<file name="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:36">
The dark mode emulation only inverts colors from inline styles, ignoring styles defined in `<style>` blocks. Since many email components in this repository use `<style>` blocks for styling (e.g., for responsiveness), this will result in a broken user experience where only parts of the email are inverted, creating an inconsistent and unusable preview.</violation>
<violation number="2" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:40">
The color inversion logic does not account for emails that already have a dark mode implementation using standard methods like `@media (prefers-color-scheme: dark)`. This will cause rendering issues, such as double-inverting colors back to light, on well-authored email templates that provide their own dark theme.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Although the inversion runs unconditionally when darkMode is enabled, the toggle never switches the iframe’s prefers-color-scheme, so templates that provide their own dark-mode styles will not automatically apply them here. The claim of “double inversion” lacks supporting evidence and contradicts the actual flow, so this is too speculative to report.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| width={width} | ||
| > | ||
| <iframe | ||
| <EmailFrame |
There was a problem hiding this comment.
EmailFrame is a plain function component, so the ref you pass here never reaches the underlying <iframe>. As a result makeIframeDocumentBubbleEvents no longer runs and iframe events stop bubbling to the shell. Please forward the ref inside EmailFrame (e.g. with React.forwardRef).
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/preview.tsx at line 187:
<comment>`EmailFrame` is a plain function component, so the `ref` you pass here never reaches the underlying `<iframe>`. As a result `makeIframeDocumentBubbleEvents` no longer runs and iframe events stop bubbling to the shell. Please forward the ref inside `EmailFrame` (e.g. with `React.forwardRef`).</comment>
<file context>
@@ -165,19 +184,18 @@ const Preview = ({ emailTitle, className, ...props }: PreviewProps) => {
width={width}
>
- <iframe
+ <EmailFrame
className="max-h-full rounded-lg bg-white [color-scheme:auto]"
+ darkMode={isDarkModeEnabled}
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| return ( | ||
| <Tooltip> | ||
| <Tooltip.Trigger asChild> | ||
| <Toggle.Root |
There was a problem hiding this comment.
This toggle renders only an icon, so it needs an accessible label. Add an aria-label (or similar) on <Toggle.Root> so screen readers can announce what the button does.
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 18:
<comment>This toggle renders only an icon, so it needs an accessible label. Add an `aria-label` (or similar) on `<Toggle.Root>` so screen readers can announce what the button does.</comment>
<file context>
@@ -0,0 +1,38 @@
+ return (
+ <Tooltip>
+ <Tooltip.Trigger asChild>
+ <Toggle.Root
+ value="dark"
+ className={cn(
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| 'text-slate-12 bg-slate-4': enabled, | ||
| }, | ||
| )} | ||
| onClick={() => onChange(!enabled)} |
There was a problem hiding this comment.
<Toggle.Root> is treated as a controlled toggle (the enabled prop drives styling), but the component never binds the Radix pressed state. When enabled is set externally (e.g., on initial load), the toggle's internal state and aria-pressed stay false, so assistive tech sees the wrong state. Bind pressed={enabled} and use onPressedChange={onChange} instead of the onClick handler.
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 27:
<comment>`<Toggle.Root>` is treated as a controlled toggle (the `enabled` prop drives styling), but the component never binds the Radix `pressed` state. When `enabled` is set externally (e.g., on initial load), the toggle's internal state and `aria-pressed` stay `false`, so assistive tech sees the wrong state. Bind `pressed={enabled}` and use `onPressedChange={onChange}` instead of the `onClick` handler.</comment>
<file context>
@@ -0,0 +1,38 @@
+ 'text-slate-12 bg-slate-4': enabled,
+ },
+ )}
+ onClick={() => onChange(!enabled)}
+ >
+ <IconMoon />
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
Incorrect TypeScript types are installed for the new colorjs.io dependency. The package.json adds types for the color package, but the implementation uses colorjs.io. This will lead to type errors and developer confusion as the types do not match the library being used.
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>Incorrect TypeScript types are installed for the new `colorjs.io` dependency. The `package.json` adds types for the `color` package, but the implementation uses `colorjs.io`. This will lead to type errors and developer confusion as the types do not match the library being used.</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| const { contentDocument, contentWindow } = iframe; | ||
| if (!contentDocument || !contentWindow) return; | ||
|
|
||
| if (contentDocument.body.hasAttribute('inverted-colors')) return; |
There was a problem hiding this comment.
The color inversion logic does not account for emails that already have a dark mode implementation using standard methods like @media (prefers-color-scheme: dark). This will cause rendering issues, such as double-inverting colors back to light, on well-authored email templates that provide their own dark theme.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Although the inversion runs unconditionally when darkMode is enabled, the toggle never switches the iframe’s prefers-color-scheme, so templates that provide their own dark-mode styles will not automatically apply them here. The claim of “double inversion” lacks supporting evidence and contradicts the actual flow, so this is too speculative to report.
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 40:
<comment>The color inversion logic does not account for emails that already have a dark mode implementation using standard methods like `@media (prefers-color-scheme: dark)`. This will cause rendering issues, such as double-inverting colors back to light, on well-authored email templates that provide their own dark theme.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Although the inversion runs unconditionally when darkMode is enabled, the toggle never switches the iframe’s prefers-color-scheme, so templates that provide their own dark-mode styles will not automatically apply them here. The claim of “double inversion” lacks supporting evidence and contradicts the actual flow, so this is too speculative to report.</comment>
<file context>
@@ -0,0 +1,138 @@
+ const { contentDocument, contentWindow } = iframe;
+ if (!contentDocument || !contentWindow) return;
+
+ if (contentDocument.body.hasAttribute('inverted-colors')) return;
+
+ contentDocument.body.setAttribute('inverted-colors', '');
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: System Design Agent
| const colorRegex = | ||
| /#[0-9a-fA-F]{3,4}|#[0-9a-fA-F]{6,8}|rgba?\(.*?\)|hsl\(.*?\)|hsv\(.*?\)|oklab\(.*?\)|oklch\(.*?\)/g; | ||
|
|
||
| function applyColorInversion(iframe: HTMLIFrameElement) { |
There was a problem hiding this comment.
The dark mode emulation only inverts colors from inline styles, ignoring styles defined in <style> blocks. Since many email components in this repository use <style> blocks for styling (e.g., for responsiveness), this will result in a broken user experience where only parts of the email are inverted, creating an inconsistent and unusable preview.
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 36:
<comment>The dark mode emulation only inverts colors from inline styles, ignoring styles defined in `<style>` blocks. Since many email components in this repository use `<style>` blocks for styling (e.g., for responsiveness), this will result in a broken user experience where only parts of the email are inverted, creating an inconsistent and unusable preview.</comment>
<file context>
@@ -0,0 +1,138 @@
+const colorRegex =
+ /#[0-9a-fA-F]{3,4}|#[0-9a-fA-F]{6,8}|rgba?\(.*?\)|hsl\(.*?\)|hsv\(.*?\)|oklab\(.*?\)|oklch\(.*?\)/g;
+
+function applyColorInversion(iframe: HTMLIFrameElement) {
+ const { contentDocument, contentWindow } = iframe;
+ if (!contentDocument || !contentWindow) return;
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx">
<violation number="1" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:18">
Please bind the Radix toggle’s `pressed` state to the `enabled` prop so the rendered `aria-pressed` reflects the actual theme selection; otherwise screen readers will report the wrong state when the component initializes with dark mode enabled.
• **Exa queries**: "radix ui react toggle pressed state controlled"</violation>
</file>
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
`@types/color` provides type definitions for the `color` package, but the codebase imports `Color` from `colorjs.io`, which already ships its own TypeScript definitions. Keeping this mismatched types package adds an unused dependency and can cause confusion; please remove it.</violation>
</file>
<file name=".changeset/dirty-needles-chew.md">
<violation number="1" location=".changeset/dirty-needles-chew.md:2">
This changeset bumps `react-email`, but the dark mode switcher work in this PR targets the preview server package. Without releasing `@react-email/preview-server`, the new feature will not ship to users. Please update the changeset to point at the preview server package.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| return ( | ||
| <Tooltip> | ||
| <Tooltip.Trigger asChild> | ||
| <Toggle.Root |
There was a problem hiding this comment.
Please bind the Radix toggle’s pressed state to the enabled prop so the rendered aria-pressed reflects the actual theme selection; otherwise screen readers will report the wrong state when the component initializes with dark mode enabled.
• Exa queries: "radix ui react toggle pressed state controlled"
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 18:
<comment>Please bind the Radix toggle’s `pressed` state to the `enabled` prop so the rendered `aria-pressed` reflects the actual theme selection; otherwise screen readers will report the wrong state when the component initializes with dark mode enabled.
• **Exa queries**: "radix ui react toggle pressed state controlled"</comment>
<file context>
@@ -0,0 +1,38 @@
+ return (
+ <Tooltip>
+ <Tooltip.Trigger asChild>
+ <Toggle.Root
+ value="dark"
+ className={cn(
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
@types/color provides type definitions for the color package, but the codebase imports Color from colorjs.io, which already ships its own TypeScript definitions. Keeping this mismatched types package adds an unused dependency and can cause confusion; please remove it.
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>`@types/color` provides type definitions for the `color` package, but the codebase imports `Color` from `colorjs.io`, which already ships its own TypeScript definitions. Keeping this mismatched types package adds an unused dependency and can cause confusion; please remove it.</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "react-email": minor | |||
There was a problem hiding this comment.
This changeset bumps react-email, but the dark mode switcher work in this PR targets the preview server package. Without releasing @react-email/preview-server, the new feature will not ship to users. Please update the changeset to point at the preview server package.
Prompt for AI agents
Address the following comment on .changeset/dirty-needles-chew.md at line 2:
<comment>This changeset bumps `react-email`, but the dark mode switcher work in this PR targets the preview server package. Without releasing `@react-email/preview-server`, the new feature will not ship to users. Please update the changeset to point at the preview server package.</comment>
<file context>
@@ -0,0 +1,5 @@
+---
+"react-email": minor
+---
+
</file context>
[internal] Confidence score: 6/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
4 issues found across 9 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx">
<violation number="1" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:27">
`Toggle.Root` should be controlled with `pressed={enabled}` and `onPressedChange={onChange}`; relying on `onClick` leaves Radix’s internal `aria-pressed` state out of sync with the theme prop, breaking assistive technology expectations.</violation>
</file>
<file name="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:99">
`EmailFrame` is declared as a plain function component, so React will not forward refs to it. `preview.tsx` still renders `<EmailFrame ref={...}>` to wire `makeIframeDocumentBubbleEvents`, which now fails with the "Function components cannot be given refs" warning and breaks the bubbling logic. Wrap the component in `React.forwardRef` (or otherwise forward the iframe ref) so the parent ref keeps working.</violation>
<violation number="2" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:108">
The callback ref adds a `load` listener but relies on returning a cleanup function to remove it. React ignores return values from callback refs, so the listener is never removed; on re-renders new listeners accumulate and `applyColorInversion` fires multiple times. Move cleanup to the `iframe === null` branch or manage it with an effect so the listener is properly detached.</violation>
</file>
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
`@types/color` provides typings for the legacy `color` package, but this project depends on `colorjs.io`. Because the runtime import is `colorjs.io`, this added type package is unused and misleading; please remove it or replace it with typings for `colorjs.io`.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| 'text-slate-12 bg-slate-4': enabled, | ||
| }, | ||
| )} | ||
| onClick={() => onChange(!enabled)} |
There was a problem hiding this comment.
Toggle.Root should be controlled with pressed={enabled} and onPressedChange={onChange}; relying on onClick leaves Radix’s internal aria-pressed state out of sync with the theme prop, breaking assistive technology expectations.
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 27:
<comment>`Toggle.Root` should be controlled with `pressed={enabled}` and `onPressedChange={onChange}`; relying on `onClick` leaves Radix’s internal `aria-pressed` state out of sync with the theme prop, breaking assistive technology expectations.</comment>
<file context>
@@ -0,0 +1,38 @@
+ 'text-slate-12 bg-slate-4': enabled,
+ },
+ )}
+ onClick={() => onChange(!enabled)}
+ >
+ <IconMoon />
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| }: EmailFrameProps) { | ||
| return ( | ||
| <Slot | ||
| ref={(iframe: HTMLIFrameElement) => { |
There was a problem hiding this comment.
The callback ref adds a load listener but relies on returning a cleanup function to remove it. React ignores return values from callback refs, so the listener is never removed; on re-renders new listeners accumulate and applyColorInversion fires multiple times. Move cleanup to the iframe === null branch or manage it with an effect so the listener is properly detached.
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 108:
<comment>The callback ref adds a `load` listener but relies on returning a cleanup function to remove it. React ignores return values from callback refs, so the listener is never removed; on re-renders new listeners accumulate and `applyColorInversion` fires multiple times. Move cleanup to the `iframe === null` branch or manage it with an effect so the listener is properly detached.</comment>
<file context>
@@ -0,0 +1,138 @@
+}: EmailFrameProps) {
+ return (
+ <Slot
+ ref={(iframe: HTMLIFrameElement) => {
+ if (!iframe) return;
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| darkMode: boolean; | ||
| } | ||
|
|
||
| export function EmailFrame({ |
There was a problem hiding this comment.
EmailFrame is declared as a plain function component, so React will not forward refs to it. preview.tsx still renders <EmailFrame ref={...}> to wire makeIframeDocumentBubbleEvents, which now fails with the "Function components cannot be given refs" warning and breaks the bubbling logic. Wrap the component in React.forwardRef (or otherwise forward the iframe ref) so the parent ref keeps working.
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 99:
<comment>`EmailFrame` is declared as a plain function component, so React will not forward refs to it. `preview.tsx` still renders `<EmailFrame ref={...}>` to wire `makeIframeDocumentBubbleEvents`, which now fails with the "Function components cannot be given refs" warning and breaks the bubbling logic. Wrap the component in `React.forwardRef` (or otherwise forward the iframe ref) so the parent ref keeps working.</comment>
<file context>
@@ -0,0 +1,138 @@
+ darkMode: boolean;
+}
+
+export function EmailFrame({
+ markup,
+ width,
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
@types/color provides typings for the legacy color package, but this project depends on colorjs.io. Because the runtime import is colorjs.io, this added type package is unused and misleading; please remove it or replace it with typings for colorjs.io.
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>`@types/color` provides typings for the legacy `color` package, but this project depends on `colorjs.io`. Because the runtime import is `colorjs.io`, this added type package is unused and misleading; please remove it or replace it with typings for `colorjs.io`.</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
|
@cubic-dev-local review this |
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
1 similar comment
@sanxroz I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx">
<violation number="1" location="packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx:18">
Bind the Radix toggle to the controlled `enabled` state using `pressed={enabled}` with `onPressedChange={onChange}` instead of relying on `onClick`. Without this, `aria-pressed` and `data-state` stay `off` when the parent sets dark mode programmatically, so the button advertises the wrong state to assistive tech.</violation>
</file>
<file name="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx">
<violation number="1" location="packages/preview-server/src/app/preview/[...slug]/email-frame.tsx:122">
React ignores the value returned from a ref callback, so this cleanup never runs. Because this inline ref is recreated each render, the old listener stays attached and a new one is added, leading to accumulating `load` handlers on the same iframe. Please move the add/remove logic into an effect or handle the null ref case explicitly to detach the previous listener.</violation>
</file>
<file name="packages/preview-server/package.json">
<violation number="1" location="packages/preview-server/package.json:64">
`@types/color` provides typings for the `color` package, but the new code imports `colorjs.io`. This dependency does not match any usage and should be removed or replaced with the correct typings to avoid a dead/misleading dependency.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| return ( | ||
| <Tooltip> | ||
| <Tooltip.Trigger asChild> | ||
| <Toggle.Root |
There was a problem hiding this comment.
Bind the Radix toggle to the controlled enabled state using pressed={enabled} with onPressedChange={onChange} instead of relying on onClick. Without this, aria-pressed and data-state stay off when the parent sets dark mode programmatically, so the button advertises the wrong state to assistive tech.
Prompt for AI agents
Address the following comment on packages/preview-server/src/components/topbar/emulated-dark-mode-toggle.tsx at line 18:
<comment>Bind the Radix toggle to the controlled `enabled` state using `pressed={enabled}` with `onPressedChange={onChange}` instead of relying on `onClick`. Without this, `aria-pressed` and `data-state` stay `off` when the parent sets dark mode programmatically, so the button advertises the wrong state to assistive tech.</comment>
<file context>
@@ -0,0 +1,38 @@
+ return (
+ <Tooltip>
+ <Tooltip.Trigger asChild>
+ <Toggle.Root
+ value="dark"
+ className={cn(
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| }; | ||
|
|
||
| iframe.addEventListener('load', handleLoad); | ||
| return () => { |
There was a problem hiding this comment.
React ignores the value returned from a ref callback, so this cleanup never runs. Because this inline ref is recreated each render, the old listener stays attached and a new one is added, leading to accumulating load handlers on the same iframe. Please move the add/remove logic into an effect or handle the null ref case explicitly to detach the previous listener.
Prompt for AI agents
Address the following comment on packages/preview-server/src/app/preview/[...slug]/email-frame.tsx at line 122:
<comment>React ignores the value returned from a ref callback, so this cleanup never runs. Because this inline ref is recreated each render, the old listener stays attached and a new one is added, leading to accumulating `load` handlers on the same iframe. Please move the add/remove logic into an effect or handle the null ref case explicitly to detach the previous listener.</comment>
<file context>
@@ -0,0 +1,138 @@
+ };
+
+ iframe.addEventListener('load', handleLoad);
+ return () => {
+ iframe.removeEventListener('load', handleLoad);
+ };
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| "@react-email/components": "workspace:*", | ||
| "@types/babel__core": "7.20.5", | ||
| "@types/babel__traverse": "7.20.7", | ||
| "@types/color": "4.2.0", |
There was a problem hiding this comment.
@types/color provides typings for the color package, but the new code imports colorjs.io. This dependency does not match any usage and should be removed or replaced with the correct typings to avoid a dead/misleading dependency.
Prompt for AI agents
Address the following comment on packages/preview-server/package.json at line 64:
<comment>`@types/color` provides typings for the `color` package, but the new code imports `colorjs.io`. This dependency does not match any usage and should be removed or replaced with the correct typings to avoid a dead/misleading dependency.</comment>
<file context>
@@ -59,6 +61,7 @@
"@react-email/components": "workspace:*",
"@types/babel__core": "7.20.5",
"@types/babel__traverse": "7.20.7",
+ "@types/color": "4.2.0",
"@types/fs-extra": "11.0.1",
"@types/mime-types": "2.1.4",
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
We might also want to improve the images we have in
staticfor the demo, since they seem quite bad when using the new emulation. We also might want a better design touch, mainly for a way to tell users the dark mode is just an emulation, and not exactly what email clients do.How to use this?
react-email@canaryhttps://pkg.pr.new/resend/react-email/@react-email/preview-server@2589email devand see http://localhost:300or checkout the demo preview deploy for this, which also works!
Summary by cubic
Adds a dark mode switcher to the preview server that emulates email client color inversion for more realistic dark mode previews. Users can toggle light/dark in the topbar; the theme persists via URL.
New Features
?theme=URL param.Dependencies
color@5.0.2.