fix(preview): dark swatches respect the dark-overrides-enabled toggle - #152
Merged
Conversation
When dark overrides are OFF the live preview still injected explicit
--sf-color-*-dark vars (from stored values or default hex-hint
fallbacks), so Dark mode showed wrong colors the framework wouldn't use.
Read dark_overrides_enabled in inlineStyle:
- ON (default): inject stored/hint dark values as before.
- OFF: inject the same CSS relative-color formula the framework uses:
brand/status: oklch(from var(--sf-color-X-light)
clamp(0.65, calc(0.95 - l * 0.5), 0.88) calc(c * 0.9) h)
base: oklch(from var(--sf-color-base-light)
clamp(0.16, calc(1.18 - l), 0.24) calc(c * 0.5) h)
https://claude.ai/code/session_019MD9F8rrpYzCeDTmSTLA1K
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLivePreview.svelte adds module-level helpers to derive dark color values and changes inline CSS generation to choose dark tokens based on ChangesDark Mode Color Variable Generation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
When the dark-override toggle is ON but only some colors have an explicit dark value, the preview was falling back to the default brand_dark_hex_hints — hardcoded palette swatches that don't reflect the user's customised light colors. The framework itself falls back to auto-derivation (CSS relative color syntax) for any --sf-color-*-dark that isn't explicitly set. The preview now does the same: a stored dark value is used when present; otherwise the autoDark() formula is applied regardless of toggle state. This makes partially-overridden palettes render accurately in both Dark and Light preview modes. https://claude.ai/code/session_019MD9F8rrpYzCeDTmSTLA1K
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integrations/bricks/admin-app/src/components/LivePreview.svelte (1)
57-72: ⚡ Quick winCentralize the derived dark formula to avoid drift from
core/tokens.css.The relative-color formula is inlined three times: brand-base, brand non-base, and status (with brand non-base and status using the same expression). Extract two small helpers so all call sites share one source of truth.
♻️ Proposed refactor
Define once (near the other module-level constants around Line 29):
const brandDark = (light) => `oklch(from var(${light}) clamp(0.65, calc(0.95 - l * 0.5), 0.88) calc(c * 0.9) h)`; const baseDark = (light) => `oklch(from var(${light}) clamp(0.16, calc(1.18 - l), 0.24) calc(c * 0.5) h)`;Then in the loops:
} else { - const formula = name === 'base' - ? `oklch(from var(--sf-color-base-light) clamp(0.16, calc(1.18 - l), 0.24) calc(c * 0.5) h)` - : `oklch(from var(--sf-color-${name}-light) clamp(0.65, calc(0.95 - l * 0.5), 0.88) calc(c * 0.9) h)`; - pairs.push(`--sf-color-${name}-dark:${formula}`); + const light = `--sf-color-${name}-light`; + pairs.push(`--sf-color-${name}-dark:${name === 'base' ? baseDark(light) : brandDark(light)}`); }- } else { - pairs.push(`--sf-color-${name}-dark:oklch(from var(--sf-color-${name}-light) clamp(0.65, calc(0.95 - l * 0.5), 0.88) calc(c * 0.9) h)`); - } + } else { + pairs.push(`--sf-color-${name}-dark:${brandDark(`--sf-color-${name}-light`)}`); + }The framework tokens derive non-base dark with
clamp(.65, calc(.95 - l * .5), .88) ... c * .9and base dark withclamp(.16, calc(1.18 - l), .24) ... c * .5, so this refactor should remain behavior-preserving while reducing copy/paste drift risk.🤖 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 `@integrations/bricks/admin-app/src/components/LivePreview.svelte` around lines 57 - 72, Extract two module-level helper functions, e.g. brandDark(light) and baseDark(light), near the other constants around the top of LivePreview.svelte (around the existing module-level constants) that return the respective derived dark formulas (base uses clamp(0.16, calc(1.18 - l), 0.24) calc(c * 0.5) and non-base/status use clamp(0.65, calc(0.95 - l * 0.5), 0.88) calc(c * 0.9)). Then replace the three inlined formulas in the loops: the base branch where name === 'base' (currently producing the longer base formula), the brand non-base branch, and the status else branch (currently producing the non-base/status formula) so they call baseDark(`--sf-color-${name}-light`) or brandDark(`--sf-color-${name}-light`) respectively; keep the existing variable names (pairs, name, statuses, colors, defaultColors, darkEnabled) and preserve behavior/arguments.
🤖 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 `@integrations/bricks/admin-app/src/components/LivePreview.svelte`:
- Around line 57-72: Extract two module-level helper functions, e.g.
brandDark(light) and baseDark(light), near the other constants around the top of
LivePreview.svelte (around the existing module-level constants) that return the
respective derived dark formulas (base uses clamp(0.16, calc(1.18 - l), 0.24)
calc(c * 0.5) and non-base/status use clamp(0.65, calc(0.95 - l * 0.5), 0.88)
calc(c * 0.9)). Then replace the three inlined formulas in the loops: the base
branch where name === 'base' (currently producing the longer base formula), the
brand non-base branch, and the status else branch (currently producing the
non-base/status formula) so they call baseDark(`--sf-color-${name}-light`) or
brandDark(`--sf-color-${name}-light`) respectively; keep the existing variable
names (pairs, name, statuses, colors, defaultColors, darkEnabled) and preserve
behavior/arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4178607-a9ed-4e8c-9efa-89b9c1a494c8
📒 Files selected for processing (2)
integrations/bricks/admin-app/src/components/LivePreview.svelteintegrations/bricks/assets/admin-app/app.js
Move autoDarkStandard/autoDarkBase/autoDark out of the $derived.by() closure so they are defined once at module load rather than recreated on every reactive re-render. Behaviour is unchanged. https://claude.ai/code/session_019MD9F8rrpYzCeDTmSTLA1K
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--sf-color-*-darkcustom properties — from previously stored token values or the default hex-hint fallbacks — regardless of the toggle state. Switching to Dark mode in the preview showed those stored/default colors instead of the auto-derived ones.inlineStylenow readsdark_overrides_enabledfrom the colors section and branches:The formulas mirror
core/tokens.cssverbatim:Test plan
https://claude.ai/code/session_019MD9F8rrpYzCeDTmSTLA1K
Generated by Claude Code
Summary by CodeRabbit