Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
11 changes: 11 additions & 0 deletions .changeset/two-clouds-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@salt-ds/core": patch
---

Added support for read-only `Switch` via `readOnly` prop.

Example usage:

```jsx
<Switch readOnly label="Read only" />
```
17 changes: 17 additions & 0 deletions packages/core/src/__tests__/__e2e__/switch/Switch.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,21 @@ describe("GIVEN a Switch", () => {
cy.findByRole("switch").should("be.focused");
cy.findByRole("switch").should("be.checked");
});

describe("WHEN readOnly is true", () => {
it("THEN should not toggle if clicked", () => {
cy.mount(<Switch readOnly />);
cy.findByRole("switch").should("not.be.checked");
cy.findByRole("switch").realClick();
cy.findByRole("switch").should("not.be.checked");
});

it("THEN should not toggle when pressing the Space key", () => {
cy.mount(<Switch readOnly />);
cy.findByRole("switch").should("not.be.checked");
Copy link
Contributor

Choose a reason for hiding this comment

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

no test of focus but it's something we call out as a differentiator between disabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

read-only attribute as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point thanks, I've included them in the tests

cy.realPress("Tab");
cy.realPress("Space");
cy.findByRole("switch").should("not.be.checked");
});
});
});
30 changes: 30 additions & 0 deletions packages/core/src/switch/Switch.css
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,32 @@
width: calc((var(--salt-size-selectable) + var(--salt-spacing-25) + var(--salt-size-fixed-100)) * 2);
}

.saltSwitch-readOnly {
cursor: var(--salt-cursor-text);
}

.saltSwitch-readOnly .saltSwitch-track,
.saltSwitch-readOnly:hover .saltSwitch-track {
border-color: var(--salt-selectable-borderColor-readonly);
color: var(--salt-content-primary-foreground);
}

.saltSwitch-readOnly:hover .saltSwitch-thumb {
border-color: var(--salt-selectable-borderColor-readonly);
}

.saltSwitch-readOnly .saltSwitch-thumb,
.saltSwitch-readOnly.saltSwitch-checked .saltSwitch-thumb {
background: var(--salt-container-primary-background);
border: var(--salt-size-fixed-100) var(--salt-borderStyle-dashed) var(--salt-selectable-borderColor-readonly);
}

.saltSwitch-readOnly .saltSwitch-input:focus-visible + .saltSwitch-track,
.saltSwitch-readOnly .saltSwitch-input:focus-visible + .saltSwitch-track .saltSwitch-thumb {
border-color: var(--salt-selectable-borderColor-readonly);
color: var(--salt-content-primary-foreground);
}

@media (prefers-reduced-motion) {
.saltSwitch-input:focus-visible + .saltSwitch-track .saltSwitch-thumb,
.saltSwitch:hover .saltSwitch-thumb {
Expand All @@ -121,6 +147,10 @@
border: 0;
}

.salt-density-high .saltSwitch-readOnly .saltSwitch-thumb {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume it's to do with sizing, could you check with the designers if what is there meets the expected outcome please

Copy link
Contributor

Choose a reason for hiding this comment

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

Figma doesn't have a stroke on the switch thumb in all the densities. Read-only states look correct though.

border: var(--salt-size-fixed-100) var(--salt-borderStyle-dashed) var(--salt-selectable-borderColor-readonly);
}

/* Styles applied when switch is inside a form field with label left or right, ensuring alignment with base / input height */
.saltFormField-labelRight .saltSwitch,
.saltFormField-labelLeft .saltSwitch {
Expand Down
25 changes: 20 additions & 5 deletions packages/core/src/switch/Switch.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CheckmarkSolidIcon } from "@salt-ds/icons";
import { CheckmarkIcon, CheckmarkSolidIcon } from "@salt-ds/icons";
import { useComponentCssInjection } from "@salt-ds/styles";
import { useWindow } from "@salt-ds/window";
import { clsx } from "clsx";
Expand Down Expand Up @@ -61,6 +61,10 @@ export interface SwitchProps
* The value of the switch.
*/
value?: string;
/**
* If `true`, the switch will be read-only.
*/
readOnly?: boolean;
}

const withBaseName = makePrefixer("saltSwitch");
Expand All @@ -79,6 +83,7 @@ export const Switch = forwardRef<HTMLLabelElement, SwitchProps>(
onChange,
onFocus,
value,
readOnly: readOnlyProp,
...rest
} = props;

Expand All @@ -104,14 +109,18 @@ export const Switch = forwardRef<HTMLLabelElement, SwitchProps>(
state: "checked",
});

const { a11yProps: formFieldA11yProps, disabled: formFieldDisabled } =
useFormFieldProps();
const {
a11yProps: formFieldA11yProps,
disabled: formFieldDisabled,
readOnly: formFieldReadOnly,
} = useFormFieldProps();

const disabled = formFieldDisabled || disabledProp;
const readOnly = formFieldReadOnly || readOnlyProp;

const handleChange: ChangeEventHandler<HTMLInputElement> = (event) => {
// Workaround for https://github.com/facebook/react/issues/9023
if (event.nativeEvent.defaultPrevented) {
if (event.nativeEvent.defaultPrevented || readOnly) {
return;
}

Expand All @@ -128,13 +137,15 @@ export const Switch = forwardRef<HTMLLabelElement, SwitchProps>(
{
[withBaseName("disabled")]: disabled,
[withBaseName("checked")]: checked,
[withBaseName("readOnly")]: readOnly,
},
className,
)}
ref={ref}
{...rest}
>
<input
aria-readonly={readOnly || undefined}
aria-describedby={clsx(
formFieldA11yProps?.["aria-describedby"],
inputDescribedBy,
Expand All @@ -149,6 +160,7 @@ export const Switch = forwardRef<HTMLLabelElement, SwitchProps>(
className={clsx(withBaseName("input"), inputClassName)}
defaultChecked={defaultChecked}
disabled={disabled}
readOnly={readOnly}
onBlur={onBlur}
onChange={handleChange}
onFocus={onFocus}
Expand All @@ -159,12 +171,15 @@ export const Switch = forwardRef<HTMLLabelElement, SwitchProps>(
/>
<span className={withBaseName("track")}>
<span className={withBaseName("thumb")}>
{checked && (
{checked && !readOnly && (
<CheckmarkSolidIcon
aria-hidden
className={withBaseName("icon")}
/>
)}
{checked && readOnly && (
<CheckmarkIcon aria-hidden className={withBaseName("icon")} />
)}
</span>
</span>
{label && <span className={withBaseName("label")}>{label}</span>}
Expand Down
9 changes: 8 additions & 1 deletion packages/core/stories/switch/switch.qa.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const AllExamplesGrid: StoryFn<
> = (props) => {
const { className } = props;
return (
<QAContainer cols={4} {...props}>
<QAContainer cols={6} {...props}>
<Switch className={className} label="Default" />
<Switch className={className} checked label="Checked" />
<Switch className={className} disabled label="Disabled" />
Expand All @@ -28,6 +28,13 @@ export const AllExamplesGrid: StoryFn<
disabled
label="Checked + Disabled"
/>
<Switch
className={className}
checked
readOnly
label="Checked + Readonly"
/>
<Switch className={className} readOnly label="Readonly" />
</QAContainer>
);
};
Expand Down
7 changes: 7 additions & 0 deletions packages/core/stories/switch/switch.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,10 @@ export const WithFormField: StoryFn<typeof Switch> = (args) => {
</StackLayout>
);
};

export const Readonly: StoryFn<typeof Switch> = () => (
<StackLayout>
<Switch readOnly checked label="Read only + checked" />
<Switch readOnly label="Read only" />
</StackLayout>
);
4 changes: 4 additions & 0 deletions site/docs/components/switch/accessibility.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ data:
$ref: ./#/data
---

## Best Practice

Ideally we shouln't use readOnly
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to talk more about why we shouldn't? We included a note in the example section, wondering if we should move it here? @karlgoldstraw

Copy link
Contributor

Choose a reason for hiding this comment

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

@wemmyo I was walking Karl through how to edit the docs. I believe he might add more information to this but I'll take a look as well. Also, I did make a slight change to the read only note and tried to align it with what we are using elsewhere (Checkbox for example). Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sounds good thanks!


## Keyboard interactions

<KeyboardControls>
Expand Down
10 changes: 10 additions & 0 deletions site/docs/components/switch/examples.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ Use a disabled checked state for switches that have permissions, dependencies or

<LivePreview componentName="switch" exampleName="DisabledChecked" />

## Read-only

A read-only switch allows the user to select and copy its text, but does not allow them to change its state. You can set a switch as read-only via the prop `readOnly`.

### Best practices

Unlike disabled switches, read-only switches are focusable and shown in the tab order when using a keyboard, allowing users to interact with its text. Use a read-only Switch when its text contains information that is valuable to the user.

<LivePreview componentName="switch" exampleName="Readonly" />

Copy link
Contributor

Choose a reason for hiding this comment

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

No guidance currently on the accessibility tab.
Feels like accessibility info is on this page instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From @jake-costa's comment earlier @karlgoldstraw might be adding more here but we thought the note in the examples section might be beneficial. We can move it to the accessibility tab if that's better

Copy link
Contributor

Choose a reason for hiding this comment

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

@wemmyo @mark-tate In addition, we have this kind of language on other read only examples (checkbox, radio, etc.)

## Left aligned label

Use a form field if you need a left aligned label, avoid also having a switch label.
Expand Down
9 changes: 9 additions & 0 deletions site/src/examples/switch/Readonly.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { StackLayout, Switch } from "@salt-ds/core";
import type { ReactElement } from "react";

export const Readonly = (): ReactElement => (
<StackLayout>
<Switch readOnly checked label="Read only + checked" />
<Switch readOnly label="Read only" />
</StackLayout>
);
1 change: 1 addition & 0 deletions site/src/examples/switch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from "./DefaultChecked";
export * from "./Disabled";
export * from "./DisabledChecked";
export * from "./LeftAlignedLabel";
export * from "./Readonly";
Loading