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

Font based sizing #2508

Open
4 of 7 tasks
Tracked by #1045
eirikbacker opened this issue Sep 24, 2024 · 7 comments
Open
4 of 7 tasks
Tracked by #1045

Font based sizing #2508

eirikbacker opened this issue Sep 24, 2024 · 7 comments
Assignees
Labels
☂️ epic Issues ready 🕵️ investigate Needs investigation

Comments

@eirikbacker
Copy link
Contributor

eirikbacker commented Sep 24, 2024

TL;DR: Her er et kjørende eksempel: https://codepen.io/eirikbacker/pen/RwXNpXo

Prerequisite #2526

Tasks

  1. $ tokens
    Febakke mimarz
  2. 10 of 10
    css
    Febakke Thunear
    eirikbacker
  3. css react
    eirikbacker
  4. 💡feature request
  5. react 🎨 theme-builder 📔 storybook 🖥️ storefront
    Barsnes

Har tenkt en stund på dette med size. I de fleste tilfeller, vil vi at når man setter size, på en komponent, så påvirker det også bestanddelene inni.
F.eks; Pagination size="sm" gjør at Pagination.Button også skal ha size="sm", Dropdown size="sm" gjør at Dropdown.Item skal ha size="sm" og så videre.

  • I dag kreves React/Javascript som kjørere i nettleseren (ikke på server) for å få til dette, via useContext APIet som sender size nedover. Jeg tenker det er både kjipt fordi det ikke lar seg oversette til oss som jobber uten React, og det er kjipt fordi det gjør applikasjonene tyngre da det krever med rendering i nettleser.
  • Videre øker mengden kode generelt slik vi har implementert i dag
  • Det hindrer konsumenter fra å lage ordentlig fluid scaling dersom de ønsker å implementere det
  • I dag er er ikke mulig å skalere komponentene baser på skjermstørrelse med CSS; man må faktisk bytte på HTMLen
    Så, hva kan vi gjøre isteden?

La oss ta et eksempel med Pagination:

  1. CSS er skikkelig god på å arve en stil fra forelder og nedover til barna.
  2. Enste utfordringen er da at det må være samme stil, så f.eks setter jeg padding: 5px på Pagination, så er det lett å arve padding: 5pxPagination.Button.
  3. Men, vi vil jo ikke ha padding: 5pxPagination, vi vil ha padding: 0Pagination, men fortstatt ha padding: 5pxPagination.Button - hvordan får vi til det?
  4. Jo, det er én ting vi vil at skal arves nedover: font-size
  5. Det betyr, at hvis padding kan basere seg på font-size, så får vi til noen nydelige padding-skalaer - også på Pagination.Button - som blir større og mindre, når font-size justeres.
  6. Det vil også bety, at når gap skal øke på selve Pagination, så trenger vi ikke å definere gap mange ganger, fordi gap også kan basere seg på font-size.

Dette vil være relevant for gap/padding/margin/width/height - alt som handler om størrelse, kan være skala-basert på font-size

I stede for å skrive slik som i dag:

.ds-button {
  --dsc-button-font-size: var(--ds-font-size-5);
  --dsc-button-gap: var(--ds-spacing-2);
  --dsc-button-padding-block: var(--ds-spacing-2);
  --dsc-button-padding-inline: var(--ds-spacing-4);

  font-size: var(--dsc-button-font-size);
  gap: var(--dsc-button-gap);
  padding: var(--dsc-button-padding-block) var(--dsc-button-padding-inline);

  &[data-size='sm'] {
    --dsc-button-font-size: var(--ds-font-size-4);
    --dsc-button-gap: var(--ds-sizing-1);
    --dsc-button-padding-block: var(--ds-spacing-2);
    --dsc-button-padding-inline: var(--ds-spacing-3);
  }

  &[data-size='lg'] {
    --dsc-button-font-size: var(--ds-font-size-6);
    --dsc-button-gap: var(--ds-sizing-3);
    --dsc-button-padding-block: var(--ds-spacing-3);
    --dsc-button-padding-inline: var(--ds-spacing-5);
  }
}

Kunne vi ha skrevet dette:

.ds-button {
  --dsc-button-font-size: var(--ds-font-size-5);

  font-size: var(--dsc-button-font-size);
  gap: var(--ds-sizing-scale-2);
  padding: var(--ds-sizing-scale-2) var(--ds-sizing-scale-4);

  &[data-size='sm'] { --dsc-button-font-size: var(--ds-font-size-4); }
  &[data-size='lg'] { --dsc-button-font-size: var(--ds-font-size-6); }
}

Og isteden for å skrive slik når (man ikke bruker React):

<nav class="ds-pagination" data-size="sm"> <-- data-size må gjentas
  <ol>
    <li><button type="button" data-size="sm">1</button></li> <-- data-size må gjentas
    <li><button type="button" data-size="sm">2</button></li> <-- data-size må gjentas
    <li><button type="button" data-size="sm">3</button></li> <-- data-size må gjentas
    <li><button type="button" data-size="sm">4</button></li> <-- data-size må gjentas
    <li><button type="button" data-size="sm">5</button></li> <-- data-size må gjentas
    <li><button type="button" data-size="sm">6</button></li> <-- data-size må gjentas
  </ol>
</nav>

Vil man kunne skrive dette:

<nav class="ds-pagination" data-size="sm"> <-- data-size er satt én gang wee
  <ol>
    <li><button type="button">1</button></li>
    <li><button type="button">2</button></li>
    <li><button type="button">3</button></li>
    <li><button type="button">4</button></li>
    <li><button type="button">5</button></li>
    <li><button type="button">6</button></li>
  </ol>
</nav>

Systemet baserer seg på at man lager en (eller flere) skala, basert på font-size:

:root {
  --ds-sizing-scale: .875rem;
  /* 👆 14px at 1rem, must be smaller than smallest font-size to have effect (because 14px font size - 14px scale = 0) */

  --ds-sizing-scale-1: calc(1em - var(--ds-sizing-scale) * 1);
  /* 👆 4px at sm, 8px at md, 12px at lg */
  --ds-sizing-scale-2: calc(2em - var(--ds-sizing-scale) * 2);
  /* 👆 6px at sm, 12px at md, 18px at lg */
  --ds-sizing-scale-3: calc(3em - var(--ds-sizing-scale) * 3);
  /* 👆 8px at sm, 16px at md, 24px at lg */
  --ds-sizing-scale-4: calc(4em - var(--ds-sizing-scale) * 4);
}

Da går det også an å gjøre:

/* For å endre størrelse til mobil med ren CSS: */
@media(max-width: 80em) {
  .ds-button { --dsc-button-font-size: var(--ds-font-size-4) }
}

...så dette tror jeg vil være mulig å gjenskape i Figma også?

Ser at det vil kreve en liten opprydding; er ikke alltid vi skalerer gap f.eks konsekvent, men det tror jeg er fint å rydde i uansett.
Vil gjerne høre hva dere tenker og gjerne sparre med @Thunear og @Febakke litt på om det designmessig gir mening også.

Vil også løst #1781
Vil også påvirke #1583

@eirikbacker eirikbacker converted this from a draft issue Sep 24, 2024
@eirikbacker eirikbacker added 🎨 figma Everything related to changes in Figma react @digdir/designsystemet-react 🕵️ investigate Needs investigation css @digdir/designsystemet-css labels Sep 24, 2024
@eirikbacker eirikbacker changed the title Font based sizing? Font based sizing and less useContext? Sep 24, 2024
@eirikbacker eirikbacker changed the title Font based sizing and less useContext? Font based scaling and less useContext? Sep 24, 2024
@mimarz
Copy link
Collaborator

mimarz commented Sep 25, 2024

Good suggestion. We'll definitely have a look at this.

To avoid more subject and ongoing refactors, I suggest we have a look at this after we finish issues we have already comited to doing.

Relevant issues on this subject:

Edit: Updated list of finished issues before this as we are able to do more things in parallel now.

@unekinn
Copy link
Contributor

unekinn commented Sep 25, 2024

A less radical approach to solve only #2453 and getting rid of useContext would be stuff like this:

.ds-pagination {
  &[data-size='sm'] .ds-button {
    --dsc-button-font-size: var(--ds-font-size-4);
    --dsc-button-gap: var(--ds-sizing-1);
    --dsc-button-padding-block: var(--ds-spacing-2);
    --dsc-button-padding-inline: var(--ds-spacing-3);
  }

  &[data-size='lg'] .ds-button {
    --dsc-button-font-size: var(--ds-font-size-6);
    --dsc-button-gap: var(--ds-sizing-3);
    --dsc-button-padding-block: var(--ds-spacing-3);
    --dsc-button-padding-inline: var(--ds-spacing-5);
  }
}

Which would still make this work

<nav class="ds-pagination" data-size="sm"> <-- data-size er satt én gang wee
  <ol>
    <li><button type="button">1</button></li>
    <li><button type="button">2</button></li>
    <li><button type="button">3</button></li>
    <li><button type="button">4</button></li>
    <li><button type="button">5</button></li>
    <li><button type="button">6</button></li>
  </ol>
</nav>

As for scaling sizes based on font-size, this warrants further consideration:

  • We would need to test that this doesn't introduce rendering artifacts like half-pixels etc between different browsers or different zoom levels.
  • Personally, I do not think there is a linear relationship between the font-size and the spacing necessary for a component. This is especially clear when scaling up a user interface by setting font-size at the root, at which point the amount of spacing can make the user interface unusable.

Also, I can't seem to get your example to work. I tried setting it up in a codesandbox but the relative sizing variables don't actually do anything – see the missing padding.
image

@eirikbacker
Copy link
Contributor Author

@unekinn The approach of

.ds-pagination {
  &[data-size='sm'] .ds-button {
    --dsc-button-font-size: var(--ds-font-size-4);
    --dsc-button-gap: var(--ds-sizing-1);
    --dsc-button-padding-block: var(--ds-spacing-2);
    --dsc-button-padding-inline: var(--ds-spacing-3);
  }
}

Might be a direction, but I'm concerned of:

  • Increased size of CSS: We need to reproduce the scaling of all sub components inside the source of all parent components
  • Increased chance of error: Probably we would do a @composes ds-button[data-size="sm"] instead of replicating (and thus forgetting to update) CSS, but we can quickly run into specificity issues as each component file hava certain specificity logic, and this can quickly break if composing. Also, if composing, should it also compose i.e. .ds-button[ds-size="sm"][aria-busy="true"] or not?
  • It will break if later introducing CSS modules, as each module file have its own unique hash based on file content, meaning we can not target .ds-button from inside .ds-pagination
  • As you say, it solves sizing parent<=>child relation, but not i.e. the possibility to change size based on @media etc.

Sorry - some errors in initially posted pseudo-code 😇
Here is a corrected working example: https://codepen.io/eirikbacker/pen/RwXNpXo

As we only multiply by whole numbers, there is no chance of half-pixels ❤️ (unless changing the root font-size to something like 16.5px but that is indeed forcing a wave of half-pixel-problems anyway ☺️ )

@eirikbacker
Copy link
Contributor Author

Status: Lasse is experimenting with this setup as "modes" in Figma

@Febakke Febakke added the 🧭 roadmap Used in roadmap label Sep 27, 2024
@eirikbacker eirikbacker changed the title Font based scaling and less useContext? Font based sizing / scaling and less useContext? Sep 27, 2024
@mrosvik mrosvik removed 🧭 roadmap Used in roadmap 🎨 figma Everything related to changes in Figma react @digdir/designsystemet-react css @digdir/designsystemet-css labels Sep 27, 2024
@eirikbacker
Copy link
Contributor Author

eirikbacker commented Oct 1, 2024

Oppdatert matematisk formel, som tilpasser skalaen basert på font-størrelse, og setter en minimum skala:
Se demo

:root {
  /* The config */
  --ds-sizing-scale-base: var(--ds-font-size-5); /* 18px */
  --ds-sizing-scale-min: 0.125rem; /* Minimum 2px steps */
  --ds-sizing-scale: 0.25rem; /* Default 4px steps */
  --ds-sizing-adjust: calc((var(--ds-sizing-scale-base) - 1em) * .5); /* Fallback if not supporting round() */
  --ds-sizing-adjust: round(up, calc((var(--ds-sizing-scale-base) - 1em) * .5), 0.0625rem); /* stylelint-disable-line declaration-block-no-duplicate-custom-properties */

  /* The scale */
  --ds-sizing-scale-1: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 1 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 1));
  --ds-sizing-scale-2: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 2 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 2));
  --ds-sizing-scale-3: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 3 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 3));
  --ds-sizing-scale-4: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 4 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 4));
  --ds-sizing-scale-5: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 5 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 5));
  --ds-sizing-scale-6: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 6 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 6));
  --ds-sizing-scale-7: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 7 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 7));
  --ds-sizing-scale-8: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 8 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 8));
  --ds-sizing-scale-9: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 9 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 9));
  --ds-sizing-scale-10: max(calc((var(--ds-sizing-scale) - var(--ds-sizing-adjust)) * 10 + var(--ds-sizing-adjust)), calc(var(--ds-sizing-scale-min) * 10));
}

@eirikbacker
Copy link
Contributor Author

Forenklet:

skala: 4px
minimum: 2px
fontStørrelseStart: 18px;

justering: (fontStørrelseStart - fontStørrelseNå) * .5;
skala-1: (skala - justering) * 1 + justering

==========================================================================================

så f.eks med tall, dersom fontStørrelseNå = 20px:

justering: (18px - 20px) * .5 = 1px;
skala-1: (4px - 1px) * 1 + 1px;

@mrosvik
Copy link
Member

mrosvik commented Oct 9, 2024

Next step: Should be tested in Figma. Create some sketches/ user interfaces with the suggested scale.

@eirikbacker eirikbacker removed their assignment Oct 16, 2024
@mimarz mimarz changed the title Font based sizing / scaling and less useContext? Font based sizing Oct 16, 2024
@mimarz mimarz added the ☂️ epic Issues ready label Oct 16, 2024
eirikbacker added a commit that referenced this issue Oct 24, 2024
Work on #2508 
Minor adjustments will be done in #2665

---------

Co-authored-by: Michael Marszalek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☂️ epic Issues ready 🕵️ investigate Needs investigation
Projects
Status: ☂ Todo Epics
Development

No branches or pull requests

5 participants