Implemented an effective 'dark mode' when tertiary color is too light #586
Conversation
Klakurka
added
bug
WalkthroughAdds a new color utility module for color normalization and WCAG luminance-based darkMode detection, re-exports it from util index, updates the Widget component to use darkMode on the theme tertiary color to switch many UI styles, and adjusts a test mock to preserve actual util exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
react/lib/components/Widget/Widget.tsx (2)
353-365: Dark-mode styling based onisLightTertiarylooks coherentThe conditional styling for:
root/header backgrounds ('#2a2a2a' / '#3a3a3a'vs light backgrounds),- QR container background/border,
- copy text container & text shadow,
- footer and SideShift link colors, and
- QRCodeSVG
bgColor,all consistently flip to a darker shell when
theme.palette.tertiaryis light, while keeping text/QR foreground astheme.palette.tertiary. This should address the “white tertiary makes QR unreadable” issue without breaking the default light theme.One minor note:
isLightTertiaryalready derives purely fromtheme.palette.tertiary, so theuseMemohere is more an optimization than a necessity; you could inlineisLightColor(theme.palette.tertiary)and drop the extra hook if you prefer less hook surface.Also applies to: 384-399, 409-432, 484-485, 1033-1047, 1064-1071
1350-1352: Donation UI colors correctly adapt, but rely on hex/named palette valuesThe donation heart icon stroke and the donation rate input/label colors now branch on
isLightTertiary, which keeps them visible against both darkened and light backgrounds.Be aware this relies on
theme.palette.tertiarybeing a hex or named color thatisLightColorcan parse; if you ever move to CSS variables orrgb()/hsl()strings in the palette,isLightColorwill currently treat them as “dark” (luminance 0). In that case you’d want to extend the color utility to handle those formats.Also applies to: 1381-1389, 1399-1402
react/lib/util/color.ts (2)
5-150: Named-color map and normalization are fine but limited to names and hexThe
colorNameToHextable plusnormalizeColorToHexgive you robust coverage for common CSS named colors and 3/6‑digit hex (with or without#). That’s sufficient for typical theme palette values.Right now anything that isn’t a known name or hex (e.g.
rgb(...),hsl(...),var(--color)or 4/8‑digit hex with alpha) will normalize to an invalid hex and end up with luminance0. That’s a safe failure mode, but if you ever store such formats intheme.palette.*,isLightColorwill misclassify them as “dark”. Consider documenting this or extending normalization when/if those formats appear in the theme.Also applies to: 178-190
220-227:isLightColorthreshold is reasonable; consider centralizing if reusedWrapping
getLuminancewith a defaultthreshold = 0.5is straightforward and produces intuitive results (dark vs light). If you expect multiple call sites with different thresholds, consider exporting a named constant (e.g.DEFAULT_LIGHT_THRESHOLD) so you don’t end up with scattered magic numbers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
react/lib/components/Widget/Widget.tsx(12 hunks)react/lib/util/color.ts(1 hunks)react/lib/util/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/Widget/Widget.tsx (2)
react/lib/util/color.ts (1)
isLightColor(225-227)docs/darkmode.js (1)
theme(39-39)
🪛 GitHub Actions: Run tests
react/lib/util/color.ts
[error] 1-1: Test suite failed: Unhandled exception in Widget.test.tsx caused by runtime error in Widget component.
react/lib/components/Widget/Widget.tsx
[error] 326-326: Uncaught TypeError: (0 , util_1.isLightColor) is not a function. Ensure isLightColor is exported from the util module and properly imported in Widget.tsx.
react/lib/util/index.ts
[error] 1-1: Test suite failed: Unhandled exception in Widget.test.tsx caused by runtime error in Widget component.
⏰ 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). (1)
- GitHub Check: Run build
🔇 Additional comments (2)
react/lib/util/color.ts (1)
155-172: Luminance computation matches WCAG 2.0 and looks correct
hexToRgbcorrectly handles 3- and 6-digit hex and cleanly returnsnullfor invalid values.getLuminance:
- Normalizes the input color,
- Converts to sRGB,
- Applies gamma correction, and
- Uses the WCAG coefficients (0.2126, 0.7152, 0.0722),
so the luminance result will be in
[0, 1]as expected. This is appropriate for the light/dark decision logic.Also applies to: 197-218
react/lib/components/Widget/Widget.tsx (1)
44-45: I'll help you verify the concerns in this review comment. Let me start by exploring the codebase to confirm the issues with theisLightColorimport and util exports.
<function_calls>
#!/bin/bashFirst, let's locate Widget.tsx and understand the file structure
echo "== Finding Widget.tsx =="
fd "Widget.tsx" -t fecho
echo "== Checking repo structure =="
git ls-files react/lib/components/Widget/ 2>/dev/null | head -20
</function_calls>
<function_calls>
#!/bin/bashLet's look at the util barrel and its exports
echo "== Util barrel structure =="
fd "util" -t d react/lib/ | head -10echo
echo "== Check for index files in util =="
fd "index.(ts|tsx|js|jsx)" react/lib/util/ -o react/lib/utils/ 2>/dev/null | head -10
</function_calls>
<function_calls>
#!/bin/bashSearch for isLightColor definition and exports
echo "== Search for isLightColor definition =="
rg -n "isLightColor" --type ts --type tsx -A 2 -B 2echo
echo "== Search for color utility files =="
fd "color" -t f react/lib/util/
</function_calls>
<function_calls>
#!/bin/bashCheck util directory structure more closely
echo "== Contents of react/lib/util =="
ls -la react/lib/util/ 2>/dev/null || echo "Directory not found at react/lib/util"echo
echo "== Try alternate paths =="
fd "util" -t d | grep -E "(react|lib)" | head -10
</function_calls>
<function_calls>
#!/bin/bashList all TypeScript/TSX files in the repo to understand structure
echo "== Repository structure =="
git ls-files | grep -E ".(ts|tsx)$" | head -30
</function_calls>
<function_calls>
#!/bin/bashLook at Widget.tsx content around lines 44-45
echo "== Widget.tsx content (lines 1-100) =="
find . -name "Widget.tsx" -type f -exec cat -n {} ; | head -100
</function_calls>
<function_calls>
#!/bin/bashSearch more broadly for isLightColor
echo "== Broad search for isLightColor =="
rg "isLightColor" -necho
echo "== Search for color-related exports =="
rg "export.*color" -n --type ts --type tsx | head -20
</function_calls>
<function_calls>
#!/bin/bashCheck for Jest mock files
echo "== Search for mocks directories =="
find . -type d -name "mocks"echo
echo "== Search for jest.mock calls =="
rg "jest.mock" -n -A 3 -B 1
</function_calls>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!trimmed) return null; | ||
|
|
||
| // Use canvas to parse any CSS color value | ||
| const ctx = document.createElement('canvas').getContext('2d'); |
There was a problem hiding this comment.
logic: SSR compatibility: document is not available in server-side environments
| const ctx = document.createElement('canvas').getContext('2d'); | |
| const ctx = typeof document !== 'undefined' ? document.createElement('canvas').getContext('2d') : null; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: react/lib/util/color.ts
Line: 34:34
Comment:
**logic:** SSR compatibility: `document` is not available in server-side environments
```suggestion
const ctx = typeof document !== 'undefined' ? document.createElement('canvas').getContext('2d') : null;
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
react/lib/util/color.ts (1)
27-40: GuardnormalizeColorToHexfor non-browser / SSR environments
normalizeColorToHexassumesdocument.createElement('canvas').getContext('2d')is available. In environments without a DOM or canvas implementation (SSR, some test runners),documentorgetContextcan throw before thectxnull-check, which would bubble up throughgetLuminance/darkMode.To make this utility safer without changing behavior in the browser, consider an early guard and a defensive
getContextcall:export const normalizeColorToHex = (color: string): string | null => { if (!color) return null; const trimmed = color.trim(); if (!trimmed) return null; - - // Use canvas to parse any CSS color value - const ctx = document.createElement('canvas').getContext('2d'); - if (!ctx) return null; - - ctx.fillStyle = trimmed; - // The browser normalizes the color to a hex string (or rgb() for transparent colors) - return ctx.fillStyle; + + // Use canvas to parse any CSS color value; bail out if DOM/canvas are unavailable + if (typeof document === 'undefined') return null; + + const canvas = document.createElement('canvas'); + const ctx = canvas.getContext && canvas.getContext('2d'); + if (!ctx) return null; + + ctx.fillStyle = trimmed; + // The browser normalizes the color to a hex string (or rgb()/rgba() for transparent colors) + return ctx.fillStyle; };That way
darkModesimply returnsfalse(via luminance 0) when no DOM/canvas is available, instead of throwing.If you expect this code to run under SSR or in non-jsdom test environments, it’s worth quickly confirming how
document.createElement('canvas').getContext('2d')behaves there and adjusting the guard accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
react/lib/components/Widget/Widget.tsx(12 hunks)react/lib/tests/components/Widget.test.tsx(1 hunks)react/lib/util/color.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- react/lib/components/Widget/Widget.tsx
⏰ 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). (1)
- GitHub Check: Run build
🔇 Additional comments (3)
react/lib/tests/components/Widget.test.tsx (1)
8-47: Partial util mock correctly preserves real exportsSpreading
jest.requireActual('../../util')and then overriding specific functions is a solid pattern here: it keeps new utilities (like the color helpers/dark mode) using their real implementations while safely stubbing network and formatting helpers. This should make the tests more robust to future additions to../../utilwithout accidentally auto-mocking them.If you want extra certainty, you can quickly log
Object.keys(jest.requireActual('../../util'))in a local throwaway test to confirm all expected exports (including the new color utilities) are visible under this mock shape.react/lib/util/color.ts (2)
4-21: WCAG luminance helperhexToRgbimplementation looks correctThe hex parsing logic here is clean and idiomatic: stripping the leading
#, expanding 3‑digit hex to 6‑digit, validating via regex, and returning parsed RGB ornullon invalid input all look good and align with the intended usage ingetLuminance.
47-77: WCAG luminance anddarkModethresholding are implemented correctlyThe
getLuminancefunction correctly follows the WCAG 2.0 relative luminance formula (sRGB normalization, gamma correction, and the 0.2126/0.7152/0.0722 weighting), anddarkModecleanly wraps it with a configurable threshold (default 0.5) to flip into a dark UI when the input color is light. This matches the PR goal of triggering dark mode when the tertiary color is very light (e.g., white).
Related to #238
Description
If somebody sets the tertiary color to white (or something close), it makes the QR code and other text unusable.
Test plan
Set a button's tertiary color to be very light (eg.
#FFFFFF) and see how the dialog goes into a dark mode so that the QR code and misc text is actually visible.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Greptile Overview
Greptile Summary
This PR implements automatic dark mode for the PayButton widget when the tertiary color is too light (e.g., white or near-white), addressing the visibility issue where QR codes and text become unreadable.
Key Changes:
color.tsutility with WCAG 2.0 compliant luminance calculation to detect light colorsisDarkModecalculation to avoid unnecessary recalculationsIssues Found:
document.createElementwill crash in server-side rendering environmentsConfidence Score: 3/5
typeof document !== 'undefined'check).react/lib/util/color.ts- needs SSR compatibility fix before deploymentImportant Files Changed
File Analysis
documentusageisDarkModecalculationSequence Diagram
sequenceDiagram participant User participant Widget participant useTheme participant darkMode participant normalizeColorToHex participant getLuminance participant UI as UI Components User->>Widget: Render with theme.tertiary color Widget->>useTheme: Get theme palette useTheme-->>Widget: theme.palette.tertiary Widget->>darkMode: Calculate isDarkMode(tertiary) darkMode->>normalizeColorToHex: Normalize color to hex normalizeColorToHex->>normalizeColorToHex: document.createElement('canvas') normalizeColorToHex->>normalizeColorToHex: ctx.fillStyle = color normalizeColorToHex-->>darkMode: Hex color string darkMode->>getLuminance: Calculate luminance(hex) getLuminance->>getLuminance: Apply WCAG 2.0 formula getLuminance-->>darkMode: Luminance value (0-1) darkMode->>darkMode: Compare luminance > 0.5 darkMode-->>Widget: isDarkMode boolean Widget->>Widget: useMemo caches result Widget->>UI: Apply conditional styles alt isDarkMode = true UI->>UI: Dark backgrounds (#2a2a2a, #1a1a1a) UI->>UI: Light text colors UI->>UI: Dark QR background else isDarkMode = false UI->>UI: Light backgrounds (#f5f5f7, #fff) UI->>UI: Dark text colors UI->>UI: Light QR background end UI-->>User: Render readable widget