Preparations for CSS-package #2295

eirikbacker · 2024-08-20T11:03:01Z

After meeting with @Barsnes @mimarz @Thunear @eirikbacker 26.08.24:

We use nesting
We use data-[property]="value" instead of class names for variants
We use aria- and pseudo selectors when relevant
We avoid position: relative and change of display unless necessary
We try to use a single class name for components if DOM structure allows it
We try to use css custom properties --dsc-link-color--hover for state change instead changing of css properties directly
We move to CSS modules composes for font setup when feat: css modules #2318 is merged, but use font-size with comment in the meantime
We move to CSS modules composes for focus setup when feat: css modules #2318 is merged, but use --ds-focus-boxShadow in the meantime
We use --dsc-button-background--hover with double dashes and state in the end for state based css custom properties
We use pseudo elements with mask: url(svg...) instead of inline SVGs when graphic is part of the component
We add default styling to base css classname, instead of requiring i.e. data-size="md" being set on every DOM element

To be done after changes

Automatically sort/order CSS properies #2329

Previous discussion topics

See previous discussion topics

### Description

Starting an issue to gather toughs:

Decrease amount of class names for each tag:

Current implementation: Many class names are needed on the same HTML element, for example <button class="ds-btn ds-paragraph ds-paragraph--md ds-line-height--sm ds-focus ds-btn--md ds-btn--primary ds-btn--accent">.
Suggested implementation: Add one class name: ds.btn (imported as CSS module)
Discussion:
- Defaults: The default size ds-btn-md - should this styling just be part of ds-btn? My feeling is yes: If a consumer wants to create a "variation" of the default or md, they can easily extend with their own class names, rather than needing to add multiple styling props to every element.
- Class vs. data-attributes: See Use data attributes for css #2065
- States: I suggest we style state based on attributes and pseudo selectors whenever possible, to avoid the need for adding and removing class names/data-attributes (i.e. .ds-btn:disabled) instead of ds-btn--disabled).

Decrease amount of class names for each component:

Current implementation: Class names are needed on multiple elements, for example:

<table class="ds-paragraph ds-paragraph--md ds-line-height--md ds-table ds-table--md ds-paragraph--md">
    <thead class="ds-table__head">
        <tr class="ds-table__row">
            <th class="ds-table__header__cell ds-font-weight--medium">Header 1</th>
        </tr>
    </thead>
    <tbody>
        <tr class="ds-table__row">
            <td class="ds-table__cell">Content</td>
        </tr>
    </tbody>
</table>

Suggested implementation: When DOM structure is dictated by HTML standards, adding multiple class names should not be necessary - we can instead add child selectors, allowing markup with pure tags:

<table class="ds-table">
    <thead>
        <tr><th>Header 1</th></tr>
    </thead>
    <tbody>
        <tr><td>Content</td></tr>
    </tbody>
</table>

Discussion:
- Nesting: This means we should write .ds-table > thead > tr > th to ensure we only style the current table, and not all child th elements regardless of nesting level.
- Non-DOM-standards: There will be scenarios where the DOM specification is less strict - should we follow the current BEM pattern then? If moving to CSS modules, class names with hyphens are a bit pain (as this requires the syntac styles['table--zebra']), but underscore is okay (styles.table__expander).

Decrease amount of DOM:

Current implementation: Some components have redundant amount of HTML elements, example:

<button type="button" aria-pressed="false" class="ds-focus ds-chip--button ds-chip--md">
  <span class="ds-paragraph ds-paragraph--md ds-line-height--sm ds-chip__label">
    <span>Nynorsk</span>
  </span>
</button>

Suggested implementation: Strive to only require essential HTML elements - both for simpler and faster DOM, and for more 1:1 interface between React tags and HTML-tags:

<button type="button" class="ds-chip">Nynorsk</button>

Discussion:
- Typography: Moving to this technique, we need to avoid using <Paragraph asChild> as extensively as we do now, and rather ensure this styling is part of the CSS instead. With good use of font-tokens, this can be advantageous as each CSS file is a more concrete listing of all applied properties, but also increases file size a tiny bit.

Avoid `display: flex` OUTDATED, SEE COMMENT

Current implementation: Some components use display: flex or display: inline-flex around text elements. This makes it easy to add gap, but can have unwanted side effects if adding textual child elements, as flex causes gap and block rendering for all all child elements, example:

<Button><Star /> Text is <em>special</em></Button>

..will cause rendering: ⭐️ Text is *special*

Suggested implementation: Avoid display: flex and rather use display: inline-block + vertical-align and margin-left or margin-right based on :first-child or :last-child. This is a bit more CSS, but makes the implementation more robust.
Discussion:
- Alternative: We can keep flex, but require text to wrapped: <Button><Star /> <span>Text is <em>special</em></span></Button>. I would not recommend this.

Avoid `position: relative` et.al.

Current implementation: Some components use position: relative without any obvious reason or comment about why. - Suggested implementation: We should avoid causing new stacking contexts whenever possible for a predictable styling environment, and comment/document places where position (or other stacking context properties) are used.

Use `::before` or `::after` with `mask: url(svg)` for component graphics

Current implementation: Some components use SVG elements for component graphics such as the chevron in Accordion, example:

<button type="button" class="ds-accordion__button ds-focus" aria-expanded="false" aria-controls=":r0:">
  <svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" fill="none" viewBox="0 0 24 24" focusable="false" role="img" aria-hidden="true" class="ds-accordion__expand-icon" font-size="1.5rem"><path fill="currentColor" fill-rule="evenodd" d="M5.97 9.47a.75.75 0 0 1 1.06 0L12 14.44l4.97-4.97a.75.75 0 1 1 1.06 1.06l-5.5 5.5a.75.75 0 0 1-1.06 0l-5.5-5.5a.75.75 0 0 1 0-1.06" clip-rule="evenodd"></path></svg>
  <span class="ds-paragraph ds-paragraph--sm ds-line-height--md">Hvem kan registrere seg i Frivillighetsregisteret?</span>
</button>

Suggested implementation: We should avoid requiring our consumers to add these SVG elements manually, and instead render them with ::before or ::after elements.
Discussion:
- Component graphics vs. icons: Icons that are not part of the component, but rather a added element (such as icons in Buttons) should still be SVG elements - only component graphics should be CSS inlined. But there will be edge cases such as the icon in Alerts - is this a component graphic, or not?

Direct styling of input elements

Current implementation: Input elements are styled by adding a wrapping element around both label and input, where the outermost element is the component itself, example:

<div class="ds-paragraph ds-paragraph--md ds-line-height--md ds-textfield ds-textfield--md">
  <label class="ds-label ds-label--md ds-font-weight--medium ds-textfield__label" for="textfield-:rga:">
    <span>Label</span>
  </label>
  <div class="ds-textfield__field">
    <input class="ds-textfield__input ds-focus" type="text" size="20" id="textfield-:rga:">
  </div>
  <div class="ds-textfield__error-message" id="textfield-error-:rga:" aria-live="polite" aria-relevant="additions removals"></div>
</div>

Suggested implementation: We can style the input element directly with a class name ds-input, and rather separate out the wrapper styling (if needed) as a own class ds-form-field so it is more modular/composable, works more closely with HTML-element names, and so that the wrapper can be re-used for Select, Datepicker, Combobox, Textarea, Textfield, or even custom form controls our consumers create. If prefix/suffix is needed, this can be an optional wrapper-element - just like the Danish Designsystem is solving it: https://designsystem.dk/komponenter/inputfelter/

Spacing as responsibility of parent element

Current implementation: Some elements have props to set margin/spacing
Suggested implementation: Instead of elements (especially typographic elements) having outer margin, we should outsource spacing to the parent element. See more on https://kyleshevlin.com/no-outer-margin/

Tasks

Give feedback

Conversion to this format have been done on:

Avatar feat(Avatar): new component #2312
Button, Divider, Link fix: start implementing css principles #2313
List fix(List): css principles #2348
Accordion: fix(Accordion): implement css adjustments and native details #2363

Create a PR with changes and just use non keyword to link to this issue #2295.

The text was updated successfully, but these errors were encountered:

Barsnes · 2024-08-20T11:26:26Z

Some quick thoughs:

Decrease amount of DOM:

In the chip you have used as an example here, the wrappers are for icons, and to ensure children are in a singular span, since users will most likely send in pure text here. How would you solve this otherwise? We should be able to drop the outher chip__label span, but I think we need the one around children.

Avoid display: flex

I like the alternative, since using selectors like :first-child and :last-child will have more side effects, such as targeting wrong elements, if there are more than 3 elements.

Avoid position: relative et.al.

Agreed, we need it most places where it is set, but I see some examples where this should not be needed.

Use ::before or ::after with mask: url(svg) for component graphics

Agreed, this has been on our radar, but have fallen through the cracks. We have already done it for some elements, such as NativeSelect.

Direct styling of input elements

This is done to the input so we can adde prefixes and suffixes, not sure how you would solve this without a wrapper?

Spacing as responsibility of parent element

This will unfortunatley not work in some contexts, where users will need to add it manually themselves. I suggest having a parent element as an alternative, but not the only option.

eirikbacker · 2024-08-20T11:32:27Z

Decrease amount of DOM:
In the chip you have used as an example here, the wrappers are for icons, and to ensure children are in a singular span, since users will most likely send in pure text here. How would you solve this otherwise? We should be able to drop the outher chip__label span, but I think we need the one around children.

But could we then solve icons the same way for buttons and chips? See next comment:

Avoid display: flex
I like the alternative, since using selectors like :first-child and :last-child will have more side effects, such as targeting wrong elements, if there are more than 3 elements.

Not necessarily - .ds-btn > svg:first-child is a pretty safe selector? :)

Direct styling of input elements
This is done to the input so we can adde prefixes and suffixes, not sure how you would solve this without a wrapper?

Right.. is this much used? We should brainstorm a bit on this 🙌

Spacing as responsibility of parent element
This will unfortunatley not work in some contexts, where users will need to add it manually themselves. I suggest having a parent element as an alternative, but not the only option.

Could you elaborate on what contexts? :)

Barsnes · 2024-08-20T11:47:17Z

Not necessarily - .ds-btn > svg:first-child is a pretty safe selector? :)

We would in that case need to add

.ds-button:not(.ds-button-only-icon) svg:first-child { ... }
.ds-button:not(.ds-button-only-icon) svg:last-child { ... }

Luckly I don't think there are other elments this applies to, but that's the thing, do we 100% know that we have crossed all the boxes with this? 🤔

Right.. is this much used? We should brainstorm a bit on this 🙌

I believe this is used yes, but I also believe this comes from the old design system it was built on.

Could you elaborate on what contexts? :)

To word myself a bit differently, why should we lock someone into always having to use a wrapper around their text content, when the components themselves could solve it? Why would I need a wrapper if I just have a section with a heading and a paragraph, which would not need a wrapper to begin with.
Having it as an alternative is nice, but the only solution seems like an oversight.

eirikbacker · 2024-08-20T12:01:26Z

Do we need a class ds-button-only-icon? Hm, yeah, it is a bit unfortunate that :only-child does not detect text-content.. hm, we need to think a bit about this one 😊 Regardless, flex on elements with text should be avoided, as this also will at some point break, so we need to just land a policy either way 💪
Lets have a chat about prefix/suffix - lots of design systems have this, but in my experience, the elements visually "inside" the input is not necessarily a good solution - also in regards of accessibility. It is better to provide a good label and optional description. We should include design on this one ✅
Regarding spacing; you would probably always have an element around any element. Letting this parent have ds-stack or similar, makes the components themselves never dictate outer spacing, meaning you can add and remove elements to the stack, without needing to add or remove props based on position/index of child elements. Creating a clear separation of concerns :)
This can be elaborated in https://kyleshevlin.com/no-outer-margin/, https://ishadeed.com/article/spacing-in-css/, https://dev.to/moruno21/dont-use-margin-in-css-33f9, https://www.youtube.com/watch?v=RuRfcjzEv9w, and https://www.smashingmagazine.com/2019/07/margins-in-css/, https://stackoverflow.com/a/49205929

eirikbacker · 2024-08-20T12:12:24Z

PS: looks like the danish designsystem does not require a wrapper, unless using prefix/suffix: https://designsystem.dk/komponenter/inputfelter/#prefix-og-suffix-kode

Barsnes · 2024-08-20T12:19:09Z

Regarding spacing; you would probably always have an element around any element. Letting this parent have ds-stack or similar, makes the components themselves never dictate outer spacing, meaning you can add and remove elements to the stack, without needing to add or remove props based on position/index of child elements. Creating a clear separation of concerns :)
This can be elaborated in https://kyleshevlin.com/no-outer-margin/, https://ishadeed.com/article/spacing-in-css/, https://dev.to/moruno21/dont-use-margin-in-css-33f9, https://www.youtube.com/watch?v=RuRfcjzEv9w, and https://www.smashingmagazine.com/2019/07/margins-in-css/, https://stackoverflow.com/a/49205929

Reading thourhg some of these, I would suggest dropping the spacing prop, and not giving them a "stack" component either - as suggested in #2258.
Layout is specific to each website and app, and if we make the component customizable with props such as gap, they might as well create a component that suits them.

eirikbacker · 2024-08-20T12:23:08Z

Agreed 🙌 Stacking/gap is regardless maybe more domain specific, and quite easy to set up. If we want to provide a out-of-the-box-solution for this, we can rather launch this later ☺️

Barsnes · 2024-08-20T12:33:59Z

Agreed 🙌 Stacking/gap is regardless maybe more domain specific, and quite easy to set up. If we want to provide a out-of-the-box-solution for this, we can rather launch this later ☺️

I would rather suggest easy to copy examples as the way to go :) Gives us less to worry about, but we still give a solution to consumers

eirikbacker · 2024-08-21T09:47:10Z

Okay, so Radix, Tailwind, Chakra, Carbon, Shadcn, Ant and MUI all uses flex or inline-flex on buttons, so I guess we can say for sure this is a established convention 😅 Lets document that our consumers should wrap content in a <span> if they need several text formattings in a button / chip etc, but there is no need for extra HTML elements when using plain text :)

mimarz · 2024-09-04T13:17:48Z

Updated with tasklist for todo components

part of #2295

Barsnes · 2024-09-09T06:54:06Z

We need to do this change for typography components last, since their attrs will interfere with the components they are being merged onto.

<Paragraph asChild variant='short' size='sm'>
  <div data-size='md'>
    ...
  </div>
</Paragraph>

In the case above, size would become medium

part of #2295 --------- Co-authored-by: Eirik Backer <eirik.backer@gmail.com>

- Part of #2295 - Wraps pagination for now, but should be looked more into #2396 - Skipping #2283 for now to avoid too large PR - Renames `Pagination.Content` to `Pagination.List` following #2221 - Fixes so `Button` with `hidden` attribute is actually hidden ☺️

part of #2295

part of #2295 --------- Co-authored-by: Eirik Backer <eirik.backer@gmail.com>

- Fixes #2100 🥳 - Deprecates #2176, #2190 - Built on top of `<u-details>` for [better accessibility on mobile](https://u-elements.github.io/u-elements/elements/u-details#accessibility) - Removes `level` from `AccordionHeader` as this is not supported by native `<details>` - Removes `onHeaderClick` from `AccordionHeader` as this is identical to adding a `onClick` handler - JS-based animation can be removed and replaced by CSS when `calc-size(auto)` is fully supported 🚀 - Fixes shrinking chevron on mobile/zoom - Fixes text-align in AccordionHeader on mobile/zoom - Follows: #2295 - Also works in `dir="rtl"` Question: It is now implemented so search-in-page only works when using `defaultOpen`, as a controlled `open` should not be affected by user interaction. Just checking - does this make sense to you guys as well? :)

part of #2295 I am not renaming anything is this PR. That belongs in it's own PR

Part of #2295 and #2221

part of #2295

Part of #2295 and #2221

- Part of #2295 --------- Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>

Part of #2295 and #2221 Moves back link into `nav` as this make sense also from a11y perspective after discussion with NRK --------- Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>

Part of #2295

Part of #2295 --------- Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>

- Part of #2295 - Awaiting doing `Paragraph` as we should convert to using `@composes` in other componentsbefore using data-attributes on paragraph

Barsnes · 2024-09-27T06:14:40Z

Closing this as completed.
Components have been updated.

eirikbacker added the css @digdir/designsystemet-css label Aug 20, 2024

eirikbacker self-assigned this Aug 20, 2024

mimarz added the ☂️ epic Issues ready label Aug 20, 2024

Barsnes mentioned this issue Aug 21, 2024

How should we support typography stacking? (Primitive component) #2258

Closed

Barsnes mentioned this issue Aug 23, 2024

fix: start implementing css principles #2313

Merged

mimarz added this to the V1 milestone Aug 27, 2024

eirikbacker mentioned this issue Aug 29, 2024

fix(List): css principles #2348

Merged

mrosvik modified the milestones: V1 - Helhetlig fargesystem, V1 - Helhetlig komponentbibliotek Aug 29, 2024

Febakke mentioned this issue Aug 30, 2024

Accessibility test on all components #2355

Open

70 tasks

This was referenced Sep 2, 2024

Loading button should not look disabled #1992

Closed

fix(Accordion): implement css adjustments and native details #2363

Merged

This was referenced Sep 6, 2024

feat(dropdownmenu): css changes and data attrs #2387

Merged

feat(tooltip): remove css variables #2389

Merged

feat(spinner): css changes and data attrs #2390

Merged

Barsnes added a commit that referenced this issue Sep 6, 2024

feat(tooltip): remove css variables (#2389)

0dbcee5

part of #2295

Barsnes mentioned this issue Sep 9, 2024

feat(badge): css changes and data attrs #2391

Merged

This was referenced Sep 9, 2024

fix(Table): use css conventions #2393

Merged

fix(Pagination): start converting to new css principles #2395

Merged

mimarz pushed a commit that referenced this issue Sep 13, 2024

feat(badge): css changes and data attrs (#2391)

a0b119d

part of #2295 --------- Co-authored-by: Eirik Backer <eirik.backer@gmail.com>

Barsnes added a commit that referenced this issue Sep 16, 2024

feat(dropdownmenu): css changes and data attrs (#2387)

722fbe4

part of #2295

This was referenced Sep 16, 2024

Upgrade Biome to v1.9.x #2411

Closed

feat(modal): css changes #2418

Merged

Barsnes added a commit that referenced this issue Sep 16, 2024

feat(card): css changes and data attrs (#2398)

54d3037

part of #2295 --------- Co-authored-by: Eirik Backer <eirik.backer@gmail.com>

eirikbacker mentioned this issue Sep 16, 2024

fix(ToggleGroup): align with new css and api principles #2424

Merged

eirikbacker mentioned this issue Sep 17, 2024

fix(Breadcrumbs): css and api alignment #2428

Merged

Barsnes mentioned this issue Sep 17, 2024

feat(tabs): css changes #2431

Merged

eirikbacker mentioned this issue Sep 17, 2024

fix: errormessage, heading, ingress, label css #2433

Merged

Barsnes added a commit that referenced this issue Sep 17, 2024

feat(modal): css changes (#2418)

f71185c

part of #2295 I am not renaming anything is this PR. That belongs in it's own PR

eirikbacker mentioned this issue Sep 17, 2024

fix(Skeleton): css alignment #2435

Merged

eirikbacker added a commit that referenced this issue Sep 17, 2024

fix(ToggleGroup): align with new css and api principles (#2424)

c2b78ed

Part of #2295 and #2221

Barsnes added a commit that referenced this issue Sep 17, 2024

feat(tabs): css changes (#2431)

3451704

part of #2295

eirikbacker mentioned this issue Sep 17, 2024

fix(ErrorSummary): css and api alignments #2437

Merged

eirikbacker added a commit that referenced this issue Sep 17, 2024

fix(ErrorSummary): css and api alignments (#2437)

b8f3153

Part of #2295 and #2221

eirikbacker mentioned this issue Sep 17, 2024

fix(HelpText): css alignments #2438

Merged

eirikbacker added a commit that referenced this issue Sep 17, 2024

fix(Skeleton): css alignment (#2435)

03d776b

- Part of #2295 --------- Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>

eirikbacker mentioned this issue Sep 18, 2024

fix(Fieldset): css alignments #2447

Merged

eirikbacker removed their assignment Sep 18, 2024

eirikbacker added a commit that referenced this issue Sep 18, 2024

fix(Fieldset): css alignments (#2447)

d3229a0

Part of #2295

mimarz mentioned this issue Sep 19, 2024

Modal has wrong padding-bottom when Footer is removed #2063

Closed

eirikbacker added a commit that referenced this issue Sep 20, 2024

fix(HelpText): css alignments (#2438)

0e4faee

Part of #2295 --------- Co-authored-by: Tobias Barsnes <tobias.barsnes@digdir.no>

mimarz mentioned this issue Sep 23, 2024

Remove spacing as property from our Typography components #2491

Open

eirikbacker added a commit that referenced this issue Sep 23, 2024

fix: errormessage, heading, ingress, label css (#2433)

9a14b6a

- Part of #2295 - Awaiting doing `Paragraph` as we should convert to using `@composes` in other componentsbefore using data-attributes on paragraph

mimarz mentioned this issue Sep 25, 2024

Font based sizing / scaling and less useContext? #2508

Open

Barsnes closed this as completed Sep 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preparations for CSS-package #2295

Preparations for CSS-package #2295

eirikbacker commented Aug 20, 2024 •

edited by Barsnes

Loading

Decrease amount of class names for each tag:

Decrease amount of class names for each component:

Decrease amount of DOM:

Avoid `display: flex` OUTDATED, SEE COMMENT

Avoid `position: relative` et.al.

Use `::before` or `::after` with `mask: url(svg)` for component graphics

Direct styling of input elements

Spacing as responsibility of parent element

Tasks

Components

Barsnes commented Aug 20, 2024 •

edited

Loading

eirikbacker commented Aug 20, 2024

Barsnes commented Aug 20, 2024 •

edited

Loading

eirikbacker commented Aug 20, 2024

eirikbacker commented Aug 20, 2024

Barsnes commented Aug 20, 2024

eirikbacker commented Aug 20, 2024 •

edited

Loading

Barsnes commented Aug 20, 2024

eirikbacker commented Aug 21, 2024

mimarz commented Sep 4, 2024

Barsnes commented Sep 9, 2024

Barsnes commented Sep 27, 2024

Preparations for CSS-package #2295

Preparations for CSS-package #2295

Comments

eirikbacker commented Aug 20, 2024 • edited by Barsnes Loading

After meeting with @Barsnes @mimarz @Thunear @eirikbacker 26.08.24:

To be done after changes

Previous discussion topics

Decrease amount of class names for each tag:

Decrease amount of class names for each component:

Decrease amount of DOM:

Avoid display: flex OUTDATED, SEE COMMENT

Avoid position: relative et.al.

Use ::before or ::after with mask: url(svg) for component graphics

Direct styling of input elements

Spacing as responsibility of parent element

Tasks

Conversion to this format have been done on:

Components

Barsnes commented Aug 20, 2024 • edited Loading

Decrease amount of DOM:

Avoid display: flex

Avoid position: relative et.al.

Use ::before or ::after with mask: url(svg) for component graphics

Direct styling of input elements

Spacing as responsibility of parent element

eirikbacker commented Aug 20, 2024

Barsnes commented Aug 20, 2024 • edited Loading

eirikbacker commented Aug 20, 2024

eirikbacker commented Aug 20, 2024

Barsnes commented Aug 20, 2024

eirikbacker commented Aug 20, 2024 • edited Loading

Barsnes commented Aug 20, 2024

eirikbacker commented Aug 21, 2024

mimarz commented Sep 4, 2024

Barsnes commented Sep 9, 2024

Barsnes commented Sep 27, 2024

eirikbacker commented Aug 20, 2024 •

edited by Barsnes

Loading

Avoid `display: flex` OUTDATED, SEE COMMENT

Avoid `position: relative` et.al.

Use `::before` or `::after` with `mask: url(svg)` for component graphics

Barsnes commented Aug 20, 2024 •

edited

Loading

Barsnes commented Aug 20, 2024 •

edited

Loading

eirikbacker commented Aug 20, 2024 •

edited

Loading