chore: Discuss token structure #1768
Conversation
github-actions
bot
temporarily deployed
to
demo-discuss-token-structure
|
|
||
| #### Body Texts Font Family | ||
|
|
||
| - `ams.body-texts.font-family.default` = `ams.font-family.body-text` |
There was a problem hiding this comment.
body-texts is een beetje vreemd. Heb je dit overwogen?
ams.text.body en ams.text.heading
of
ams.texts en ams.headings
(want headings staan ook gewoon in de body...)
There was a problem hiding this comment.
Het zit mij ook niet helemaal lekker – zie regels 238–241 hier direct boven) maar het is wel de geaccepteerde term voor het ding wat we bedoelen: de broodtekst tussen headings, zonder afbeeldingen en tabellen en zo. Ik heb hard gezocht maar niks beters gevonden.
Body is wel wat anders dan body text. De body van een artikel is alles behalve de hoofdtitel en de intro. De body van de pagina is het hele document.
We hebben juist een woord nodig waar de headings niet in zitten, omdat dat precies het onderscheid tussen beide is.
|
|
||
| 1. Je hebt een kleur rood (`ams.color.red-60`) ← Brand Token | ||
| 2. Die noem je de error kleur (`ams.feedback.color.error`) ← ??? Token… Semantic? | ||
| 3. Je hebt een Text Input met een foute border (`ams.text-input.invalid.box-shadow`) ← Component Token |
There was a problem hiding this comment.
Tot hier klopt het, ik denk dat je het component token wil verwijzen naar een common kleur (dus 2). Nummer 4 is een beetje doormalen als je het mij vraagt.
ams.text-input.invalid.box-shadow = ams.feedback.color.error
There was a problem hiding this comment.
Zou ik ook zeggen.
4e niveau aanbrengen betekent ook weer afwijken van de norm (volgens mij is 3 lagen gebruikelijk) dus dan moet daar een goede reden voor zijn. Die zie ik niet zo 1,2,3
There was a problem hiding this comment.
Yes, we zijn eruit:
- Palette: red-60, publiceren we niet
- Brand: color-warning, semantisch
- Common: form-controls-invalid-border-color, groeperend
- Component: date-input-invalid-border-color, component
|
|
||
| ## Discussie | ||
|
|
||
| ### Is drie niveaus eigenlijk wel genoeg? |
There was a problem hiding this comment.
Ik intussen ook!
| Het zou helpen als `group` altijd een meervoudsvorm zou zijn. | ||
| Dan kun je zorgen voor: ‘ui-achtig ding’ --> component, ‘meervoudig woord’ --> common, ‘vormgevingsaspect’ --> brand. | ||
|
|
||
| Zonder prefix zijn met name de Brand Tokens wel fantastisch kort. |
|
|
||
| ### Group | ||
|
|
||
| Een gangbare term voor een groep componenten, óf voor een aspect van vormgeving of gedrag. |
There was a problem hiding this comment.
De component groepen lijken mij nog vrij arbitrair. Kan me voorstellen dat we daar issues mee krijgen als we later toch dingen anders zouden willen indelen of anders willen groeperen(?)
There was a problem hiding this comment.
Bij nader inzien klopt ‘feedback’ niet in dit lijstje. Die andere twee lijken wel logisch: veel form controls delen de zwarte border, die rood wordt bij een fout, de witte achtergrond, de typografie. En alle links willen dezelfde tinten blauw gebruiken voor initieel en hover.
Laten we deze zoeken en vinden bij het implementeren. Ik stel ook voor dat we dat eerst helemaal in code afronden, daar kunnen we iets gemakkelijker ‘find en replace’ en andere refactoring doen, en daarna alles in Figma (voor zover nodig) omhangen.
| ### Modifier | ||
|
|
||
| Een variatie of status van een component. | ||
| Voorbeelden: `xs`, `level-1`, `primary`, `hover`. |
There was a problem hiding this comment.
Deze zijn niet mutually exclusive he, ik weet niet of dat uitmaakt hoor.
Maar je kan natuurlijk een 'primary' en 'hover' tegelijk hebben. Toon je ze dan dus beiden? Welke komt dan eerst?
There was a problem hiding this comment.
Klopt hoor, dit is een lijstje voorbeelden uit diverse categorieën.
|
|
||
| Even alles op een hoop: | ||
|
|
||
| - `ams.form-controls.background-color.default` (zou `fill` hier logisch zijn ipv `background-color`?) = `ams.color.neutral-0` |
There was a problem hiding this comment.
'Fill' komt wel overeen met hoe het in Figma heet ook.
There was a problem hiding this comment.
Ik refereer hier aan hoe Polaris ‘background’, ‘surface’ en ‘fill’ onderscheidt – allemaal voor achtergrondkleur. Misschien kunnen we dat adopteren als blijkt dat we het nodig hebben, hou ik in mijn achterhoofd bij het implementeren.
|
|
||
| 1. Je hebt een kleur rood (`ams.color.red-60`) ← Brand Token | ||
| 2. Die noem je de error kleur (`ams.feedback.color.error`) ← ??? Token… Semantic? | ||
| 3. Je hebt een Text Input met een foute border (`ams.text-input.invalid.box-shadow`) ← Component Token |
There was a problem hiding this comment.
Zou ik ook zeggen.
4e niveau aanbrengen betekent ook weer afwijken van de norm (volgens mij is 3 lagen gebruikelijk) dus dan moet daar een goede reden voor zijn. Die zie ik niet zo 1,2,3
| ### Tokens aanroepen vanuit componenten | ||
|
|
||
| Componenten mogen zowel Brand als Common Tokens aanroepen. | ||
| Alleen Common Tokens toestaan is wat te strikt – in sommige gevallen moet je dan een letterlijke kopie maken van Brand naar Common. |
There was a problem hiding this comment.
Ja dat lijkt me ook fijn, om het niet te dicht te timmeren qua richtlijnen in dat opzicht
| Alleen Common Tokens toestaan is wat te strikt – in sommige gevallen moet je dan een letterlijke kopie maken van Brand naar Common. | ||
| Ook NLDS geeft aan dat beide mag. | ||
|
|
||
| Wel moeten mensen op de een of andere manier zorgen dat ze een Common Token kiezen als daar een toepasselijke van bestaat. |
| Alleen Common Tokens toestaan is wat te strikt – in sommige gevallen moet je dan een letterlijke kopie maken van Brand naar Common. | ||
| Ook NLDS geeft aan dat beide mag. | ||
|
|
||
| Wel moeten mensen op de een of andere manier zorgen dat ze een Common Token kiezen als daar een toepasselijke van bestaat. |
There was a problem hiding this comment.
Ik heb nu geen tijd om de hele PR door te nemen, maar hier zit volgens mij de crux van een issue met de voorgestelde opzet: Waarom publiceren we tokens die gebruikers niet mogen gebruiken?
Ik denk dat dit issue is ontstaan door deze keuze:
[Een group is] Een gangbare term voor een groep componenten, óf voor een aspect van vormgeving of gedrag.
Als je 'een aspect van vormgeving of gedrag' in brand neerzet (bv ams.brand.color.feedback.error) en group / common bewaart voor groepen tokens die in meerdere componenten voor komen, heb je dat issue niet meer denk ik.
There was a problem hiding this comment.
Dank voor dit inzicht! Die ‘óf’ was al een ‘modelling smell’ :)
|
|
||
| ### Group | ||
|
|
||
| Een gangbare term voor een groep componenten, óf voor een aspect van vormgeving of gedrag. |
There was a problem hiding this comment.
Bij nader inzien klopt ‘feedback’ niet in dit lijstje. Die andere twee lijken wel logisch: veel form controls delen de zwarte border, die rood wordt bij een fout, de witte achtergrond, de typografie. En alle links willen dezelfde tinten blauw gebruiken voor initieel en hover.
Laten we deze zoeken en vinden bij het implementeren. Ik stel ook voor dat we dat eerst helemaal in code afronden, daar kunnen we iets gemakkelijker ‘find en replace’ en andere refactoring doen, en daarna alles in Figma (voor zover nodig) omhangen.
| ### Modifier | ||
|
|
||
| Een variatie of status van een component. | ||
| Voorbeelden: `xs`, `level-1`, `primary`, `hover`. |
There was a problem hiding this comment.
Klopt hoor, dit is een lijstje voorbeelden uit diverse categorieën.
|
|
||
| #### Body Texts Font Family | ||
|
|
||
| - `ams.body-texts.font-family.default` = `ams.font-family.body-text` |
There was a problem hiding this comment.
Het zit mij ook niet helemaal lekker – zie regels 238–241 hier direct boven) maar het is wel de geaccepteerde term voor het ding wat we bedoelen: de broodtekst tussen headings, zonder afbeeldingen en tabellen en zo. Ik heb hard gezocht maar niks beters gevonden.
Body is wel wat anders dan body text. De body van een artikel is alles behalve de hoofdtitel en de intro. De body van de pagina is het hele document.
We hebben juist een woord nodig waar de headings niet in zitten, omdat dat precies het onderscheid tussen beide is.
|
|
||
| Even alles op een hoop: | ||
|
|
||
| - `ams.form-controls.background-color.default` (zou `fill` hier logisch zijn ipv `background-color`?) = `ams.color.neutral-0` |
There was a problem hiding this comment.
Ik refereer hier aan hoe Polaris ‘background’, ‘surface’ en ‘fill’ onderscheidt – allemaal voor achtergrondkleur. Misschien kunnen we dat adopteren als blijkt dat we het nodig hebben, hou ik in mijn achterhoofd bij het implementeren.
|
|
||
| ## Discussie | ||
|
|
||
| ### Is drie niveaus eigenlijk wel genoeg? |
There was a problem hiding this comment.
Ik intussen ook!
|
|
||
| 1. Je hebt een kleur rood (`ams.color.red-60`) ← Brand Token | ||
| 2. Die noem je de error kleur (`ams.feedback.color.error`) ← ??? Token… Semantic? | ||
| 3. Je hebt een Text Input met een foute border (`ams.text-input.invalid.box-shadow`) ← Component Token |
There was a problem hiding this comment.
Yes, we zijn eruit:
- Palette: red-60, publiceren we niet
- Brand: color-warning, semantisch
- Common: form-controls-invalid-border-color, groeperend
- Component: date-input-invalid-border-color, component
| Alleen Common Tokens toestaan is wat te strikt – in sommige gevallen moet je dan een letterlijke kopie maken van Brand naar Common. | ||
| Ook NLDS geeft aan dat beide mag. | ||
|
|
||
| Wel moeten mensen op de een of andere manier zorgen dat ze een Common Token kiezen als daar een toepasselijke van bestaat. |
There was a problem hiding this comment.
Dank voor dit inzicht! Die ‘óf’ was al een ‘modelling smell’ :)
I’ve been researching how to structure our tokens, especially the common ones. I started writing everything down to organize my thoughts. Then, the document became a suitable discussion piece. This pull request facilitates that conversation; I don’t intend to merge it.