feat(tile): add design tokens #10476
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,42 +7,65 @@ | |
| * @prop --calcite-tile-border-color: Specifies the border color of the component. | ||
| * @prop --calcite-tile-description-text-color: Specifies the description text color of the component. | ||
| * @prop --calcite-tile-heading-text-color: Specifies the heading text color of the component. | ||
| * @prop --calcite-tile-icon-color: Specifies the icon color in the component. | ||
| * @prop --calcite-tile-selection-icon-color: Specifies the color of the selection icon in the component. | ||
| * @prop --calcite-tile-selection-icon-color-hover: Specifies the color of the selection icon in the component on hover. | ||
|
There was a problem hiding this comment. suggestion: There was a problem hiding this comment. How would a user override the icon color separately from the title or description text? Just There was a problem hiding this comment. Because Tile now uses an actual There was a problem hiding this comment. The content one. I thought the original set you had here made the most sense, just IMO - to your point it makes the distinction between selection icon more clear. There was a problem hiding this comment. Good point. calcite-icon {
color: var(--calcite-title-icon-color, var(--calcite-icon-color, currentColor));
}There was a problem hiding this comment. Yeah that looks good. I think the “selection-“ prefix does make it kind of clear and we use it elsewhere, so agreed with some consistent naming there it’s not as bad. I don’t think using component specific naming is an issue there - even when we have the global overrides I think we’d want publicly documented component-specific ones. Having these at the component level even if there are underlying higher level props makes documentation easier as well IMO. Users won’t need to track down these possible properties. There was a problem hiding this comment. Agreed! .selection-icon-example-class {
color: var(--calcite-title-selection-icon-color, var(--calcite-selection-icon-color, “whatever underlying default”));
}looks good. There was a problem hiding this comment. Am I reading that right? Should it be --calcite-tile-icon-color ? |
||
| * @prop --calcite-tile-shadow: Specifies the box-shadow of the component. | ||
| */ | ||
|
|
||
| /** Internal variables | ||
| * --calcite-internal-tile-border-color | ||
| * --calcite-internal-tile-description-text-color | ||
| * --calcite-internal-tile-heading-text-color | ||
| * --calcite-internal-tile-icon-color | ||
| * --calcite-internal-tile-outline-color | ||
| * --calcite-internal-tile-selection-icon-color | ||
| * --calcite-internal-tile-shadow | ||
| */ | ||
|
|
||
| :host { | ||
| --calcite-internal-tile-border-color: var(--calcite-tile-border-color, var(--calcite-color-border-2)); | ||
| --calcite-internal-tile-description-text-color: var( | ||
| --calcite-tile-description-text-color, | ||
| var(--calcite-color-text-3) | ||
| ); | ||
| --calcite-internal-tile-heading-text-color: var(--calcite-tile-heading-text-color, var(--calcite-color-text-2)); | ||
| --calcite-internal-tile-icon-color: var(--calcite-color-text-3); | ||
|
There was a problem hiding this comment. it's confusing to set all the |
||
|
|
||
eriklharper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| box-sizing: border-box; | ||
| display: inline-block; | ||
| } | ||
|
|
||
| .container { | ||
| --calcite-internal-tile-outline-color: var(--calcite-tile-border-color, var(--calcite-internal-tile-border-color)); | ||
eriklharper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| --calcite-internal-tile-select-icon-color: var(--calcite-tile-selection-icon-color, var(--calcite-color-text-3)); | ||
|
|
||
| background-color: var(--calcite-tile-background-color, var(--calcite-color-foreground-1)); | ||
| block-size: var(--calcite-container-size-content-fluid); | ||
| box-sizing: border-box; | ||
| inline-size: var(--calcite-container-size-content-fluid); | ||
| outline: var(--calcite-border-width-sm, 1px) solid var(--calcite-internal-tile-outline-color); | ||
| user-select: none; | ||
| box-shadow: var(--calcite-tile-shadow, var(--calcite-internal-tile-shadow, none)); | ||
eriklharper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| &.interactive { | ||
| cursor: pointer; | ||
|
|
||
| &:hover, | ||
| &:focus, | ||
| &.selected { | ||
| --calcite-internal-tile-outline-color: var(--calcite-tile-border-color, var(--calcite-color-brand)); | ||
| --calcite-internal-tile-select-icon-color: var( | ||
| --calcite-tile-selection-icon-color-hover, | ||
| var(--calcite-color-brand) | ||
| ); | ||
| position: relative; | ||
| } | ||
| &.selected { | ||
| z-index: var(--calcite-z-index); | ||
| } | ||
| &:focus { | ||
| --calcite-internal-tile-shadow: inset 0 0 0 1px var(--calcite-color-brand); | ||
eriklharper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| z-index: var(--calcite-z-index-sticky); | ||
| } | ||
| } | ||
|
|
@@ -70,7 +93,7 @@ | |
| } | ||
|
|
||
| .heading { | ||
| color: var(--calcite-tile-heading-text-color, var(--calcite-internal-tile-heading-text-color)); | ||
| font-weight: var(--calcite-font-weight-medium); | ||
| line-height: 1.20313rem; | ||
| overflow-wrap: break-word; | ||
|
|
@@ -94,25 +117,31 @@ | |
| } | ||
|
|
||
| .description { | ||
| color: var(--calcite-tile-description-text-color, var(--calcite-internal-tile-description-text-color)); | ||
eriklharper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| font-weight: var(--calcite-font-weight-regular); | ||
| overflow-wrap: break-word; | ||
| } | ||
|
|
||
| calcite-icon { | ||
| --calcite-icon-color: var( | ||
|
There was a problem hiding this comment. the calcite-icon styles can be removed here as it will inherit the default "currentColor" of the component. |
||
| --calcite-tile-icon-color, | ||
| var(--calcite-internal-tile-icon-color, var(--calcite-color-text-3)) | ||
| ); | ||
|
|
||
| &.selection-icon { | ||
| --calcite-icon-color: var(--calcite-internal-tile-select-icon-color); | ||
| } | ||
| } | ||
|
|
||
| :host([alignment="center"]) { | ||
| .icon { | ||
| align-self: center; | ||
| } | ||
| .text-content { | ||
| text-align: center; | ||
| } | ||
| slot[name="content-start"]::slotted(*), | ||
| slot[name="content-end"]::slotted(*) { | ||
| align-self: center; | ||
| } | ||
| } | ||
|
|
@@ -189,6 +218,8 @@ | |
| :host([selection-appearance="border"][layout="horizontal"]), | ||
| :host([selection-appearance="border"][layout="vertical"]) { | ||
| .container.selected:focus::before { | ||
| --calcite-internal-tile-shadow: inset 0 0 0 1px var(--calcite-color-brand); | ||
|
There was a problem hiding this comment. --calcite-internal-tile-shadow-color: var(--calcite-tile-color-selected, var(--calcite-color-brand)); |
||
|
|
||
| block-size: 100%; | ||
| box-shadow: inset 0 0 0 1px var(--calcite-color-brand); | ||
| content: ""; | ||
|
|
@@ -202,21 +233,28 @@ | |
|
|
||
| :host([selection-appearance="border"][layout="horizontal"]) { | ||
| .container.selected { | ||
| --calcite-internal-tile-shadow: inset 0 -4px 0 0 var(--calcite-color-brand); | ||
|
There was a problem hiding this comment. change this back to a static style prop box-shadow: inset 0 -4px 0 0 var(--calcite-internal-tile-shadow-color, var(--calcite-color-brand)); |
||
| } | ||
| } | ||
|
|
||
| :host([selection-appearance="border"][layout="vertical"]) { | ||
| .container.selected { | ||
|
There was a problem hiding this comment. only the "x" and "y" axis lengths are in changing between these box-shadows. You could probably simplify this down to a single style prop. Something like.... .container {
box-shadow: inset var(--calcite-internal-tile-shadow-x) var(--calcite-internal-tile-shadow-y) 0 0 var(--calcite-internal-tile-shadow-color, none);
}
:host([selection-appearance="border"]) .container.selected {
--calcite-internal-tile-shadow-color: var(--calcite-tile-color-selected, var(--calcite-color-brand));
}
:host([layout="vertical"]) .container {
--calcite-internal-tile-shadow-x: 4px;
--calcite-internal-tile-shadow-y: 0;
}
:host([layout="horizontal"]) .container {
--calcite-internal-tile-shadow-x: 0;
--calcite-internal-tile-shadow-y: -4px;
} |
||
| --calcite-internal-tile-shadow: inset 4px 0 0 0 var(--calcite-color-brand); | ||
| } | ||
| } | ||
|
|
||
| :host([href]:active:not([disabled])) { | ||
| .container { | ||
| box-shadow: 0 0 0 3px var(--calcite-tile-border-color); | ||
| } | ||
| } | ||
|
|
||
| :host([href]:focus:not([disabled])), | ||
| :host([href]:hover:not([disabled])) { | ||
| --calcite-internal-tile-border-color: var(--calcite-color-text-link); | ||
| --calcite-internal-tile-heading-text-color: var(--calcite-color-text-link); | ||
| --calcite-internal-tile-icon-color: var(--calcite-color-text-link); | ||
|
There was a problem hiding this comment. I think we need to add an extra token for this case :host([href]:focus:not([disabled])),
:host([href]:hover:not([disabled])) {
.container,
.heading {
color: var(--calcite-title-href-text-color, var(--calcite-color-text-link));
}
.container {
border-color: var(--calcite-title-href-text-color, var(--calcite-color-text-link));
}
} |
||
| --calcite-ui-icon-color: var(--calcite-color-text-link); | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. Not sure if removing this would constitute a breaking change or not, so I hesitate to remove that. All of these changes straight-up copied over from the tokens branch, so my guess is this is here for a reason to prevent a breakage. Unless we're ok with breaking changes on these PRs, I suppose it would be fine to remove it. @jcfranco thoughts? There was a problem hiding this comment. the Instead this should be something like the following. calcite-icon {
color: var(--calcite-tile-href-icon-color, var(--calcite-color-text-link));
} |
||
| .container { | ||
| position: relative; | ||
| z-index: var(--calcite-z-index); | ||
|
|
@@ -225,35 +263,35 @@ | |
|
|
||
| :host([href]:active:not([disabled])) { | ||
| .container { | ||
| --calcite-internal-tile-shadow: 0 0 0 3px var(--calcite-tile-border-color); | ||
| } | ||
| } | ||
|
|
||
| :host(:hover:not([disabled])), | ||
| :host([active]:not([disabled])) { | ||
| --calcite-internal-tile-description-text-color: var(--calcite-color-text-2); | ||
| --calcite-internal-tile-heading-text-color: var(--calcite-color-text-1); | ||
| } | ||
|
|
||
| :host([embed]) { | ||
| .container { | ||
| --calcite-internal-tile-shadow: none; | ||
| padding: 0; | ||
| } | ||
| } | ||
|
|
||
| :host([selection-mode="none"]) { | ||
| .container { | ||
| --calcite-internal-tile-outline-color: var(--calcite-internal-tile-border-color); | ||
|
|
||
| &:focus { | ||
| --calcite-internal-tile-outline-color: var(--calcite-color-brand); | ||
| position: relative; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @include disabled(); | ||
| @include base-component(); | ||
|
|
||
| ::slotted(*) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { html } from "../../support/formatting"; | ||
|
|
||
| export const tileTokens = { | ||
| calciteTileBackgroundColor: "", | ||
| calciteTileBorderColor: "", | ||
| calciteTileDescriptionTextColor: "", | ||
| calciteTileHeadingTextColor: "", | ||
| calciteTileIconColor: "", | ||
| calciteTileSelectionIconColor: "", | ||
| calciteTileSelectionIconColorHover: "", | ||
| calciteTileShadow: "", | ||
| }; | ||
|
|
||
| export const tile = html` | ||
| <calcite-tile | ||
| heading="Tile heading lorem ipsum" | ||
| description="Leverage agile frameworks to provide a robust synopsis for high level overviews. Iterative approaches to corporate strategy foster collab on thinking to further the overall." | ||
| icon="layers" | ||
| selected | ||
| ></calcite-tile> | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we still want this? There are two different colors used for the heading / description UI ... I'd expect a css property to individually control each. cc @ashetland @SkyeSeitz to confirm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alisonailea 's suggestion is simply to replace
--calcite-tile-description-text-colorwith--calcite-tile-text-colorwhich effectively serves as like the "body" default text color, and because it is applied to the.containerdiv.descriptiondiv, it will set a color for the whole content of the tile, including icons and other content.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description and heading are different colors currently and will remain different colors in future Tile enhancements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there isn’t a default content slot, I’d expect the css props to match the property name for each.
Other content is slotted in. This is why I still think the “icon prop” would have its own css property.
As a user without knowledge of the component, setting the icon to the description color is confusing (or, having description / icon be text-color but then also having a “named” text color property for heading).