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.

Sign up for GitHub

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(list, list-item, list-item-group): add component tokens #10669

Merged
merged 12 commits into from
Nov 20, 2024
Merged

feat(list, list-item, list-item-group): add component tokens #10669

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d7d1467
WIP
driskull Oct 30, 2024
fee41eb
WIP
driskull Oct 31, 2024
e8410b9
cleanup
driskull Oct 31, 2024
d678ee7
fix tokens
driskull Oct 31, 2024
7f8c748
Merge branch 'dev' into dris0000/list-tokens-7180
driskull Nov 1, 2024
429b7b3
list item review fixes
driskull Nov 4, 2024
b528880
Merge branch 'dev' into dris0000/list-tokens-7180
driskull Nov 4, 2024
a753361
Update .stylelintrc.cjs
driskull Nov 4, 2024
36c10cb
cleanup
driskull Nov 4, 2024
6a6ab2d
Merge branch 'dev' into dris0000/list-tokens-7180
driskull Nov 15, 2024
3effa15
rename tokens
driskull Nov 19, 2024
4b18fee
Merge branch 'dev' into dris0000/list-tokens-7180
driskull Nov 19, 2024
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
8 changes: 1 addition & 7 deletions packages/calcite-components/.stylelintrc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
// @ts-check

// ⚠️ AUTO-GENERATED CODE - DO NOT EDIT
const customFunctions = [
"get-trailing-text-input-padding",
"medium-modular-scale",
"modular-scale",
"scale-duration",
"small-modular-scale",
];
const customFunctions = [];
// ⚠️ END OF AUTO-GENERATED CODE

const scssPatternRules = [
Expand Down
19 changes: 18 additions & 1 deletion packages/calcite-components/src/components/list-item-group/list-item-group.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { hidden, renders, disabled, defaults } from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { hidden, renders, disabled, defaults, themed } from "../../tests/commonTests";
import { CSS } from "./resources";

describe("calcite-list-item-group", () => {
describe("renders", () => {
Expand Down Expand Up @@ -33,4 +35,19 @@ describe("calcite-list-item-group", () => {
},
]);
});

describe("themed", () => {
describe("default", () => {
themed(html`<calcite-list-item-group heading="Buildings"></calcite-list-item-group>`, {
"--calcite-list-background-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
},
"--calcite-list-item-group-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "color",
},
});
});
});
});
15 changes: 13 additions & 2 deletions packages/calcite-components/src/components/list-item-group/list-item-group.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
/**
* CSS Custom Properties
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-list-background-color: Specifies the component's background color.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d expect to support “—calcite-list-item-background-color” as well.

Consider this use case which is representative of how most of our users want to theme: https://community.esri.com/t5/calcite-design-system-questions/change-listitem-background-color-in-code/m-p/1557125#M767

Copy link
Contributor

Choose a reason for hiding this comment

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

We are limiting tokens when shared between grouped components. This is documented in the wiki.

Copy link
Contributor

Choose a reason for hiding this comment

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

How is this different than Accordion Item / Accordion - where we have -item-background-color? #10181

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I was wondering this as well. There are some inconsistencies here

Copy link
Member Author

Choose a reason for hiding this comment

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

@alisonailea can you comment on the above? Currently, it seems like parent/child tokens are inconsistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

The “item tokens” path seems the easiest to follow both as a maintainer (IMO) and as someone who uses these theming options frequently day to day.

Using the Community post as just one example, it also seems to align with expectations from external folks - theming the element you want to adjust with component-specific nomenclature / rules.

Copy link
Member Author

Choose a reason for hiding this comment

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

Need direction on this one. @alisonailea

* @prop --calcite-list-item-group-color: Specifies the component's color.
*/

:host {
@apply flex flex-col bg-foreground-1;
@apply flex flex-col;
}

:host([filter-hidden]) {
Expand All @@ -9,7 +18,9 @@
@include disabled();

.container {
@apply text-n1 text-color-2 m-0 flex flex-1 p-3 font-medium;
@apply text-n1 m-0 flex flex-1 p-3 font-medium;
background-color: var(--calcite-list-background-color, var(--calcite-color-foreground-1));
color: var(--calcite-list-item-group-color, var(--calcite-color-text-2));
}

.heading {
Expand Down
75 changes: 74 additions & 1 deletion packages/calcite-components/src/components/list-item/list-item.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { newE2EPage } from "@stencil/core/testing";
import { defaults, disabled, focusable, hidden, reflects, renders, slots } from "../../tests/commonTests";
import { defaults, disabled, focusable, hidden, reflects, renders, slots, themed } from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { CSS, SLOTS } from "./resources";

Expand Down Expand Up @@ -371,4 +371,77 @@ describe("calcite-list-item", () => {
expect(await listItem.getProperty("open")).toBe(false);
expect(calciteListItemToggle).toHaveReceivedEventTimes(2);
});

describe("themed", () => {
describe(`selection-appearance="icon"`, () => {
themed(
html`<calcite-list-item
selected
label="Park offices"
interaction-mode="interactive"
description="Home base for park staff to converse with visitors."
value="offices"
bordered
selection-mode="single"
selection-appearance="icon"
></calcite-list-item>`,
{
"--calcite-list-background-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
},
"--calcite-list-background-color-hover": {
shadowSelector: `.${CSS.container}`,
state: "hover",
targetProp: "backgroundColor",
},
"--calcite-list-background-color-press": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
state: { press: { attribute: "class", value: CSS.content } },
},
"--calcite-list-item-border-color": {
shadowSelector: `.${CSS.wrapper}`,
targetProp: "borderBlockEndColor",
},
"--calcite-list-item-content-text-color": {
shadowSelector: `.${CSS.contentContainer}`,
targetProp: "color",
},
"--calcite-list-item-description-text-color": {
shadowSelector: `.${CSS.description}`,
targetProp: "color",
},
"--calcite-list-item-icon-color": {
shadowSelector: `.${CSS.selectionContainer}`,
targetProp: "color",
},
"--calcite-list-item-label-text-color": {
shadowSelector: `.${CSS.label}`,
targetProp: "color",
},
},
);
});
describe(`selection-appearance="border"`, () => {
themed(
html`<calcite-list-item
selected
label="Park offices"
description="Home base for park staff to converse with visitors."
interaction-mode="interactive"
value="offices"
bordered
selection-mode="single"
selection-appearance="border"
></calcite-list-item>`,
{
"--calcite-list-item-selection-border-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "borderInlineStartColor",
},
},
);
});
});
});
60 changes: 38 additions & 22 deletions packages/calcite-components/src/components/list-item/list-item.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
/**
* CSS Custom Properties
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-list-background-color-hover: Specifies the component's background color when hovered.
* @prop --calcite-list-background-color-press: Specifies the component's background color when pressed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to above, does this need to be “list-item” - what hover would we actually be theming here on the parent?

Copy link
Contributor

Choose a reason for hiding this comment

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

we are limiting tokens when shared between grouped components. This is documented in the wiki.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it is a bit weird to be targeting the parent hovering when its really the items being hovered.

* @prop --calcite-list-background-color: Specifies the component's background color.
* @prop --calcite-list-item-border-color: Specifies the component's border color.
* @prop --calcite-list-item-content-text-color: Specifies the content color.
* @prop --calcite-list-item-description-text-color: Specifies the description color.
* @prop --calcite-list-item-icon-color: Specifies the component's icon color.
* @prop --calcite-list-item-label-text-color: Specifies the label color.
* @prop --calcite-list-item-selection-border-color: Specifies the component's selection border color.
*/

:host {
@apply flex flex-col bg-foreground-1;
--calcite-list-item-icon-color: theme("colors.brand");
@apply flex flex-col;
}

:host([filter-hidden]),
Expand All @@ -9,28 +24,29 @@
}

.wrapper--bordered {
border-block-end: 1px solid var(--calcite-color-border-3);
border-block-end: 1px solid var(--calcite-list-item-border-color, var(--calcite-color-border-3));
}

@include disabled();

.container {
@apply bg-foreground-1
box-border
@apply box-border
flex
flex-1
overflow-hidden;
background-color: var(--calcite-list-background-color, var(--calcite-color-foreground-1));
* {
@apply box-border;
}
}

.container--hover:hover {
@apply bg-foreground-2 cursor-pointer;
@apply cursor-pointer;
background-color: var(--calcite-list-background-color-hover, var(--calcite-color-foreground-2));
}

.container:active {
@apply bg-foreground-1;
background-color: var(--calcite-list-background-color-press, var(--calcite-color-foreground-1));
}

.container--border {
Expand All @@ -39,7 +55,7 @@
}

.container--border-selected {
border-inline-start-color: theme("colors.brand");
border-inline-start-color: var(--calcite-list-item-selection-border-color, var(--calcite-color-brand));
}

.container--border-unselected {
Expand All @@ -48,16 +64,15 @@

.container:hover {
&.container--border-unselected {
@apply border-color-1;
border-inline-start-color: var(--calcite-list-item-selection-border-color, var(--calcite-color-border-1));
}
}

.nested-container {
@apply hidden flex-col
border-solid
border-0
border-color-3;

border-0;
border-color: 1px solid var(--calcite-list-item-border-color, var(--calcite-color-border-3));
margin-inline-start: var(--calcite-list-item-spacing-indent, theme("spacing.6"));
}

Expand All @@ -66,13 +81,13 @@
}

.content-container {
@apply text-color-2
select-none
@apply select-none
flex
flex-auto
font-normal
items-stretch
p-0;
color: var(--calcite-list-item-content-text-color, var(--calcite-color-text-2));
}

.content-container--unavailable {
Expand Down Expand Up @@ -114,19 +129,20 @@
}

.label {
@apply text-color-1;
color: var(--calcite-list-item-label-text-color, var(--calcite-color-text-1));
}

:host([selected]) .label {
@apply font-medium;
}

.description {
@apply text-color-3 mt-0.5;
@apply mt-0.5;
color: var(--calcite-list-item-description-text-color, var(--calcite-color-text-3));
}

:host([selected]) .description {
@apply text-color-2;
color: var(--calcite-list-item-description-text-color, var(--calcite-color-text-2));
}

.content-start {
Expand Down Expand Up @@ -157,7 +173,7 @@

.selection-container {
@apply flex py-0;
color: theme("borderColor.color.input");
color: var(--calcite-list-item-icon-color, var(--calcite-color-border-input));
padding-inline: var(--calcite-spacing-md) var(--calcite-spacing-xxs);
}

Expand All @@ -166,21 +182,21 @@
}

:host(:not([disabled]):not([selected])) .container:hover .selection-container--single {
color: theme("borderColor.color.1");
color: var(--calcite-list-item-icon-color, var(--calcite-color-border-1));
}

:host([selected]:hover) .selection-container,
:host([selected]:hover) .selection-container--single,
:host([selected]) .selection-container {
color: var(--calcite-list-item-icon-color);
color: var(--calcite-list-item-icon-color, var(--calcite-color-brand));
}

.open-container {
color: theme("textColor.color.3");
color: var(--calcite-list-item-icon-color, var(--calcite-color-text-3));
}

:host(:not([disabled])) .container:hover .open-container {
color: theme("textColor.color.1");
color: var(--calcite-list-item-icon-color, var(--calcite-color-text-1));
}

.actions-start,
Expand Down
14 changes: 13 additions & 1 deletion packages/calcite-components/src/components/list/list.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { E2EPage, newE2EPage } from "@stencil/core/testing";
import { accessible, hidden, renders, focusable, disabled, defaults, t9n } from "../../tests/commonTests";
import { accessible, hidden, renders, focusable, disabled, defaults, t9n, themed } from "../../tests/commonTests";
import { placeholderImage } from "../../../.storybook/placeholder-image";
import { html } from "../../../support/formatting";
import { CSS as ListItemCSS, activeCellTestAttribute } from "../list-item/resources";
import { GlobalTestProps, dragAndDrop, isElementFocused, getFocusedElementProp } from "../../tests/utils";
import { DEBOUNCE } from "../../utils/resources";
import { Reorder } from "../sort-handle/interfaces";
import { ListDragDetail } from "./interfaces";
import { CSS } from "./resources";

const placeholder = placeholderImage({
width: 140,
Expand Down Expand Up @@ -1574,4 +1575,15 @@ describe("calcite-list", () => {
await assertMove("three", "list2", "list1", ["three", "two"], ["one"], 0, 1);
});
});

describe("themed", () => {
describe("default", () => {
themed(html`calcite-list`, {
"--calcite-list-background-color": {
shadowSelector: `.${CSS.container}`,
targetProp: "backgroundColor",
},
});
});
});
});
13 changes: 11 additions & 2 deletions packages/calcite-components/src/components/list/list.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
/**
* CSS Custom Properties
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-list-background-color: Specifies the component's background color.
*/

:host {
@apply block;
}
Expand All @@ -6,6 +14,7 @@

.container {
position: relative;
background-color: var(--calcite-list-background-color, var(--calcite-color-foreground-1));
}

.table-container {
Expand All @@ -31,8 +40,8 @@
.sticky-pos {
@apply sticky
top-0
z-sticky
bg-foreground-1;
z-sticky;
background-color: var(--calcite-list-background-color, var(--calcite-color-foreground-1));
}

.assistive-text {
Expand Down
Loading
Loading