Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tile): add design tokens #10476

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/calcite-components/.stylelintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const customFunctions = [
"medium-modular-scale",
"modular-scale",
"scale-duration",
"scaleDuration",
"small-modular-scale"
];
// ⚠️ END OF AUTO-GENERATED CODE
Expand Down
82 changes: 80 additions & 2 deletions packages/calcite-components/src/components/tile/tile.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
import { newE2EPage } from "@stencil/core/testing";
import { accessible, defaults, disabled, focusable, hidden, reflects, renders, slots } from "../../tests/commonTests";
import {
accessible,
defaults,
disabled,
focusable,
hidden,
reflects,
renders,
slots,
themed,
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { isElementFocused } from "../../tests/utils";
import { SLOTS } from "./resources";
import { CSS, SLOTS } from "./resources";

describe("calcite-tile", () => {
describe("accessibility", () => {
Expand Down Expand Up @@ -211,4 +221,72 @@ describe("calcite-tile", () => {
expect(description).toBeNull();
});
});

describe("theme", () => {
describe("default", () => {
themed(
html`
<calcite-tile
heading="Tile heading lorem ipsum"
description="Leverage agile frameworks to provide a robust synopsis for high level overviews."
icon="layers"
interactive
selected
>
</calcite-tile>
`,
{
"--calcite-tile-background-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
},
"--calcite-tile-border-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "outlineColor",
},
"--calcite-tile-heading-text-color": {
shadowSelector: `.${CSS.heading}`,
targetProp: "color",
},
"--calcite-tile-icon-color": {
shadowSelector: "calcite-icon",
targetProp: "--calcite-icon-color",
},
"--calcite-tile-shadow": {
selector: `calcite-tile`,
targetProp: "boxShadow",
},
"--calcite-tile-text-color": {
selector: `calcite-tile`,
targetProp: "color",
},
},
);
});
describe("single selection", () => {
themed(
html`
<calcite-tile
heading="Tile heading lorem ipsum"
description="Leverage agile frameworks to provide a robust synopsis for high level overviews."
icon="layers"
interactive
selection-mode="single"
>
</calcite-tile>
`,
{
"--calcite-tile-selection-icon-color": {
shadowSelector: `calcite-icon.${CSS.selectionIcon}`,
targetProp: "--calcite-icon-color",
},
"--calcite-tile-selection-icon-color-hover": {
shadowSelector: `calcite-icon.${CSS.selectionIcon}`,
targetProp: "--calcite-icon-color",
state: "hover",
},
},
);
});
});
});
88 changes: 53 additions & 35 deletions packages/calcite-components/src/components/tile/tile.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,49 @@
*
* @prop --calcite-tile-background-color: Specifies the background color of the component.
* @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.
Copy link
Contributor

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.

Copy link
Contributor Author

@eriklharper eriklharper Oct 3, 2024

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-color with --calcite-tile-text-color which effectively serves as like the "body" default text color, and because it is applied to the .container div host element instead of the .description div, it will set a color for the whole content of the tile, including icons and other content.

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.

Copy link
Contributor

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).

* @prop --calcite-tile-heading-text-color: Specifies the heading text color of the component.
* @prop --calcite-tile-heading-text-color-hover: Specifies the heading text color of the component on hover.
* @prop --calcite-tile-href-icon-color-hover: Specifies the icon color of the component when using a href on hover.
* @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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion:
replace all three icon tokens and the "tile-description" with
--calcite-tile-text-color and --calcite-tile-color-selected which covers the states and retains the overall design pattern

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would a user override the icon color separately from the title or description text? Just --calcite-icon-color? Wouldn't it be more expected to use --calcite-tile-icon-color for the display icon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Tile now uses an actual calcite-icon for the radio and checkbox UI, the meaning of "icon" easily generates confusion too because there's the "selection" icon (radio or checkbox) and the "content" icon that displays above the heading. @macandcheese which icon are you referring to here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.
Per some other discussions, like a more general "close-button" pattern I think there is a more global "selection-icon" pattern that we could use. As for the tile-icon, what about something like the following?

calcite-icon {
  color: var(--calcite-title-icon-color, var(--calcite-icon-color, currentColor));
}

Copy link
Contributor

@macandcheese macandcheese Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.selection-icon-example-class {
  color: var(--calcite-title-selection-icon-color, var(--calcite-selection-icon-color, “whatever underlying default”));
}

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

.selection-icon-example-class {
  color: var(--calcite-title-selection-icon-color, var(--calcite-selection-icon-color, “whatever underlying default”));
}

looks good.

Copy link
Contributor Author

@eriklharper eriklharper Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I reading that right? Should it be --calcite-tile-icon-color ?

* @prop --calcite-tile-shadow: Specifies the shadow around the component.
* @prop --calcite-tile-text-color: Specifies the text color of the component.
*/

:host {
--calcite-tile-background-color: var(--calcite-color-foreground-1);
--calcite-tile-border-color: var(--calcite-color-border-2);
--calcite-tile-description-text-color: var(--calcite-color-text-3);
--calcite-tile-heading-text-color: var(--calcite-color-text-2);
--calcite-icon-color: var(--calcite-color-text-3);
/** Internal variables
* --calcite-internal-tile-selection-icon-color
*/

:host {
box-shadow: var(--calcite-tile-shadow, var(--calcite-shadow-none));
box-sizing: border-box;
color: var(--calcite-tile-text-color, var(--calcite-color-text-3));
display: inline-block;
}

.container {
background-color: var(--calcite-tile-background-color);
--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-tile-border-color);
outline: var(--calcite-border-width-sm, 1px) solid var(--calcite-tile-border-color, var(--calcite-color-border-2));
user-select: none;

&.interactive {
cursor: pointer;

&:hover,
&:focus,
&.selected {
outline-color: var(--calcite-color-brand);
--calcite-internal-tile-select-icon-color: var(
--calcite-tile-selection-icon-color-hover,
var(--calcite-color-brand)
);
position: relative;
.selection-icon {
--calcite-icon-color: var(--calcite-color-brand);
}
}
&.selected {
z-index: var(--calcite-z-index);
Expand Down Expand Up @@ -70,7 +81,7 @@
}

.heading {
color: var(--calcite-tile-heading-text-color);
color: var(--calcite-tile-heading-text-color, var(--calcite-color-text-2));
font-weight: var(--calcite-font-weight-medium);
line-height: 1.20313rem;
overflow-wrap: break-word;
Expand All @@ -94,25 +105,26 @@
}

.description {
color: var(--calcite-tile-description-text-color);
font-weight: var(--calcite-font-weight-regular);
overflow-wrap: break-word;
}

calcite-icon {
color: var(--calcite-tile-icon-color, var(--calcite-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-container {
justify-content: center;
}
.text-content {
text-align: center;
}
slot[name="content-start"]::slotted(*),
slot[name="content-end"]::slotted(*),
slot[name="content-top"]::slotted(*),
slot[name="content-bottom"]::slotted(*) {
slot[name="content-end"]::slotted(*) {
align-self: center;
}
}
Expand Down Expand Up @@ -202,43 +214,56 @@

:host([selection-appearance="border"][layout="horizontal"]) {
.container.selected {
// TODO: Add a component design token here?
box-shadow: inset 0 -4px 0 0 var(--calcite-color-brand);
}
}

:host([selection-appearance="border"][layout="vertical"]) {
.container.selected {
// TODO: Add a component design token here?
box-shadow: inset 4px 0 0 0 var(--calcite-color-brand);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

}
}

: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-tile-border-color: var(--calcite-color-text-link);
--calcite-tile-heading-text-color: var(--calcite-color-text-link);
--calcite-icon-color: var(--calcite-color-text-link);
outline-color: var(--calcite-color-text-link);
calcite-icon {
color: var(--calcite-tile-href-icon-color-hover, var(--calcite-color-text-link));
}
.container {
position: relative;
z-index: var(--calcite-z-index);
}
.heading {
color: var(--calcite-color-text-link);
}
}

:host([href]:active:not([disabled])) {
.container {
box-shadow: 0 0 0 3px var(--calcite-tile-border-color);
:host(:hover:not([disabled])),
:host([active]:not([disabled])) {
color: var(--calcite-tile-text-color, --calcite-color-text-2);
.heading {
color: var(--calcite-tile-heading-text-color-hover, var(--calcite-color-text-1));
}
}

:host([embed]) {
.container {
box-shadow: none;
padding: 0;
}
}

:host([selection-mode="none"]) {
.container {
outline-color: var(--calcite-tile-border-color);
outline-color: var(--calcite-tile-border-color, var(--calcite-color-border-2));
&:focus {
outline-color: var(--calcite-color-brand);
position: relative;
Expand All @@ -247,13 +272,6 @@
}

@include disabled();

:host(:hover:not([disabled])),
:host([active]:not([disabled])) {
--calcite-tile-description-text-color: var(--calcite-color-text-2);
--calcite-tile-heading-text-color: var(--calcite-color-text-1);
}

@include base-component();

::slotted(*) {
Expand Down
5 changes: 4 additions & 1 deletion packages/calcite-components/src/custom-theme.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { slider } from "./custom-theme/slider";
import { tabs } from "./custom-theme/tabs";
import { textArea, textAreaTokens } from "./custom-theme/text-area";
import { avatarIcon, avatarInitials, avatarThumbnail, avatarTokens } from "./custom-theme/avatar";
import { tileTokens, tile } from "./custom-theme/tile";

const globalTokens = {
calciteColorBrand: "#007ac2",
Expand Down Expand Up @@ -115,7 +116,7 @@ const kitchenSink = (args: Record<string, string>, useTestValues = false) =>
<div>${checkbox}</div>
${chips} ${pagination} ${slider}
</div>
<div class="demo-column">${datePicker} ${tabs} ${loader} ${calciteSwitch} ${avatarIcon} ${avatarInitials} ${avatarThumbnail} ${progress} ${handle} ${textArea} ${popover}</div>
<div class="demo-column">${datePicker} ${tabs} ${loader} ${calciteSwitch} ${avatarIcon} ${avatarInitials} ${avatarThumbnail} ${progress} ${handle} ${textArea} ${popover} ${tile}</div>
${alert}
</div>
</div>
Expand All @@ -142,6 +143,7 @@ export default {
...progressTokens,
...inputTokens,
...textAreaTokens,
...tileTokens,
},
};

Expand Down Expand Up @@ -169,6 +171,7 @@ export const theming = (): string => {
...progressTokens,
...inputTokens,
...textAreaTokens,
...tileTokens,
},
true,
);
Expand Down
22 changes: 22 additions & 0 deletions packages/calcite-components/src/custom-theme/tile.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { html } from "../../support/formatting";

export const tileTokens = {
calciteTileBackgroundColor: "",
calciteTileBorderColor: "",
calciteTileHeadingTextColor: "",
calciteTileIconColor: "",
calciteTileSelectionIconColor: "",
calciteTileSelectionIconColorHover: "",
calciteTileShadow: "",
calciteTileTextColor: "",
calciteTileTextColorHover: "",
};

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>
`;
Loading