Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
components/tile/src/_mixin.scss
Outdated
| color: var(--utrecht-color-grey-10); | ||
| display: flex; | ||
| justify-content: space-between; | ||
| min-block-size: 6.25rem; | ||
| padding-block-end: var(--utrecht-space-inline-md); |
There was a problem hiding this comment.
Mooi om meteen tokens te zien al! Ik zie dat er veel common of brand tokens nu nog in de CSS staan.
Common of brand tokens wil zeggen dat ze uit proprietary/design-tokens/src/{common,brand} komen. Die zijn "utrecht specifiek" en wil eigenlijk niet terug zien in de CSS van het component. In plaats daar van kan je dit doen in je CSS:
color: var(--utrecht-tile-color);
padding-block-end: var(--utrecht-tile-padding-block-end);De design tokens .json file voor het component kan vervolgens de connectie leggen naar de common of brand laag. Bijvoorbeeld
"utrecht": {
"tile": {
"color": {
"value": "{utrecht.color.grey.10}"
},
}
}There was a problem hiding this comment.
Ik zie dat je het tokens json stukje al hebt ;)
| import iconSet from '@utrecht/icon/dist/index.json'; | ||
| import readme from '@utrecht/tile-css/README.md?raw'; | ||
| import tokensDefinition from '@utrecht/tile-css/src/tokens.json'; | ||
| import React, { type ComponentProps, type CSSProperties } from 'react'; |
There was a problem hiding this comment.
{ type ComponentProps, type CSSProperties } worden niet gebruikt dus kunnen weg
| 'utrecht-tile--active': active, | ||
| 'utrecht-tile--hover': hover, | ||
| 'utrecht-tile--focus': focus, | ||
| 'utrecht-tile--focus-visible': focusVisible, |
There was a problem hiding this comment.
deze pseudoclasses zijn hier niet nodig verwacht ik, waarschijnlijk hoeven ze ook niet in de TileProps te staan
packages/storybook-css/src/Tile.tsx
Outdated
| focusVisible?: boolean; | ||
| keyboardSupport?: boolean; | ||
| icon?: ReactNode; | ||
| color?: 'default' | 'red' | 'green' | 'blue'; |
There was a problem hiding this comment.
dit doet me denken aan de oude color attribute direct op HTML elements :), is er een semantische gedachte achter de kleuren te bedenken, en kunnen we de property wellicht veranderen naar appearance (zoals bij button)?
There was a problem hiding this comment.
Hier komen we nog op terug, input nodig van anderen.
There was a problem hiding this comment.
De waarden die hier ingevuld staan, horen eigenlijk in:
proprietary/design-tokens/src/component/utrecht/tile.tokens.json.
De tokens.json die direct in de component map staat, is bedoeld voor de documentatie van de token spec voor dit component. Zie bijvoorbeeld de alert component:
https://github.com/nl-design-system/utrecht/blob/main/components/alert/src/tokens.json
pixelgitter
force-pushed
the
feature/component-tiles
branch
from
a39c663 to
06f3a24
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2981 +/- ##
=======================================
Coverage 90.73% 90.73%
=======================================
Files 201 201
Lines 1760 1760
Branches 375 382 +7
=======================================
Hits 1597 1597
Misses 157 157
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… and appearance states
|
2 participants
Tile component opgezet. Dit is de eerste opzet, dus graag je kritische blik hierop! :)