feat(card): component tokens #10161
feat(card): component tokens #10161
Conversation
…tokens for deprecated feature - checkbox
…active/7180-card # Conflicts: # packages/calcite-components/src/components/card/card.e2e.ts
alisonailea
requested review from
jcfranco,
eriklharper,
driskull and
macandcheese
github-actions
bot
added
the
enhancement
| * @prop --calcite-card-selection-background-color-active: [Deprecated] Specifies the background color of the component's selection element when active. | ||
| * @prop --calcite-card-selection-background-color-hover: [Deprecated] Specifies the background color of the component's selection element when hovered. | ||
| * @prop --calcite-card-selection-background-color-selected: [Deprecated] Specifies the icon color of the component's selection element when `selected`. | ||
| * @prop --calcite-card-selection-background-color: [Deprecated] Specifies the background color of the component's selection element. |
There was a problem hiding this comment.
I’m still not sure why we are removing most of these. How would a user change the color of this internally rendered element? Applies to most of these deprecations.
There was a problem hiding this comment.
It prevents what is a very common use case for tokens in my mind, something like this:
There was a problem hiding this comment.
these are background colors are for for transparent design pattern behind icons that creates a shading effect. Unless there is a specific user request edge case to break this design pattern there is no need to provide these tokens.
There was a problem hiding this comment.
I think those underlying values should probably use fg-2, fg-3, not -color-transparent. I believe that may have been a holdover from when they were positioned over a thumbnail (?).
As a designer for the above design though, I'd want to be able to control those.
There was a problem hiding this comment.
I would agree we should use fg-2, fg-3, not -color-transparent & having control over the above seems reasonable.
alisonailea
added
the
pr ready for visual snapshots
| * @prop --calcite-card-selection-color-hover: Specifies the color of the component's selection element when hovered or focused. | ||
| * @prop --calcite-card-selection-background-color-active: [Deprecated] Use --calcite-card-background-color-press. Specifies the background color of the component's selection element when active. | ||
| * @prop --calcite-card-selection-background-color-hover: [Deprecated] --calcite-card-background-color-hover. Specifies the background color of the component's selection element when hovered. | ||
| * @prop --calcite-card-selection-background-color-selected: [Deprecated] Use --calcite-card-background-color. Specifies the icon color of the component's selection element when `selected`. |
There was a problem hiding this comment.
Are these still intended to be deprecated?
I would not expect to use “—calcite-card-background-color” to control the background color of the internal “checkbox element” like this deprecation message suggests.
The newly introduced names are confusing to me, there is no general hover or press state or interaction case for a Card component itself, those states should be applied separately via css prop imo. If a user wants to set calcite-card:hover { whatever } they can do so, but this internally rendered element is different.
There was a problem hiding this comment.
Yes.
- the checkbox component should probably control it's own hover styles anyway
- this is added style complexity which breaks our existing design patterns unless there is a current user request use case I don't know about.
- While the entire card background doesn't change there is a design pattern of foreground-1 for default, foreground-2 for hover, and foreground-3 for active, which is basically what we are doing. The goal is not to recreate CSS styling or JS props with CSS variables but to create a baseline set of styles which allow flexibility while maintaining our styling cadence. Naming every sub-element in a component with it's own set of variables when changing only the styles of that one element would break the design pattern without cause is an anti-pattern IMO
There was a problem hiding this comment.
It's not actually a checkbox component - it's just a styled div with an icon as it is in most other selection-enabled components.
The design I posted above with the multiple selection cards is a real-world use case I'd like to achieve (and is currently possible) - so removing these would prevent that design. We shouldn't be arbiters of theming choices - if we are truly making our components fully them able via tokens, everything should be theme able, not just what some deem appropriate.
I think this proposed nomenclature is more confusing - there is no hover state for the card - and we want folks to use stateful overrides (:hover, etc.) for actual parent-level adjustments based on those interactions right?
IMO setting --calcite-card-background-color-hover to control the bg of a portion of the card doesn't make sense.
There was a problem hiding this comment.
if we are truly making our components fully them able via tokens, everything should be theme able, not just what some deem appropriate.
This is not what we have discussed as a team. What we are providing is white labeling. We still have design patterns we have set and want to adhere to. That said, I can understand your argument against --calcite-card-background-color-hover, I've updated the tokens. Is this an acceptable compromise?
alisonailea
added
pr ready for visual snapshots
…openPR/7180-card # Conflicts: # packages/calcite-components/src/components/card/card.scss
alisonailea
added
pr ready for visual snapshots
alisonailea
added
pr ready for visual snapshots
Related Issue: #7180
Summary
Removes some extraneous background color tokens and merges color and icon tokens to align with the design pattern. This also resolves an icon issue discovered in #10062
Tokens
--calcite-card-accent-color-selected: Specifies the accent color of the component when
selected.--calcite-card-background-color: Specifies the background color of the component.
--calcite-card-border-color: Specifies the border color of the component.
--calcite-card-corner-radius: Specifies the corner radius of the component.
--calcite-card-selection-background-color-hover: Specifies the background color of the component's selection element when hovered.
--calcite-card-selection-background-color-press: Specifies the background color of the component's selection element when active.
--calcite-card-selection-color-hover: Specifies the color of the component's selection element when hovered or focused.
--calcite-card-selection-color: Specifies the color of the component's selection element.
--calcite-card-shadow: Specifies the shadow of the component.
Deprecated
--calcite-card-selection-background-color-active: Use --calcite-card-selection-background-color-press. Specifies the background color of the component's selection element when active.
--calcite-card-selection-background-color-selected: Use --calcite-card-background-color. Specifies the icon color of the component's selection element when
selected.--calcite-card-selection-background-color: Use --calcite-card-background-color. Specifies the background color of the component's selection element.
--calcite-card-selection-icon-color-hover: Use --calcite-card-selection-color-hover. Specifies the icon color of the component's selection element when hovered.
--calcite-card-selection-icon-color-selected: Use --calcite-card-accent-color-selected. Specifies the icon color of the component's selection element when
selected.--calcite-card-selection-icon-color: Use --calcite-card-selection-color. Specifies the icon color of the component's selection element.