chore: update tokens #2773
chore: update tokens #2773
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
This is the first really basic generation of component tokens (the buttons). Let's check that to see if that could be a good start for the buttons' implementation. |
There was a problem hiding this comment.
Questions
- Will we have a layer of CSS variables somehow to avoid duplicating these Sass variables for each theme ?
- Some colors aren't accessible (
#eeeeeeand#f16e00or#ffffffand#f16e00(text is too small) for instances). - There isn't any mention of the minimal dimensions of the button (and the maximal as well).
There isn't any mention of the typography in here, do we fix it manually ?I think they are default semantic usage.No mention of the negative nor the strong variants ?I think they are default semantic usage.- Do we support
:hasfor this component ? It could be easier to use for our users but maybe not compatible everywhere. - We don't have any specification for the loader, do we use the one from Bootstrap ? Or should we wait for the loader to be defined ? Do we use pseudo elements to hide the content of the button ? Is the loading state launched on click or is this handled by the projects ? What are the hover/focus state of the loading state ? Is it pointer-event disabled ? What pointer should we display ?
- We don't have any specification for the focus, do we use the one from Boosted ? Or should we wait for the focus to be defined ? Is it the same for most components ?
- Disabled state is almost invisible, it doesn't look accessible at the first sight, can't we have another way than color to handle disabled state ?
- Not a big fan of the way the icon of the dropdown is handled. I'd change the orientation of a determined icon rather than change the icon, any thought in here ? Maybe ieasier to handle for b-brands ?
| // Button | ||
|
|
||
| // scss-docs-start ouds-component-button | ||
| $ouds-button-background-color-default-disabled: $ouds-color-transparent-default !default; |
There was a problem hiding this comment.
Could we have bg instead of background-color that is a bit heavy ?
| $ouds-button-border-color-minimal-loading-on-bg-emphasized: $ouds-color-transparent-default !default; | ||
| $ouds-button-border-color-minimal-pressed: $ouds-color-transparent-default !default; | ||
| $ouds-button-border-color-minimal-pressed-on-bg-emphasized: $ouds-color-transparent-default !default; | ||
| $ouds-button-border-radius-radius: $ouds-border-radius-none !default; |
|
|
||
| // scss-docs-start ouds-component-button | ||
| $ouds-button-background-color-default-disabled: $ouds-color-transparent-default !default; | ||
| $ouds-button-background-color-default-disabled-on-bg-emphasized: $ouds-color-transparent-default !default; |
There was a problem hiding this comment.
I'd say that the variable should rather be called $ouds-default-button-disabled-bg-on-bg-emphasized or $ouds-button-default-disabled-bg-on-bg-emphasized
| $ouds-button-background-color-default-disabled: $ouds-color-transparent-default !default; | ||
| $ouds-button-background-color-default-disabled-on-bg-emphasized: $ouds-color-transparent-default !default; | ||
| $ouds-button-background-color-default-enabled: $ouds-color-transparent-default !default; | ||
| $ouds-button-background-color-default-enabled-on-bg-emphasized: $ouds-color-transparent-default !default; |
There was a problem hiding this comment.
There shouldn't be any on-bg-emphasized imo, we rather have a theme handling this. I won't go any further since we don't have the color tokens implemented now.
| $ouds-button-border-color-minimal-pressed-on-bg-emphasized: $ouds-color-transparent-default !default; | ||
| $ouds-button-border-radius-radius: $ouds-border-radius-none !default; | ||
| $ouds-button-border-width-default: $ouds-border-width-default !default; | ||
| $ouds-button-border-width-minimal: $ouds-border-width-none !default; |
There was a problem hiding this comment.
I think button variants should mainly change the colors of the buttons rather than changing its architecture, but maybe it's too early to see the impacts in here ? Maybe components will later change their own structure ? I think it can be misleading but I'm open to suggestions.
There is no mention of the border width of the default button on hover or focus.
| $ouds-button-space-column-gap-icon-end: $ouds-space-column-gap-with-arrow-short !default; | ||
| $ouds-button-space-column-gap-icon-start: $ouds-space-column-gap-with-icon-medium !default; | ||
| $ouds-button-space-inset-icon-alone: $ouds-space-inset-medium !default; | ||
| $ouds-button-space-padding-block: $ouds-space-padding-block-medium !default; |
There was a problem hiding this comment.
I'm wondering how we should do it if the spacing was scaled over breakpoints in here. Should we introduce Sass variables that are CSS Variables or are you coming up with our CSS variables ?
| $ouds-button-size-icon: $ouds-size-icon-with-label-large-size-shorter !default; | ||
| $ouds-button-size-icon-only: $ouds-size-icon-with-label-large-size-short !default; | ||
| $ouds-button-size-loader: $ouds-size-icon-with-label-large-size-shorter !default; | ||
| $ouds-button-space-column-gap-icon-end: $ouds-space-column-gap-with-arrow-short !default; |
There was a problem hiding this comment.
On our side, it's more related to a dropdown, do we still consider it as a button variant ?
|
Closing this PR, but let's keep in mind your comments @louismaximepiton for the implementation of the buttons ;) |
This PR is generated automatically, it updates the tokens based on Figma Variables.