fix(field, fieldset, suggestion): add experimental v2 export and missing variables#4903
fix(field, fieldset, suggestion): add experimental v2 export and missing variables#4903mimarz wants to merge 12 commits into
Conversation
🦋 Changeset detectedLatest commit: b19cecf The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 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)
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 |
|
Preview deployments for this pull request: storybook - |
There was a problem hiding this comment.
Pull request overview
This PR expands the CSS customization surface by introducing missing component-level CSS variables (Field, Fieldset, Suggestion), adjusts dropdown heading styling, and adds an experimental “v2” CSS entry point that layers future token mapping overrides on top of the current stylesheet.
Changes:
- Added new component CSS variables and refactored hardcoded values to use them (Field/Fieldset/Suggestion).
- Adjusted dropdown heading styling by removing hardcoded
colorandfont-weight. - Introduced a new
@digdir/designsystemet-css/v2export backed by_v2-index.css+_v2-overrides.css.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/css/src/suggestion.css | Adds a missing checkmark border color CSS variable and uses it in the outline. |
| packages/css/src/fieldset.css | Adds a fieldset description color variable and fixes typos in compatibility comments. |
| packages/css/src/field.css | Adds field description/affix variables and replaces hardcoded values with variables. |
| packages/css/src/dropdown.css | Removes hardcoded heading color/font-weight rules. |
| packages/css/src/_v2-overrides.css | New v2 override mappings from neutral tokens to color-alias tokens. |
| packages/css/src/_v2-index.css | New v2 entrypoint importing the existing index plus overrides. |
| packages/css/package.json | Exposes a new ./v2 export path for consumers. |
| .changeset/petite-falcons-stick.md | Changeset for new fieldset variable. |
| .changeset/nice-queens-enter.md | Changeset for new field variable (needs update to cover all newly added vars). |
| .changeset/neat-rockets-build.md | Changeset for new suggestion variable (contains a stray control character). |
| .changeset/fresh-parks-sneeze.md | Changeset documenting dropdown heading style adjustment. |
Comments suppressed due to low confidence (1)
packages/css/src/dropdown.css:34
- The dropdown heading selector no longer sets
font-weight. Because these are nativeh2–h6elements, UA styles typically applyfont-weight: bolder, so headings may render bold and visually diverge from other dropdown items. Consider explicitly resetting tofont-weight: inherit(or a dedicated dropdown heading variable) so the component styling is consistent across browsers.
& > :is(h2, h3, h4, h5, h6) {
align-items: center;
box-sizing: border-box;
font-size: inherit;
margin: 0;
min-height: var(--dsc-dropdown-item-size);
padding: var(--dsc-dropdown-item-padding);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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 `@packages/css/src/_v2-index.css`:
- Around line 1-2: The two CSS imports in _v2-index.css currently use the
url(...) form which violates the stylelint import-notation rule; update the
import statements to the plain string form (remove url(...) and use quoted
module paths) for both './index.css' and './_v2-overrides.css' so the file
complies with the linter.
In `@packages/css/src/_v2-overrides.css`:
- Around line 87-91: Rename the misspelled selector `.ds-tooltipe` to the
correct `.ds-tooltip` so the v2 overrides apply; in the `_v2-overrides.css`
block that defines `--dsc-tooltip-background` and `--dsc-tooltip-color`, update
the selector name (`.ds-tooltipe` → `.ds-tooltip`) and keep the existing token
assignments intact to ensure the tooltip component picks up the color swaps.
🪄 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: 289619ea-14c1-4972-9b1d-c594cc2a5153
📒 Files selected for processing (11)
.changeset/fresh-parks-sneeze.md.changeset/neat-rockets-build.md.changeset/nice-queens-enter.md.changeset/petite-falcons-stick.mdpackages/css/package.jsonpackages/css/src/_v2-index.csspackages/css/src/_v2-overrides.csspackages/css/src/dropdown.csspackages/css/src/field.csspackages/css/src/fieldset.csspackages/css/src/suggestion.css
💤 Files with no reviewable changes (1)
- packages/css/src/dropdown.css
| @import url('./index.css'); | ||
| @import url('./_v2-overrides.css'); |
There was a problem hiding this comment.
Fix import notation to satisfy stylelint.
These @import statements use url(...), which violates the configured import-notation rule and will fail linting.
Proposed fix
-@import url('./index.css');
-@import url('./_v2-overrides.css');
+@import './index.css';
+@import './_v2-overrides.css';📝 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.
| @import url('./index.css'); | |
| @import url('./_v2-overrides.css'); | |
| `@import` './index.css'; | |
| `@import` './_v2-overrides.css'; |
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 1-1: Expected "url('./index.css')" to be "'./index.css'" (import-notation)
(import-notation)
[error] 2-2: Expected "url('./_v2-overrides.css')" to be "'./_v2-overrides.css'" (import-notation)
(import-notation)
🤖 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 `@packages/css/src/_v2-index.css` around lines 1 - 2, The two CSS imports in
_v2-index.css currently use the url(...) form which violates the stylelint
import-notation rule; update the import statements to the plain string form
(remove url(...) and use quoted module paths) for both './index.css' and
'./_v2-overrides.css' so the file complies with the linter.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Thanks for working on this @mimarz! I can see in the playbook that the changes suggested here adresses most issues highlighted by @Febakke in #4506 With the tabs change (to only use theme color) the "active" state might need a change in visualization. Either with bold as suggested or some other form of visual key. |
Changes related to support for inverted theme #4740
Summary by CodeRabbit
New Features
Bug Fixes