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

Jump to bottom

feat(chip): add component tokens #10179

Merged
merged 20 commits into from
Sep 20, 2024
+400 −360
Merged

feat(chip): add component tokens #10179

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9507e9a
feat(chip): refactor chip for component tokens
alisonailea Aug 27, 2024
c6d5f1b
test(chip): add component tokens to stories
alisonailea Aug 28, 2024
a7ddcc2
fix(chip): remove mouse state tokens
alisonailea Aug 28, 2024
b226cb9
test(chip): update e2e tests
alisonailea Aug 28, 2024
b052c8d
feat(chip-group): add component tokens
alisonailea Aug 28, 2024
865757a
Merge branch 'dev' of github.com:Esri/calcite-design-system into ali-…
alisonailea Aug 28, 2024
3b0ac61
Revert "feat(chip-group): add component tokens"
alisonailea Aug 30, 2024
2332a09
fix(chip): chip element spacing
alisonailea Aug 31, 2024
5fe9b0c
fix(chip): internal variables in selection states
alisonailea Sep 2, 2024
819021c
fix(chip): minor issues with spacing
alisonailea Sep 6, 2024
798c7c4
fix(chip): add chip-icon-color token
alisonailea Sep 10, 2024
f9d0ec0
fix(chip): add select and close token options
alisonailea Sep 10, 2024
b6e6c97
test(chip): simplify theme tests
alisonailea Sep 11, 2024
d9cad9a
test(chip): fix theme setup
alisonailea Sep 11, 2024
be4ac0f
Revert package.json in test(chip): simplify theme tests”
alisonailea Sep 11, 2024
a709177
refactor(chip): restore —calcite-icon-color styles
alisonailea Sep 12, 2024
89fa877
Merge branch 'dev' of github.com:Esri/calcite-design-system into ali-…
alisonailea Sep 12, 2024
a8e74a8
Merge branch 'dev' into astump/7180-chip
alisonailea Sep 13, 2024
9ce58e7
Merge branch 'dev' into astump/7180-chip
alisonailea Sep 16, 2024
fa3f3e7
chore(custom-theme): fix imports
alisonailea Sep 16, 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
Prev Previous commit
Next Next commit
fix(chip): add select and close token options
  • Loading branch information
@alisonailea
alisonailea committed Sep 10, 2024
commit f9d0ec0ebd93621e9dcbdfe439169eaf6267a36b
28 changes: 28 additions & 0 deletions packages/calcite-components/src/components/chip/chip.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,34 @@ describe("calcite-chip", () => {
});
});

describe("closable", () => {
themed(html`<calcite-chip closable>Layers</calcite-chip>`, {
"--calcite-chip-close-icon-color": {
shadowSelector: `.${CSS.close}`,
targetProp: "color",
},
});
});

describe("selectable", () => {
describe("default", () => {
themed(html`<calcite-chip select="single">Layers</calcite-chip>`, {
"--calcite-chip-select-icon-color": {
shadowSelector: `.${CSS.selectIcon}`,
targetProp: "color",
},
});
});
describe("selected", () => {
themed(html`<calcite-chip select="single" selected>Layers</calcite-chip>`, {
"--calcite-chip-select-icon-color-pressed": {
shadowSelector: `.${CSS.selectIcon}`,
targetProp: "color",
},
});
});
});

describe("icon", () => {
themed(html`<calcite-chip icon="layer">Layers</calcite-chip>`, {
"--calcite-chip-icon-color": {
Expand Down
16 changes: 12 additions & 4 deletions packages/calcite-components/src/components/chip/chip.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
*
* @prop --calcite-chip-background-color: Specifies the background color of the component.
* @prop --calcite-chip-border-color: Specifies the border color of the component.
* @prop --calcite-chip-corner-radius: Specifies the border radius of the component.
* @prop --calcite-chip-corner-radius: Specifies the corner radius of the component.
* @prop --calcite-chip-text-color: Specifies the text color of the component.
* @prop --calcite-chip-icon-color: Specifies the icon color of the component.
* @prop --calcite-close-icon-color: Specifies the icon color of the close element of the component.
* @prop --calcite-chip-close-icon-color: Specifies the icon color of the close element of the component.
* @prop --calcite-chip-select-icon-color: Specifies the icon color of the selection element of the component.
* @prop --calcite-chip-select-icon-color-pressed: Specifies the icon color of the selection element of the component when active.
*
Copy link
Contributor

@macandcheese macandcheese Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we'd at the very least want to add --calcite-chip-icon-color and --calcite-chip-selection-icon-color (or whatever name) from the 7180 PR:

calcite-design-system/packages/calcite-components/src/components/chip/chip.scss

Line 10 in 2c5e985

* @prop --calcite-chip-icon-color: Specifies the color of the component's icon property.

We can't just set --calcite-icon-color on a Calcite Chip - it has too many internally rendered icons that would be affected. The 7180 PR also had --calcite-chip-close-icon-color (and, props to control internal close button states, which it sounds like we did not want to pursue - I still think this prevents a lot of valid use cases)

Copy link
Contributor Author

@alisonailea alisonailea Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The design pattern for calcite-chip is for icons to follow the text color. If a user wants to have a special color they can do something like calcite-chip.my-chip { --calicte-icon-color: red }. Similarly. IMO, all close sub-elements (should) have the same interaction pattern with a shared token set. Something like

--calcite-close-color
--calcite-close-background-color-hover
--calcite-close-background-color-press

@SkyeSeitz @ashetland

Copy link
Contributor

@macandcheese macandcheese Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed they should be (ideally) consistently implemented when overridden - but not adding component-specific ones (even if it falls back to the global one) to the component means they won't be documented alongside all the other component tokens - wherever that doc is consumed (including places we can't manually supplement it with info about these generic "close-color" props).

Currently it seems like text-color property here also sets the selection affordance icon as well as the icon-start icon. At the very least I'd like to be able to not have this happen (which is the current behavior):

The current behavior that is available (just overriding the global css prop - changes the affordance as well as the icon, which I don't think is always preferable).

Screenshot 2024-08-28 at 1 57 43 PM

My use case for overriding an icon color is probably to match map symbology or layer display color / graphics (or something like that). So I don't think I'd want the selection icon to change as well:

Screenshot 2024-08-28 at 1 57 54 PM

*/

Expand Down Expand Up @@ -42,7 +44,7 @@
}

.close {
color: var(--calcite-close-icon-color, var(--calcite-color-text-3));
color: var(--calcite-chip-close-icon-color, var(--calcite-close-icon-color, var(--calcite-color-text-3)));
}
}
:host([appearance="outline"]) .container {
Expand Down Expand Up @@ -87,7 +89,7 @@
color: var(--calcite-chip-text-color, var(--calcite-color-text-1));

.close {
color: var(--calcite-close-icon-color, var(--calcite-color-text-3));
color: var(--calcite-chip-close-icon-color, var(--calcite-close-icon-color, var(--calcite-color-text-3)));
}
}

Expand Down Expand Up @@ -367,11 +369,13 @@
transition:
opacity 0.15s ease-in-out,
inline-size 0.15s ease-in-out;
color: var(--calcite-chip-select-icon-color, currentColor);

&.select-icon--active {
position: relative;
visibility: visible;
opacity: 0.5;
color: var(--calcite-chip-select-icon-color-pressed, var(--calcite-chip-select-icon-color, currentColor));
}
}

Expand All @@ -387,6 +391,10 @@
inline-size: var(--calcite-internal-chip-icon-size, 1.5rem /* 24px */);
}

.close {
color: var(--calcite-chip-close-icon-color, var(--calcite-close-icon-color, currentColor));
}

slot[name="image"]::slotted(*) {
@apply rounded-half flex h-full w-full overflow-hidden;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/calcite-components/src/custom-theme/chips.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export const chipTokens = {
calciteChipBorderColor: "",
calciteChipCornerRadius: "",
calciteChipTextColor: "",
calciteChipIconColor: "",
calciteChipCloseIconColor: "",
calciteChipSelectIconColor: "",
calciteChipSelectIconColorPressed: "",
};

export const chips = html`<div>
Expand Down
Loading