feat(web): customize themes with CSS and chat wallpaper#10412

🟠 High settings/TimelineBackgroundSettings.tsx:73

Files under MAX_COMPRESSIBLE_SOURCE_BYTES can still declare hundreds of millions of pixels, so compressImageForStash decodes a highly compressible PNG/WebP into a multi-gigabyte RGBA bitmap and can crash or OOM-kill the browser tab. Validate the image dimensions and enforce a decoded-pixel limit before calling compressImageForStash; the source-byte ceiling does not bound decoded memory.

This is an existing limitation of the shared imageCompression pipeline, also used by composer attachments and prompt stashes. General pre-decode dimension validation across supported image formats belongs in a shared pipeline hardening follow-up. Maria requested a focused settings simplification and asked us to avoid review-driven scope growth, so this layout pass retains the existing shared import policy.

🟡 Medium settings/SettingsPanels.tsx:721

restoreDefaults resets timelineBackgroundImage but does not invalidate an in-flight TimelineBackgroundSettings.applyImage request, so a later successful probe reapplies the image via persistClientSettingsUpdate and silently restores the wallpaper after the reset succeeds. Invalidate or cancel pending image requests before clearing the timeline background.

🟡 Medium chat/ChatTimelineBackground.tsx:23

HTTP-only image URLs pass the settings validation but fail to display when the hosted app is served over HTTPS, because this <img src> request is upgraded to HTTPS and then blocked. Restrict persisted URLs to https: for the hosted app, or surface this limitation before saving.

Tailwind ownership: Move these fixed blur and mask styles into Tailwind classes on the layer elements instead of an inline style object.

Suggested fix: No diff — encode each fixed layer's blur and mask as Tailwind arbitrary-value/property classes.

^{Posted via Macroscope — UI Consistency}

Maria explicitly requested reusing the progressive blur from her marketing redesign. These module-level generated masks preserve that implementation without duplicating seven long mask strings. Converting them to arbitrary Tailwind classes is a style-only rewrite outside this correction; retaining the existing implementation.

src/index.css:260

Tailwind ownership: Keep this static text-shadow and descendant styling on the components that render it as Tailwind arbitrary/descendant classes instead of adding a global utility.

Suggested fix: No diff — moving it requires updating both component call sites.

No longer relevant as of e29f94b975a53e3f49f76b7c4ad7f43f586941e7

🟠 High components/ChatView.tsx:8304

ChatTimelineBackground renders above the timeline rows at full wallpaper opacity, making messages unreadable while only the header and composer remain visible. The z-0 class overrides the component’s built-in -z-10 via cn/twMerge; remove it so the wallpaper stays behind the chat content.

🟡 Medium components/Sidebar.tsx:3

Removing the LinkBranchPullRequestButton render sites means a row with a discovered pr but no linked PR can no longer durably link that PR to its thread; users can only open the badge, and changing branches makes the discovery disappear. Restore the former render conditions and LinkBranchPullRequestButton usage so the discovered PR can be retained.

This removal is upstream commit 0a37240a87 (#11179), not wallpaper work. The latest head records that ancestry, and the sidebar files are no longer in the PR diff. Keeping upstream behavior.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Fix the invalid declaration position.

Stylelint reports no-invalid-position-declaration on this declaration. Restructure the fallback as a valid top-level @supports rule, or otherwise reorder the rule, so the stylesheet satisfies the configured lint contract.

Source: Linters/SAST tools

This is a Tailwind @utility definition. Tailwind expands the nested @supports declaration beneath the generated utility selector; it is valid in the generated CSS. Vite compilation and CI Check pass. Treating the unexpanded Tailwind source as ordinary CSS produces this false positive; moving it out would change the utility behavior.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use non-default values in the persistence fixture.

These assignments duplicate DEFAULT_CLIENT_SETTINGS. If serialization drops the wallpaper fields, decoding restores the same defaults and the test still passes.

Use a non-empty image and non-default opacity and blur values.

✅ Addressed in commit d56044a

Updated the existing fixture to a non-empty URL, 47% opacity, and 8px blur in d56044a0c4. All 11 desktop settings tests pass.

@maria-rcks, confirmed. The persistence fixture now uses non-default wallpaper values, so serialization cannot silently omit these fields. Thank you.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Abort the in-flight wallpaper import during editor cleanup.

request.current++ only ignores stale results. It does not stop compressImageForStash, the new Image() load/decode, or the 15-second timeout. Pass one cancellation signal through the compression stages where supported, and clear the probe, listeners, and timeout during cleanup. Treat cancellation as a silent exit.

🟡 Medium components/ChatView.tsx:5255

A header resize leaves the anchored message at its old timelineAnchorOffset, so it can remain underneath the top bar even though onTimelineAnchorReady has been recreated with the new offset. The positionedTimelineAnchorRef early return at line 5255 suppresses the corrective scrollToIndex; track the offset used for positioning and rerun when it changes.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the collapsed fade consistent with the glass surface.

surface-glass uses the user-configurable --glass-opacity, which defaults to 80%. The collapsed overlay instead reaches var(--card) at 95% opacity and can substantially reduce wallpaper visibility at the bottom of the card. Use a glass-based fade or a mask fade.

The fade exists to hide the clipped plan text under the expand control, so it has to end near-opaque in the card color; a mask fade would let that text show through the glass. The card now only uses surface-glass when a wallpaper is set, and with one set the fade still resolves to the same card color the glass tints toward, so the bottom of a collapsed plan reads as slightly more solid card, which is the intent. Leaving as is.

@maria-rcks, thanks for the clarification. The near-opaque from-card/95 fade intentionally hides clipped plan text. A glass or mask fade would weaken that behavior. I am withdrawing this finding.

🐇

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium src/themePalette.ts:1413

Choosing Keep Both for an imported theme with an existing id drops its custom CSS, so the copied theme installs without theme.css even though a non-conflicting import preserves it. ThemeImportDialog.versionedCopy rebuilds both copy candidates with colors and variants but omits theme.css; include css when constructing each candidate.

fixed in bf3b0f677c. both copy candidates retain css. batch renaming does too, and automatic pairing keeps themes with conflicting css separate. focused import and timeline checks passed on blacksmith.

Confirmed: both copy paths now preserve css.

🟡 Medium components/ChatView.tsx:5370

With a timeline background, timelineRealContentOverflowsViewport subtracts timelineHeaderInset twice, so short conversations are reported as overflowing even when their rows cannot scroll. An upward wheel then calls cancelTimelineLiveFollowForUserNavigation, disabling end-follow and causing later streamed output to stop tracking the end. Pass an offset that excludes the header inset, and update the callback dependencies accordingly.

fixed in bf3b0f677c. both overflow callers and the active-turn end-reveal calculation now use the constant clearance. initial anchoring still includes the header. 76 focused timeline tests passed on blacksmith.

🟠 High chat/MessagesTimeline.tsx:4333

The row errors during a transition from no props.icon to an icon instead of rendering the newly available icon. Because the stable-row logic enriches the same cached row, use(TimelineRowCtx) is skipped on the initial render and then called on the next render after the conditional return, violating React's hook-order requirement; move the hook before the fallback branch.

No longer relevant as of bf3b0f677c6b05eacebe68bc32ea1d9b1dc2f309

this uses React's use(context) API, which explicitly supports conditional calls and calls after early returns. it is not useContext; the reported hook-order failure does not apply. leaving the existing behavior unchanged. https://react.dev/reference/react/use

🟡 Medium chat/ChangedFilesTree.tsx:62

When glass is true, the sticky header has no background, so scrolling file rows show through its z-10 layer and make the summary and controls difficult to read. Apply CHAT_BACKGROUND_GLASS_SURFACE_CLASSES in the glass branch as well.

No longer relevant as of edb2682af5093d9c3f7a6450b173b78e9997119d

fixed at edb2682af5: the sticky changed-file header uses the existing glass surface when a wallpaper is enabled. web typecheck and scoped lint/formatting passed; actual-client scroll capture is being completed.