feat: add custom gradient editor to background settings#81
Conversation
Adds a color-wheel-based gradient editor to the background settings panel, allowing users to build custom linear gradients instead of being limited to presets only. - New GradientEditor component with color wheel, harmony modes, brightness slider, and gradient angle knob - New buildGradientFromEditor utility to convert editor state to CSS - classifyWallpaper now handles composite backgrounds (url overlay + trailing gradient) so custom gradients work with the existing preview - i18n keys for 'presets' and 'custom' labels across all 13 locales - Unit tests for gradientBuilder and the new composite-background parsing
📝 WalkthroughWalkthroughAdds an interactive gradient editor, CSS gradient construction, video background integration, localized section labels, and composite wallpaper gradient classification with tests. ChangesCustom gradient backgrounds
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GradientEditor
participant SettingsPanel
participant buildGradientFromEditor
participant WallpaperState
User->>GradientEditor: Adjust wheel, brightness, or angle
GradientEditor->>SettingsPanel: Emit editor state
SettingsPanel->>buildGradientFromEditor: Build CSS gradient
buildGradientFromEditor-->>SettingsPanel: Return gradient string
SettingsPanel->>WallpaperState: Persist gradient wallpaper
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (1)
src/components/ui/gradient-editor.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type-only React event import.
Reactis only referenced as a type. ImportPointerEventwithtypeand replaceReact.PointerEventto preserve Biome’s type-only import discipline.As per coding guidelines, “Follow the repo's import discipline, including type-only imports handled by Biome.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/gradient-editor.tsx` at line 1, Update the import in gradient-editor.tsx to bring in PointerEvent as a type-only import, then replace all React.PointerEvent references with PointerEvent so React is no longer imported solely for a type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/gradient-editor.tsx`:
- Around line 266-359: Ensure all drag interactions clean up global pointer
listeners on both pointerup and pointercancel, and when the component unmounts.
Update the handlers associated with
onMainPointerDown/onMainPointerMove/onMainPointerUp, onSliderPointerDown, and
onAngleKnobPointerDown to use shared or stable cleanup logic, and add unmount
cleanup that removes any active listeners and resets drag state.
- Around line 398-408: Make the gradient controls keyboard accessible: update
the main draggable point, brightness slider, and angle knob in the corresponding
component (including the controls around the referenced symbols) to use
focusable elements with appropriate slider semantics, accessible labels, and
keyboard handlers. Support arrow-key adjustments that update the same gradient
state as pointer interactions, and preserve pointer behavior and visual styling.
In `@src/lib/gradientBuilder.test.ts`:
- Around line 5-13: Update the test fixture function baseState to include the
required GradientEditorState angle property, using a representative value such
as 135 alongside the existing default state fields.
In `@src/lib/wallpaper.ts`:
- Around line 46-55: Update extractTrailingGradient to recognize
repeating-linear-gradient, repeating-radial-gradient, and
repeating-conic-gradient in addition to the existing variants, while preserving
the current trailing-gradient extraction behavior and alignment with
GRADIENT_RE.
---
Nitpick comments:
In `@src/components/ui/gradient-editor.tsx`:
- Line 1: Update the import in gradient-editor.tsx to bring in PointerEvent as a
type-only import, then replace all React.PointerEvent references with
PointerEvent so React is no longer imported solely for a type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ba8c67e-d037-4872-a850-2052f75530ce
📒 Files selected for processing (19)
src/components/ui/gradient-editor.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/it/settings.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/pt-BR/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/vi/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.jsonsrc/lib/gradientBuilder.test.tssrc/lib/gradientBuilder.tssrc/lib/wallpaper.test.tssrc/lib/wallpaper.ts
…port order - Type-only import for PointerEvent (Biome import discipline) - Drag handlers clean up on pointerpointercancel and unmount via shared ref - Main point, brightness slider, and angle knob are keyboard-focusable with role=slider, aria-* attrs, and arrow-key handlers - baseState() test fixture includes the required angle field - extractTrailingGradient handles repeating-* gradient variants - Added test for composite backgrounds with repeating-linear-gradient
EtienneLescot
left a comment
EtienneLescot
left a comment
There was a problem hiding this comment.
Review — custom gradient editor
Well-structured, self-contained feature with good coverage at the pure-function layer (buildGradientFromEditor, classifyWallpaper). One must-fix correctness bug and a couple of quality/robustness items, left as inline comments.
Must-fix
- Duplicate point IDs / React keys after the remove→add interaction (see inline on
addPoint).
Recommended
- Preview brightness (CSS
filter) doesn't match the applied brightness (linear per-channel multiply) — WYSIWYG mismatch. extractTrailingGradientparsing is whitespace-sensitive.
Nits (non-blocking)
movedRefis written but never read (dead code).- Add/remove/harmony
<button>s lacktype="button". - No tests for the editor's add/remove/harmony state machine — a unit test on ID generation would have caught the key-collision bug.
Nice work overall — the polar/HSL math and the pointer-drag cleanup (matched add/remove + unmount effect) are handled carefully, and the i18n coverage across all 13 locales is complete.
|
|
||
| const removePoint = () => { | ||
| if (totalColors <= 1) return; | ||
| setOrbitPoints((prev) => prev.slice(0, -1)); |
There was a problem hiding this comment.
Duplicate point IDs / React keys (must-fix).
idCounter starts at 1 (line 30) but the initial orbit points are hardcoded "o1"/"o2" (line 197). Because totalColors starts at MAX_COLORS (3), Add is disabled until the user removes a point first — so the common path is remove→add on a fresh mount:
- remove →
orbitPoints = [{ id: "o1" }] - add →
id = "o1"(idCounter was 1) →orbitPoints = [{ id: "o1" }, { id: "o1" }]
Both points now share id: "o1", so they render with the same key → React duplicate-key warning and possible DOM-node reuse glitches during reconciliation.
Fix: seed the counter past the initial IDs (e.g. const idCounter = useRef(orbitPoints.length + 1)), generate the initial IDs from the same factory, or use crypto.randomUUID(). A ref-based counter is also cleaner than the module-level let, which is currently shared across every mounted instance.
|
|
||
| /* ---------- keyboard handlers ---------- */ | ||
| const onMainKeyDown = useCallback((e: React.KeyboardEvent) => { | ||
| const step = e.shiftKey ? 10 : 5; |
There was a problem hiding this comment.
Preview vs. applied brightness mismatch.
The wheel dims its points with a CSS filter here: brightness(0.62 * (0.55 + (brightness/100)*0.9)). But the exported gradient (applyBrightness in gradientBuilder.ts) uses a plain linear per-channel multiply k = brightness/100.
At brightness = 50 the preview renders brightness(0.62) while the output multiplies channels by 0.5 — the color the user drags to isn't the color they get. Worth reconciling the two formulas (or documenting the intent), since direct manipulation is the whole point of the wheel.
| "repeating-radial", | ||
| "repeating-conic", | ||
| ]) { | ||
| const idx = value.lastIndexOf(`, ${type}-gradient(`); |
There was a problem hiding this comment.
Whitespace-sensitive parsing.
lastIndexOf(', ' + type + '-gradient(') matches exactly one comma + one space, and start = Math.max(...matches) + 2 hardcodes skipping the , . A composite serialized as url(...),linear-gradient(...) (no space) or with two spaces won't match and silently falls through to { kind: "color" }.
Since buildGradientFromEditor never emits composites, the only sources are external/other CSS where spacing isn't guaranteed. A regex reusing the existing gradient vocabulary would be more robust and drop the magic + 2:
const m = value.match(/,\s*((?:repeating-)?(?:linear|radial|conic)-gradient\()/g);Low severity, but easy to harden.
- Replace module-level idCounter with per-instance useRef seeded from the initial orbit points, fixing duplicate React keys after remove→add - Remove unused movedRef dead code - Align preview brightness filter with applyBrightness multiplier (WYSIWYG) - Add type=button to add/remove/harmony controls - Make extractTrailingGradient whitespace-tolerant with regex - Add gradient-editor.test.tsx covering ID generation, add/remove/harmony state machine, and disabled button states
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ui/gradient-editor.test.tsx (1)
1-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM!
The tests solidly cover the core
onChangecontract: unique point IDs, "main" persistence, button disabled-state boundaries atMAX_COLORS/one-color, and harmony cycling. ThegetLastStatehelper is well-guarded with a call-count assertion before indexing. Type-only import on line 4 and theas GradientEditorStatecast on line 10 are both clean.One optional observation: the PR objectives mention draggable color points, brightness control, and gradient angle control, but the current suite only exercises button-click interactions. Consider adding pointer/drag interaction tests and brightness/angle change assertions in a follow-up to broaden coverage of the editor's interactive surface.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/gradient-editor.test.tsx` around lines 1 - 69, Optionally expand the GradientEditor test suite with interaction coverage beyond the existing button tests: add pointer/drag tests for draggable color points, and assertions that brightness and gradient-angle controls emit the expected updated state through onChange. Keep the current ID, button-boundary, and harmony-cycle tests unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/ui/gradient-editor.test.tsx`:
- Around line 1-69: Optionally expand the GradientEditor test suite with
interaction coverage beyond the existing button tests: add pointer/drag tests
for draggable color points, and assertions that brightness and gradient-angle
controls emit the expected updated state through onChange. Keep the current ID,
button-boundary, and harmony-cycle tests unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 398f8ad7-5a13-4f76-9780-8db8601475ca
📒 Files selected for processing (4)
src/components/ui/gradient-editor.test.tsxsrc/components/ui/gradient-editor.tsxsrc/lib/wallpaper.test.tssrc/lib/wallpaper.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/wallpaper.test.ts
- src/lib/wallpaper.ts
- src/components/ui/gradient-editor.tsx
2 participants
Summary
Adds a color-wheel-based gradient editor to the background settings panel, allowing users to build custom linear gradients instead of being limited to presets only.
What changed
GradientEditorcomponent — interactive color wheel with draggable main point, harmony-driven secondary points (complementary, analogous, split-complementary, triadic, square), brightness slider, and gradient angle knobbuildGradientFromEditorutility — converts editor state to a clean CSSlinear-gradientclassifyWallpaperenhancement — now handles composite backgrounds (e.g.url("noise.png"), linear-gradient(...)) by extracting the trailing gradient, so custom gradients work with the existing live previewbackground.presetsandbackground.customkeys across all 13 localesScreenshot
Type of change
Release impact
Desktop impact
Testing
npm run lint— cleannpm run test— 320/320 passed (including 4 new gradientBuilder tests and 1 new classifyWallpaper composite-background test)New files
src/components/ui/gradient-editor.tsxsrc/lib/gradientBuilder.tssrc/lib/gradientBuilder.test.tsModified files
src/lib/wallpaper.tssrc/lib/wallpaper.test.tssrc/components/video-editor/SettingsPanel.tsxsrc/i18n/locales/*/settings.jsonpresetsandcustomkeys (13 locales)Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests