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.

Sign up for GitHub

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

Jump to bottom

fix: start implementing css principles #2313

Merged
merged 29 commits into from
Sep 6, 2024
Merged

fix: start implementing css principles #2313

merged 29 commits into from
Sep 6, 2024

Conversation

eirikbacker
Copy link
Contributor

@eirikbacker eirikbacker commented Aug 22, 2024 •
edited by mimarz
Loading

  • ✅ Changes to only Alert, Button, Divider and Link to demonstrate principles :)
  • ✅ Using the pattern dsc-button-background and dsc-button-background--active for recognition of state variables
  • ✅ Using pseudo elements with mask instead of inline <svg> elements for easy reconstruction in other frameworks
  • ✅ Alphabetically sorting CSS properties for consistent setup - or whatever is automated by our formatter <3
  • ✅ Using attributes for setting size, variant or colors - aligning the props interface across frameworks and avoiding invalid states (▶️ Move to data-ds-variant="" aka element.dataset.dsVariant)
  • ✅ Moving to :focus-visible styling to CSS token as well as utility class, using box-shadow only to avoid half pixels
  • ✅ Moving default styling to main class name, as this allows skipping setting data-size="md" on all components
  • ✅ Moving to color instead of severity for Alert as this aligns with other components props API
  • ✅ Rendering Alert icon inside first heading if present as this makes typographic alignment better (worked with Øyvind)
  • ✅ Several TODO/questions added to code we should discuss, and font sizing is being discussed with designers 23.08.24
  • Closes Missing Divider component in Figma - and, should it have variants or not? #2320
  • Closes Loading button should not look disabled #1992
  • Closes Use data attributes for css #2065

@eirikbacker eirikbacker self-assigned this Aug 22, 2024
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 22, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 4a7ae66

The changes in this PR will be included in the next version bump.

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 22, 2024 •
edited
Loading

Preview deployments for this pull request:

📖 Storybook 6. Sep 2024 - 10:17 (Norwegian time)

🖥 Storefront 6. Sep 2024 - 10:19 (Norwegian time)

See all deployments at https://dev.designsystemet.no

Barsnes
Barsnes reviewed Aug 23, 2024
View reviewed changes
packages/css/button.css Outdated Show resolved Hide resolved
@Barsnes
Copy link
Member

Barsnes commented Aug 23, 2024

related to #2065, #2295, #2221

eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/button.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/button.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/button.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/link.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/link.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/link.css Outdated Show resolved Hide resolved
eirikbacker
eirikbacker commented Aug 26, 2024
View reviewed changes
packages/css/alert.css Outdated Show resolved Hide resolved
@mimarz
Copy link
Collaborator

mimarz commented Aug 26, 2024 •
edited
Loading

To summaries our meeting on 26.08.24 with @Barsnes @Thuneer @eirikbacker:

Lots of good improvements we agreed on.

  • Make sure to make separate changesets for each of the changes to the React component props.
  • We'll remove some classes and add them back if needed, such as -md.
  • We will compose typography classes using compose from css-modules in feat: css modules #2318

@Barsnes Barsnes mentioned this pull request Aug 27, 2024
Merged
@eirikbacker eirikbacker mentioned this pull request Aug 27, 2024
Closed
@eirikbacker eirikbacker mentioned this pull request Sep 2, 2024
Closed
@eirikbacker eirikbacker marked this pull request as ready for review September 2, 2024 10:04
Barsnes
Barsnes requested changes Sep 4, 2024
View reviewed changes
Copy link
Member

@Barsnes Barsnes left a comment

Choose a reason for hiding this comment

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

Few comments about discrepancies.
Kept it to major comments for now

packages/css/alert.css Outdated Show resolved Hide resolved
packages/css/button.css Outdated Show resolved Hide resolved
packages/css/utilities.css Show resolved Hide resolved
packages/react/src/components/Alert/Alert.test.tsx Show resolved Hide resolved
packages/react/src/components/Alert/Alert.tsx Show resolved Hide resolved
packages/react/src/components/Button/Button.tsx Show resolved Hide resolved
packages/react/src/components/Button/Button.tsx Outdated Show resolved Hide resolved
packages/react/src/components/Button/Button.tsx Outdated Show resolved Hide resolved
packages/react/src/components/Divider/Divider.tsx Show resolved Hide resolved
packages/css/link.css Show resolved Hide resolved
@eirikbacker eirikbacker requested review from Barsnes and removed request for Thunear September 4, 2024 14:50
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 4, 2024 •
edited
Loading

Preview deployments

Theme 6. Sep 2024 - 10:18 (Norwegian time)

Barsnes
Barsnes reviewed Sep 5, 2024
View reviewed changes
Copy link
Member

@Barsnes Barsnes left a comment

Choose a reason for hiding this comment

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

We probably need to omit loading in ToggleGroupItem.
Since you already updated the Button docs, could you also remove the duplicate info for icons 😅
image

packages/css/button.css Outdated Show resolved Hide resolved
packages/css/button.css Outdated Show resolved Hide resolved
mimarz
mimarz approved these changes Sep 5, 2024
View reviewed changes
Copy link
Collaborator

@mimarz mimarz left a comment

Choose a reason for hiding this comment

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

I don't have much to add to what @Barsnes already pointed out.

Good job 🤝

@eirikbacker @Barsnes
Update packages/css/button.css
ef72733 
Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>
@Barsnes
Copy link
Member

Barsnes commented Sep 6, 2024

We probably need to omit loading in ToggleGroupItem. Since you already updated the Button docs, could you also remove the duplicate info for icons 😅 image

Just realized this was not duplicate, so it's all good 😄
ToggleGroup still needs to be updated

@eirikbacker eirikbacker merged commit f45d853 into next Sep 6, 2024
8 checks passed
@eirikbacker eirikbacker deleted the feat/css-preparations-part-1 branch September 6, 2024 08:27
@github-actions github-actions bot mentioned this pull request Sep 11, 2024
Merged
mimarz pushed a commit that referenced this pull request Sep 17, 2024
@github-actions
chore: new release candidate (next) (#2361)
5bf8c21 
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to next, this PR will
be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`next` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `next`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @digdir/designsystemet-css@1.0.0-next.33

### Patch Changes

- Pagination: Use data attrs instead of class names
([#2395](#2395))

- Badge: Style using css attributes
([#2391](#2391))

- TableHeaderCell: Remove `sortable` prop, `sort` now handles this
([#2393](#2393))

- dropdownmenu: Style using data attributes
([#2387](#2387))

- Chip: Text color is now `accent`
([#2371](#2371))

- List: Remove `List.Root` and `List.Heading`, which changes API
([#2348](#2348))

- Alert, Avatar, Button, Divider, Link: Use data-attributes for variant,
size and color and move icons to CSS
([#2313](#2313))

- Box: Remove component
([#2372](#2372))

- Popover: ([#2369](#2369))

    -   Rename `<Popover.Root>` to `<Popover.Context>`
- use Popover API, allowing `<Popover>` to be used without
`Popover.Context`
    -   Remove `portal` prop

- Tooltip: Only expose background css variable
([#2389](#2389))

- Switch: don't show check when not checked in readonly
([#2377](#2377))

- Select: Rename from `NativeSelect`
([#2404](#2404))

- Accordion: Now uses details and summary HTML elements
([`5d1c5062b526e6829c322ce66c6df08568bb9f63`](5d1c506))

- Spinner: Style using data attributes
([#2390](#2390))

- Avatar: new component
([#2312](#2312))

- Tag: Make neutral default color in CSS
([#2397](#2397))

- Card: Use data attrs
([#2398](#2398))

## @digdir/designsystemet-react@1.0.0-next.33

### Patch Changes

- Pagination: Use data attrs instead of class names
([#2395](#2395))

- Badge: Style using css attributes
([#2391](#2391))

- TableHeaderCell: Remove `sortable` prop, `sort` now handles this
([#2393](#2393))

- dropdownmenu: Style using data attributes
([#2387](#2387))

- List: Remove `List.Root` and `List.Heading`, which changes API
([#2348](#2348))

- Alert, Avatar, Button, Divider, Link: Use data-attributes for variant,
size and color and move icons to CSS
([#2313](#2313))

- Box: Remove component
([#2372](#2372))

- Popover: ([#2369](#2369))

    -   Rename `<Popover.Root>` to `<Popover.Context>`
- use Popover API, allowing `<Popover>` to be used without
`Popover.Context`
    -   Remove `portal` prop

- Select: Rename from `NativeSelect`
([#2404](#2404))

- Table: Set sort button type to prevent form submit
([#2402](#2402))

- Heading: default level is now 2
([#2378](#2378))

- Select: ([#2415](#2415))

    -   Add Select.Option and Select.Optgroup compond components
    -   Remove `multiple` prop

- Accordion: Now uses details and summary HTML elements
([`5d1c5062b526e6829c322ce66c6df08568bb9f63`](5d1c506))

- Spinner: Style using data attributes
([#2390](#2390))

- Avatar: new component
([#2312](#2312))

- Tag: Make neutral default color in CSS
([#2397](#2397))

- Card: Use data attrs
([#2398](#2398))

- Combobox: fix virtual combobox having large gap between items
([#2376](#2376))

## @digdir/designsystemet@1.0.0-next.33



## @digdir/designsystemet-theme@1.0.0-next.33

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mimarz mimarz mimarz approved these changes

@Barsnes Barsnes Barsnes approved these changes

Assignees

@eirikbacker eirikbacker

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@eirikbacker @Barsnes @mimarz