Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Darkside] Quality assurance P1 #3510

Merged
merged 9 commits into from
Jan 22, 2025
Merged

[Darkside] Quality assurance P1 #3510

merged 9 commits into from
Jan 22, 2025

Conversation

KenAJoh
Copy link
Collaborator

@KenAJoh KenAJoh commented Jan 21, 2025

Description

We are now in the process of testing each component and verifying the sync between Figma and code

This PR is one of many to more with small adjustments and fixes made to the darkside system. The changes made here was from reviewing Accordion -> ErrorSummary. There are some changes not included here that was found, but will create separate PRs for those because of the scale. So this PR can be merged as is.

Component Checklist 📝

  • JSDoc
  • Examples
  • Documentation / Decision Records
  • Storybook
  • Style mappings (@navikt/core/css/config/_mappings.js)
  • Component tokens (@navikt/core/css/tokens.json)
  • CSS class deprecations (@navikt/aksel-stylelint/src/deprecations.ts)
  • Exports (@navikt/core/react/src/index.ts and @navikt/core/react/package.json)
  • New component? CSS import (@navikt/core/css/index.css)
  • Breaking change? Update migration guide. Consider codemod.
  • Changeset (Format: <Component>: <gitmoji?> <Text>. E.g. "Button: ✨ Add feature xyz.")

@KenAJoh KenAJoh self-assigned this Jan 21, 2025
Copy link

changeset-bot bot commented Jan 21, 2025

⚠️ No Changeset found

Latest commit: 1252325

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment on lines -20 to -21
outline: 2px solid var(--ax-border-focus);
outline-offset: 2px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of a new test for focus-markings. I will create a separate PR for all the other components updating this.

@@ -230,7 +238,7 @@
/* dropdown & non selectable dropdown items */

.navds-combobox__list {
max-height: 290px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This size is a little arbitrary, but helps a little with this issue: #3498. By adjusting the height, you will by default see part of the last item before scrolling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are testing using a little darker "default"-scale for strong. This makes primary-neutral buttons, ToggleGroup, Tag etc more defined and visually equal to the counterparts with "role"

Copy link
Contributor

github-actions bot commented Jan 21, 2025

Storybook demo / Chromatic

📝 Changes to review: 2

3b1b835ab | 91 components | 135 stories

it-vegard
it-vegard previously approved these changes Jan 22, 2025
@KenAJoh KenAJoh enabled auto-merge (squash) January 22, 2025 10:23
@KenAJoh KenAJoh disabled auto-merge January 22, 2025 12:58
@KenAJoh KenAJoh merged commit 7aa85be into main Jan 22, 2025
6 checks passed
@KenAJoh KenAJoh deleted the darkside-QA-r1 branch January 22, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants