Skip to content

feat(switch): enhance component's colors for a11y and usability #11951

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

Merged
merged 8 commits into from
Apr 15, 2025
Merged

feat(switch): enhance component's colors for a11y and usability #11951

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
756f5db
feat(switch): enhance component's colors for a11y and usability
aPreciado88 Apr 14, 2025
116f709
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Apr 14, 2025
b878f6c
feat(switch): update css token descriptions and remove unneeded css
aPreciado88 Apr 15, 2025
4b4f726
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Apr 15, 2025
40131f1
feat(switch): replace hardcoded px value with css var
aPreciado88 Apr 15, 2025
f740256
Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…
aPreciado88 Apr 15, 2025
4812e82
feat(switch): replace border css var with spacing css var
aPreciado88 Apr 15, 2025
3ad5381
feat(switch): replace border css var with spacing css var
aPreciado88 Apr 15, 2025
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
14 changes: 12 additions & 2 deletions packages/calcite-components/src/components/switch/switch.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,10 @@ describe("calcite-switch", () => {
shadowSelector: `.${CSS.track}`,
targetProp: "backgroundColor",
},
"--calcite-switch-border-color": {
"--calcite-switch-background-color-hover": {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add this var to the switch custom theme story tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this, thanks for catching it!

shadowSelector: `.${CSS.track}`,
targetProp: "borderColor",
targetProp: "backgroundColor",
state: "hover",
},
"--calcite-switch-corner-radius": {
shadowSelector: `.${CSS.track}`,
Expand All @@ -163,6 +164,15 @@ describe("calcite-switch", () => {
shadowSelector: `.${CSS.handle}`,
targetProp: "backgroundColor",
},
});
});

describe("deprecated", () => {
themed(html`calcite-switch`, {
"--calcite-switch-border-color": {
shadowSelector: `.${CSS.track}`,
targetProp: "borderColor",
},
"--calcite-switch-handle-border-color": {
shadowSelector: `.${CSS.handle}`,
targetProp: "borderColor",
Expand Down
45 changes: 26 additions & 19 deletions packages/calcite-components/src/components/switch/switch.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-switch-background-color: Specifies the component's background color.
* @prop --calcite-switch-border-color: Specifies the component's border color.
* @prop --calcite-switch-background-color-hover: Specifies the component's background color when hovered or pressed.
* @prop --calcite-switch-border-color: [Deprecated] Specifies the component's border color.
Copy link
Member

Choose a reason for hiding this comment

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

Should we specify any deprecation reasoning or what they should use instead?

Copy link
Member

Choose a reason for hiding this comment

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

We don't have other similar references across our CSS doc, but since the border is no longer applicable, maybe we could go with:

Suggested change
* @prop --calcite-switch-border-color: [Deprecated] Specifies the component's border color.
* @prop --calcite-switch-border-color: [Deprecated] No longer necessary. Specifies the component's border color.

cc @DitwanP

* @prop --calcite-switch-corner-radius: Specifies the component's corner radius.
* @prop --calcite-switch-handle-background-color: Specifies the handle's background color.
* @prop --calcite-switch-handle-border-color: Specifies the handle's border color.
* @prop --calcite-switch-handle-shadow: Specifies the handle's shadow.
* @prop --calcite-switch-handle-border-color: [Deprecated] Specifies the handle's border color.
* @prop --calcite-switch-handle-shadow: [Deprecated] Specifies the handle's shadow.
*/

// sizes
Expand Down Expand Up @@ -69,59 +70,65 @@
relative
box-border
inline-block
border
border-solid
align-top
focus-base;

border-radius: var(--calcite-switch-corner-radius, 9999px);
border-color: var(--calcite-switch-border-color, var(--calcite-color-border-2));
background-color: var(--calcite-switch-background-color, var(--calcite-color-foreground-2));
border-color: var(--calcite-switch-border-color);
background-color: var(--calcite-switch-background-color, var(--calcite-color-border-input));
}

.container:focus .track {
@apply focus-outset;
}

.container:hover .track {
background-color: var(--calcite-switch-background-color-hover, var(--calcite-color-text-3));
}

.handle {
@apply pointer-events-none
absolute
block
border-2
border-solid
transition-all
duration-150
ease-in-out;
inset-block-start: -1px;
inset-inline: -1px theme("inset.auto");
inset-block-start: 2px;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a platform design token that can be used here? Maybe a border width one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this to use --calcite-border-width-md

inset-inline: 2px theme("inset.auto");
Copy link
Member

Choose a reason for hiding this comment

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

can the theme() be replaced with a css var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a css var with auto as a value, but I replaced theme("inset.auto") with just "auto".

background-color: var(--calcite-switch-handle-background-color, var(--calcite-color-foreground-1));
border-color: var(--calcite-switch-handle-border-color, var(--calcite-color-border-input));
border-color: var(--calcite-switch-handle-border-color);
border-radius: var(--calcite-switch-corner-radius, 9999px);
box-shadow: var(--calcite-switch-handle-shadow);
}

:host(:hover:not([disabled])),
:host(:focus:not([disabled])) {
.handle {
border-color: var(--calcite-switch-handle-border-color, var(--calcite-color-brand));
box-shadow: var(--calcite-switch-handle-shadow, inset 0 0 0 1px var(--calcite-color-brand));
border-color: var(--calcite-switch-handle-border-color);
box-shadow: var(--calcite-switch-handle-shadow, inset 0 0 0 0);
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to set box-shadow here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this, thanks for catching it!

}
}

:host([checked]) {
.track {
border-color: var(--calcite-switch-border-color, var(--calcite-color-brand-hover));
border-color: var(--calcite-switch-border-color);
background-color: var(--calcite-switch-background-color, var(--calcite-color-brand));
}
.handle {
border-color: var(--calcite-switch-handle-border-color, var(--calcite-color-brand));
inset-inline: theme("inset.auto") -1px;
border-color: var(--calcite-switch-handle-border-color);
inset-inline: theme("inset.auto") 2px;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a platform design token that can be used here? Maybe a border width one? can the theme() be replaced with a css var?

Copy link
Member

Choose a reason for hiding this comment

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

I think theme("inset.auto") can just be replaced with auto

Copy link
Member

Choose a reason for hiding this comment

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

I think the inset-inline could be calc(var(--calcite-spacing-base) * -1)

}

&:host(:hover:not([disabled])) {
.track {
background-color: var(--calcite-switch-background-color-hover, var(--calcite-color-brand-hover));
}
}

&:host(:hover:not([disabled])) {
.handle {
border-color: var(--calcite-switch-handle-border-color, var(--calcite-color-brand-hover));
box-shadow: var(--calcite-switch-handle-shadow, inset 0 0 0 1px var(--calcite-color-brand-hover));
border-color: var(--calcite-switch-handle-border-color);
box-shadow: var(--calcite-switch-handle-shadow, inset 0 0 0 0);
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to set box-shadow here?

}
}
}
Expand Down