fix(themes): refactor conversions and fix out of bound colors#86
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates replace numeric-scale color token values with new oklch(...) constants across dark and base theme ref.color palettes; base theme also moves font.line-height named tokens to appear after numeric entries. ChangesTheme Color Palette and Structure Updates
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coryrylan
force-pushed
the
topic-contrast-fixes
branch
2 times, most recently
from
f1d42c6 to
8ed218a
Compare
![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
🤖 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 `@projects/core/src/button-group/button-group.css`:
- Around line 43-45: The CSS rule for :host([container='flat']) sets
--background to the keyword initial which prevents var(--background, ...)
fallbacks and breaks color-mix usage; change the declaration in that selector to
set --background: transparent so the variable resolves to a valid color and
allows the fallback (and color-mix calls that compute
--nve-sys-interaction-state-base) to work correctly across tree-node, tabs-item,
steps-item and interaction-state consumers.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 650cd4b3-a251-405c-a0db-983561c83c58
⛔ Files ignored due to path filters (3)
projects/core/.visual/card.dark.pngis excluded by!**/*.pngprojects/core/.visual/textarea.dark.pngis excluded by!**/*.pngprojects/core/.visual/textarea.pngis excluded by!**/*.png
📒 Files selected for processing (16)
projects/core/src/accordion/accordion.cssprojects/core/src/button-group/button-group.cssprojects/core/src/button-group/button-group.global.cssprojects/core/src/button/button.cssprojects/core/src/card/card.cssprojects/core/src/chat-message/chat-message.cssprojects/core/src/checkbox/checkbox.cssprojects/core/src/copy-button/copy-button.cssprojects/core/src/grid/grid.cssprojects/core/src/page/page.cssprojects/core/src/progressive-filter-chip/progressive-filter-chip.global.cssprojects/core/src/radio/radio.cssprojects/core/src/steps/steps-item.cssprojects/core/src/toolbar/toolbar.cssprojects/themes/src/dark.jsonprojects/themes/src/index.json
coryrylan
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@projects/core/src/button-group/button-group.global.css`:
- Line 41: The pressed-state rule in button-group is setting --background:
initial which breaks interaction-state color calculations; change the
pressed-state to either remove the --background override or set it to a concrete
color expression (for example use var(--nve-sys-layer-canvas-background) or
another explicit CSS color) so that --nve-sys-interaction-state-base and the
color-mix in interaction-state.css receive a valid color value instead of
"initial".
In
`@projects/core/src/progressive-filter-chip/progressive-filter-chip.global.css`:
- Line 6: Replace the reset keyword assignment for the component state by
assigning a design token to --background instead of using "initial": in the
:host([status='danger']) rule (where --background is currently set to initial),
set --background to the appropriate danger token (e.g.
var(--nve-status-danger-100) or the project’s equivalent) so the color-mix()
computation in :host can resolve correctly and var(--background) always has a
valid value.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d954536c-6d4a-4fb6-86bf-821f9057100e
⛔ Files ignored due to path filters (3)
projects/core/.visual/card.dark.pngis excluded by!**/*.pngprojects/core/.visual/textarea.dark.pngis excluded by!**/*.pngprojects/core/.visual/textarea.pngis excluded by!**/*.png
📒 Files selected for processing (16)
projects/core/src/accordion/accordion.cssprojects/core/src/button-group/button-group.cssprojects/core/src/button-group/button-group.global.cssprojects/core/src/button/button.cssprojects/core/src/card/card.cssprojects/core/src/chat-message/chat-message.cssprojects/core/src/checkbox/checkbox.cssprojects/core/src/copy-button/copy-button.cssprojects/core/src/grid/grid.cssprojects/core/src/page/page.cssprojects/core/src/progressive-filter-chip/progressive-filter-chip.global.cssprojects/core/src/radio/radio.cssprojects/core/src/steps/steps-item.cssprojects/core/src/toolbar/toolbar.cssprojects/themes/src/dark.jsonprojects/themes/src/index.json
coryrylan
force-pushed
the
topic-contrast-fixes
branch
from
8ed218a to
90ad264
Compare
coryrylan
changed the title
coryrylan
force-pushed
the
topic-contrast-fixes
branch
6 times, most recently
from
24118eb to
6def0f5
Compare
coryrylan
commented
May 18, 2026
| "type": "color" | ||
| }, | ||
| "800": { | ||
| "value": "oklch(71.6% 0.113 160.3)", |
Collaborator
Author
There was a problem hiding this comment.
Rounding colors as most don't need to go to such precision (from conversion math of figma not supporting oklch) This also fixes a handful of colors where the math was incorrect and the color was falling out of the supported oklch color range resulting in incorrect hue values. Most changes have no perceived visual change due to the rounding. This is why there are few visual test changes.
The simplified rounding/numbers is a minor refactor/first step that will allow improvements to the color ramp and leverage newer css color mix features to compute the values more efficiently.
coryrylan
force-pushed
the
topic-contrast-fixes
branch
from
6def0f5 to
bf9b832
Compare
coryrylan
commented
May 18, 2026
| <label>•︎•︎•︎•︎•︎•︎</label> | ||
| <textarea rows="15" cols="40"></textarea> | ||
| <nve-control-message>•︎•︎•︎•︎•︎•︎</nve-control-message> | ||
| </nve-textarea> |
Collaborator
Author
There was a problem hiding this comment.
The row/cols and resize styles were causing flakey tests due to platform differences
johnyanarella
approved these changes
May 18, 2026
coryrylan
force-pushed
the
topic-contrast-fixes
branch
from
bf9b832 to
674aa18
Compare
- rounds values for future refactor cleanup with color mix options - fixes a few cases of invalid oklch range values Signed-off-by: Cory Rylan <crylan@nvidia.com>
Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-contrast-fixes
branch
from
674aa18 to
771fe5e
Compare
Collaborator
Author
|
🎉 This issue has been resolved in version 0.0.11 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.1.2 🎉 |
Collaborator
Author
|
🎉 This issue has been resolved in version 0.2.0 🎉 |
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 by CodeRabbit