feat(handle): add component tokens #8780

Elijbet · 2024-02-19T23:39:38Z

Related Issue: #7180

Summary

Adds the following component tokens:

--calcite-handle-text-color
--calcite-handle-text-color-hover
--calcite-handle-text-color-focus
--calcite-handle-text-color-selected

--calcite-handle-background-color
--calcite-handle-background-color-hover
--calcite-handle-background-color-focus
--calcite-handle-background-color-selected

…nent-tokens-for-handle

alisonailea

Move custom props to host and style based on the host state if you can.

packages/calcite-components/src/components/handle/handle.scss

driskull · 2024-02-21T18:12:51Z

Would be nice to have a story that applies these vars for test coverage.

…nent-tokens-for-handle

packages/calcite-components/src/components/handle/handle.scss

…nent-tokens-for-handle

packages/calcite-components/src/components/handle/handle.scss

…nent-tokens-for-handle

jcfranco

Awesome stuff, @Elijbet! One more round of changes and I think this will be good to land. 🛬

packages/calcite-components/src/components/handle/handle.scss

jcfranco · 2024-02-29T03:59:05Z

packages/calcite-components/src/components/handle/handle.scss

@@ -23,16 +54,23 @@
 :host(:not([disabled])) .handle {
  @apply cursor-move;
  &:hover {
-    @apply bg-foreground-2 text-color-1;
+    color: var(--calcite-handle-icon-color-hover, var(--calcite-color-text-1));
+    background-color: var(--calcite-handle-background-color-hover, var(--calcite-color-foreground-2));
  }
  &:focus {


Not for this PR, but I think the :focus block can be moved outside of the parent block since :focus doesn't apply when components are disabled.

packages/calcite-components/src/components/handle/handle.scss

packages/calcite-components/src/components/handle/handle.stories.ts

jcfranco · 2024-02-29T04:02:00Z

packages/calcite-components/src/components/handle/handle.stories.ts

+
+export const focused_TestOnly = (): string => html` <calcite-handle focused></calcite-handle> `;
+
+export const themedStates_TestOnly = (): string => html`


jcfranco

🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘
🤘🎸🎸🎸🎸🤘🎸🤘🤘🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘🤘🎸🎸🎸🤘🤘🎸🎸🤘🤘🎸🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘🎸🤘
🤘🎸🤘🤘🎸🤘🎸🤘🤘🤘🤘🤘🎸🤘🎸🤘🤘🤘🤘🎸🤘🤘🤘🤘🎸🤘🤘🎸🤘🎸🎸🤘🎸🎸🤘🎸🤘🤘🤘🤘🎸🤘
🤘🎸🎸🎸🎸🤘🎸🤘🤘🎸🤘🤘🎸🤘🎸🎸🎸🤘🤘🤘🎸🎸🤘🤘🎸🤘🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🎸🎸🤘🤘🎸🤘
🤘🎸🤘🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🤘🎸🤘🤘🤘🤘🤘🤘🤘🎸🤘🎸🤘🤘🎸🤘🎸🤘🤘🤘🎸🤘🎸🤘🤘🤘🤘🤘🤘
🤘🎸🤘🤘🎸🤘🤘🎸🤘🤘🤘🎸🤘🤘🎸🎸🎸🎸🤘🎸🎸🎸🤘🤘🤘🎸🎸🤘🤘🎸🤘🤘🤘🎸🤘🎸🎸🎸🎸🤘🎸🤘
🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘🤘

jcfranco · 2024-02-29T18:38:03Z

@alisonailea Would you mind giving this a quick glance? 👀

driskull

🎉

driskull · 2024-02-29T19:17:51Z

packages/calcite-components/src/components/handle/handle.scss

+* These properties can be overridden using the component's tag as selector.
+*
+* @prop --calcite-handle-icon-color: Specifies the icon color of the component.
+* @prop --calcite-handle-icon-color-hover: Specifies the icon color of the component when in hover state.


question: do we prefer the term hover state or hovered? cc @geospatialem

Should this be worded like defines the color of an icon sub-component when hovered inside the component.

Thats how i've seen these worded in other PRs. I guess we'll probably need to do some cleanup/consistency doc checks before we merge the "real" PR?

Maybe we can check the epic PR every so often and fix any inconsistencies in wording/description?

question: do we prefer the term hover state or hovered? cc @geospatialem

Should this be worded like defines the color of an icon sub-component when hovered inside the component.

Great question. The sentence reads a bit weird when coupled with "when hovered". WDYT about another option altogether, and something like "on hover"? For instance:

Specifies the component's icon color on hover.

I like the simplicity of that phrasing.

…nent-tokens-for-handle

Elijbet added 2 commits February 19, 2024 15:17

Merge branch 'main' into elijbet/7180-add-component-tokens-for-handle

77ae096

feat(handle): add component tokens

aee6ea0

Elijbet requested review from benelan, jcfranco, driskull, geospatialem and a team as code owners February 19, 2024 23:39

github-actions bot assigned Elijbet Feb 19, 2024

Elijbet changed the title ~~Elijbet/7180 add component tokens for handle~~ feat(handle): add component tokens Feb 19, 2024

Elijbet marked this pull request as draft February 19, 2024 23:41

Elijbet mentioned this pull request Feb 19, 2024

Design token phase II - Integration across components #7180

Open

alisonailea changed the base branch from main to epic/7180-component-tokens February 20, 2024 21:49

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

33491b2

…nent-tokens-for-handle

Elijbet marked this pull request as ready for review February 21, 2024 18:01

Elijbet added design-tokens Issues requiring design tokens. pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Feb 21, 2024

alisonailea requested changes Feb 21, 2024

View reviewed changes

Elijbet force-pushed the elijbet/7180-add-component-tokens-for-handle branch 2 times, most recently from e7fe8de to 33491b2 Compare February 21, 2024 22:57

Elijbet added 3 commits February 21, 2024 15:00

reassigning states on component host

2c5208e

docs

1555357

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

251fd0b

…nent-tokens-for-handle

driskull reviewed Feb 22, 2024

View reviewed changes

packages/calcite-components/src/components/handle/handle.scss Outdated Show resolved Hide resolved

seperate vars for hover, focus, and selected states

3fbe568

Elijbet marked this pull request as draft February 22, 2024 18:36

Elijbet added 3 commits February 22, 2024 11:25

state token refactors and stories

b621efd

refactor and cleanup

d33a88b

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

2f766ff

…nent-tokens-for-handle

Elijbet marked this pull request as ready for review February 22, 2024 19:49

Elijbet added 2 commits February 25, 2024 14:51

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

a88084f

…nent-tokens-for-handle

cleanup

ffd2227

Elijbet added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Feb 25, 2024

jcfranco requested changes Feb 27, 2024

View reviewed changes

packages/calcite-components/src/components/handle/handle.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/handle/handle.scss Outdated Show resolved Hide resolved

packages/calcite-components/src/components/handle/handle.scss Outdated Show resolved Hide resolved

Elijbet added 6 commits February 28, 2024 17:08

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

620eaf7

…nent-tokens-for-handle

rename icon color and add focus state for background color

49c8a59

add original value as a fallback

a032e62

add mapped sub-component tokens to parent component

3d3b910

block mapped handle sub-component tokens

f24d452

list-item mapped handle sub-component tokens

5e484c2

jcfranco requested changes Feb 29, 2024

View reviewed changes

Elijbet added 2 commits February 29, 2024 08:34

remove public props from the host

e1fc8f0

remove stories testing states

67a8d77

jcfranco approved these changes Feb 29, 2024

View reviewed changes

driskull approved these changes Feb 29, 2024

View reviewed changes

alisonailea approved these changes Feb 29, 2024

View reviewed changes

Elijbet added 2 commits February 29, 2024 17:03

Merge branch 'epic/7180-component-tokens' into elijbet/7180-add-compo…

e9bc1b8

…nent-tokens-for-handle

docs cleanup

8ee73d8

Elijbet merged commit 2a0be81 into epic/7180-component-tokens Mar 1, 2024
5 checks passed

Elijbet deleted the elijbet/7180-add-component-tokens-for-handle branch March 1, 2024 01:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(handle): add component tokens #8780

feat(handle): add component tokens #8780

Elijbet commented Feb 19, 2024 •

edited

Loading

alisonailea left a comment

driskull commented Feb 21, 2024

jcfranco left a comment

jcfranco Feb 29, 2024

jcfranco Feb 29, 2024

jcfranco left a comment

jcfranco commented Feb 29, 2024

driskull left a comment

driskull Feb 29, 2024

driskull Feb 29, 2024

geospatialem Feb 29, 2024

Elijbet Feb 29, 2024


		export const focused_TestOnly = (): string => html` <calcite-handle focused></calcite-handle> `;

		export const themedStates_TestOnly = (): string => html`

feat(handle): add component tokens #8780

feat(handle): add component tokens #8780

Conversation

Elijbet commented Feb 19, 2024 • edited Loading

Summary

alisonailea left a comment

Choose a reason for hiding this comment

driskull commented Feb 21, 2024

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Feb 29, 2024

Choose a reason for hiding this comment

jcfranco Feb 29, 2024

Choose a reason for hiding this comment

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco commented Feb 29, 2024

driskull left a comment

Choose a reason for hiding this comment

driskull Feb 29, 2024

Choose a reason for hiding this comment

driskull Feb 29, 2024

Choose a reason for hiding this comment

geospatialem Feb 29, 2024

Choose a reason for hiding this comment

Elijbet Feb 29, 2024

Choose a reason for hiding this comment

Elijbet commented Feb 19, 2024 •

edited

Loading