feat(module): support primary: 'neutral' in app config#6608
feat(module): support primary: 'neutral' in app config#6608benjamincanac wants to merge 5 commits into
primary: 'neutral' in app config#6608Conversation
Allow any color alias (e.g. `primary`) to be set to `'neutral'`, rendering it with the `neutral` variant rather than a recolored numeric scale. - tv: remap a color-bearing variant key to `neutral` when its alias resolves to `'neutral'`, falling back to the resolved `defaultVariants` so the default color is covered too. The `neutral` variant uses semantic tokens (`bg-inverted`, `bg-elevated`, …) a CSS-variable swap can't reproduce. - colors: a non-neutral alias set to `neutral` mirrors the resolved neutral scale, and its `--ui-*` alias points at `--ui-bg-inverted` (high contrast) instead of shade 500/400. - templates: surface `'neutral'` in the `Color` type for autocomplete. - docs: replace the `blackAsPrimary` hack with a real `primary: 'neutral'`.
📝 WalkthroughWalkthroughThe PR replaces the legacy Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 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 `@src/runtime/plugins/colors.ts`:
- Around line 46-49: The condition `value === 'neutral'` in the
Object.entries(colors).map call on line 46 is being applied to all keys
including when key equals 'neutral', which contradicts the behavior of
generateShades() and can mis-map the --ui-neutral variable. Update the ternary
condition to additionally check that the key is not 'neutral' by changing the
condition to value === 'neutral' && key !== 'neutral' so that the neutral
remapping only applies to non-neutral keys.
In `@test/components/primary-neutral.spec.ts`:
- Around line 11-15: The afterEach hook in the test suite hardcodes the
colors.primary value to 'green' instead of restoring the original baseline
value, creating an order dependency issue if the config changes. Store the
original colors.primary value before any test modifications (either at the top
of the describe block or in a beforeEach hook), and then restore that stored
original value in the afterEach hook instead of the hardcoded 'green' string.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ac5cbcb-295a-409d-83c6-cbab8526a490
📒 Files selected for processing (10)
docs/app/components/theme-picker/ThemePicker.vuedocs/app/composables/useTheme.tsdocs/app/plugins/theme.tsdocs/content/docs/1.getting-started/5.theme/1.design-system.mddocs/content/docs/1.getting-started/5.theme/2.css-variables.mddocs/content/index.ymlsrc/runtime/plugins/colors.tssrc/runtime/utils/tv.tssrc/templates.tstest/components/primary-neutral.spec.ts
| ${Object.entries(colors).map(([key, value]: [string, string]) => value === 'neutral' ? `--ui-${key}: var(--ui-bg-inverted);` : generateColor(key, 500)).join('\n ')} | ||
| } | ||
| .dark { | ||
| ${Object.keys(colors).map(key => generateColor(key, 400)).join('\n ')} | ||
| ${Object.entries(colors).filter(([, value]) => value !== 'neutral').map(([key]) => generateColor(key, 400)).join('\n ')} |
There was a problem hiding this comment.
Guard neutral remapping to non-neutral keys only.
Line 46 and Line 49 currently apply the value === 'neutral' branch to every key, including key === 'neutral'. That contradicts the non-neutral alias behavior implemented in generateShades() and can mis-map --ui-neutral when ui.colors.neutral = 'neutral'.
🔧 Proposed fix
- ${Object.entries(colors).map(([key, value]: [string, string]) => value === 'neutral' ? `--ui-${key}: var(--ui-bg-inverted);` : generateColor(key, 500)).join('\n ')}
+ ${Object.entries(colors).map(([key, value]: [string, string]) => (value === 'neutral' && key !== 'neutral') ? `--ui-${key}: var(--ui-bg-inverted);` : generateColor(key, 500)).join('\n ')}
}
.dark {
- ${Object.entries(colors).filter(([, value]) => value !== 'neutral').map(([key]) => generateColor(key, 400)).join('\n ')}
+ ${Object.entries(colors).filter(([key, value]) => key === 'neutral' || value !== 'neutral').map(([key]) => generateColor(key, 400)).join('\n ')}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ${Object.entries(colors).map(([key, value]: [string, string]) => value === 'neutral' ? `--ui-${key}: var(--ui-bg-inverted);` : generateColor(key, 500)).join('\n ')} | |
| } | |
| .dark { | |
| ${Object.keys(colors).map(key => generateColor(key, 400)).join('\n ')} | |
| ${Object.entries(colors).filter(([, value]) => value !== 'neutral').map(([key]) => generateColor(key, 400)).join('\n ')} | |
| ${Object.entries(colors).map(([key, value]: [string, string]) => (value === 'neutral' && key !== 'neutral') ? `--ui-${key}: var(--ui-bg-inverted);` : generateColor(key, 500)).join('\n ')} | |
| } | |
| .dark { | |
| ${Object.entries(colors).filter(([key, value]) => key === 'neutral' || value !== 'neutral').map(([key]) => generateColor(key, 400)).join('\n ')} |
🤖 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/runtime/plugins/colors.ts` around lines 46 - 49, The condition `value ===
'neutral'` in the Object.entries(colors).map call on line 46 is being applied to
all keys including when key equals 'neutral', which contradicts the behavior of
generateShades() and can mis-map the --ui-neutral variable. Update the ternary
condition to additionally check that the key is not 'neutral' by changing the
condition to value === 'neutral' && key !== 'neutral' so that the neutral
remapping only applies to non-neutral keys.
commit: |
- Remap loadingColor/spotlightColor too, so a neutral alias stays correct if their neutral variant ever diverges from the CSS-variable swap - Point --ui-primary at --ui-bg-inverted in the docs no-flash script when primary is neutral, matching the runtime colors plugin - Restore the captured baseline primary in the test afterEach
benjamincanac
changed the title
primary: 'neutral' in app configprimary: 'neutral' in app config
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/app/plugins/theme.ts (1)
125-125: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep a one-time migration for
nuxt-ui-black-as-primary.After Line 125, this plugin no longer reads the legacy
nuxt-ui-black-as-primarykey anywhere. The provideddocs/app/composables/useTheme.tssnippet only migratessettings.blackAsPrimaryinside the saved theme blob, so returning docs users who still have only the standalone key will silently lose their persisted monochrome primary until they open the picker again.Suggested migration
if (import.meta.client) { const primary = localStorage.getItem('nuxt-ui-primary') - if (primary) appConfig.ui.colors.primary = primary + const legacyBlackAsPrimary = localStorage.getItem('nuxt-ui-black-as-primary') + if (primary) appConfig.ui.colors.primary = primary + else if (legacyBlackAsPrimary) appConfig.ui.colors.primary = 'neutral' ... } // no-flash script - var primaryColor = localStorage.getItem('nuxt-ui-primary'); + var primaryColor = localStorage.getItem('nuxt-ui-primary'); + if (!primaryColor && localStorage.getItem('nuxt-ui-black-as-primary')) primaryColor = 'neutral';🤖 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 `@docs/app/plugins/theme.ts` at line 125, The theme plugin is dropping the legacy standalone nuxt-ui-black-as-primary setting, so add a one-time migration path in the theme initialization logic in theme.ts alongside the existing useTheme migration. Read the legacy key if it exists, map it into the current blackAsPrimary state, and then clear or mark it migrated so existing docs users keep their persisted monochrome primary without needing to reopen the picker. Use the existing theme setup flow and related symbols in theme.ts and useTheme.ts to keep both migrations aligned.
🤖 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.
Outside diff comments:
In `@docs/app/plugins/theme.ts`:
- Line 125: The theme plugin is dropping the legacy standalone
nuxt-ui-black-as-primary setting, so add a one-time migration path in the theme
initialization logic in theme.ts alongside the existing useTheme migration. Read
the legacy key if it exists, map it into the current blackAsPrimary state, and
then clear or mark it migrated so existing docs users keep their persisted
monochrome primary without needing to reopen the picker. Use the existing theme
setup flow and related symbols in theme.ts and useTheme.ts to keep both
migrations aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a4464d1-7725-4cec-bdee-c2eb36cebddb
📒 Files selected for processing (2)
docs/app/plugins/theme.tssrc/runtime/utils/tv.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/runtime/utils/tv.ts
|
I'm on the phone so I cannot test something, but atm I both agree with coderabbit's review on the plugin's color which we need to make sure to not spill any key, as well as I'm concerned about having vue's reactivity within the tv utility. Which now should be treated like a composable within vue lifecycle/script setup (becoming a breaking change). What caused the requirement of computed and reactive in tv? |
2 participants
🔗 Linked issue
❓ Type of change
📚 Description
You can now set any color alias to
neutralin your config (e.g.primary: 'neutral'), and that alias renders with theneutralvariant rather than a recolored numeric scale. This gives a coherent monochrome primary across every variant (solid, soft, outline, ghost, hover, ring), and replaces the docs' old--ui-primary: blackrecipe which only overrode the solid alias and left the rest of the scale on the previous color.neutralis treated as a first-class color value, consistent withcolor="neutral"already working on every component (and with shadcn's neutral-as-primary default / HeroUI's nameddefault). It is distinct fromprimary: 'zinc', which stays a mid-tone grey accent.How it works
The
neutralcolor is a distinct class set built from semantic tokens (bg-inverted,bg-elevated,text-default,ring-accented) that a CSS-variable swap can't reproduce, so the substitution happens at thetv()boundary — the one place every color source converges (explicit props, form-field colors, per-item colors, and the resolved default).src/runtime/utils/tv.ts— remaps thecolor/highlightColorvariant key toneutralwhen its alias resolves to'neutral', falling back to the component's resolveddefaultVariantsso the default color is covered too.src/runtime/plugins/colors.ts— a non-neutral alias set toneutralmirrors the resolved neutral scale, and its--ui-*alias points at--ui-bg-inverted(high contrast) instead of shade500/400.src/templates.ts— surfaces'neutral'in theColortype for autocomplete.primary: 'neutral'; design-system and css-variables docs updated.Added
test/components/primary-neutral.spec.ts; full test suite, typecheck and lint pass.📝 Checklist