Skip to content
Open
Show file tree
Hide file tree
Changes from all 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" />
```
28 changes: 28 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,32 @@ 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");
});

it("THEN should have aria-readonly attribute", () => {
cy.mount(<Switch readOnly />);
cy.findByRole("switch").should("have.attr", "aria-readonly", "true");
});

it("THEN should be focusable", () => {
cy.mount(<Switch readOnly />);
cy.realPress("Tab");
cy.findByRole("switch").should("be.focused");
});
});
});
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>
);
12 changes: 12 additions & 0 deletions site/docs/components/switch/examples.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ 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`.

**Note:** We recommend avoiding read-only switches due to the lack of adequate `aria-readonly` support and conflicting user expectations. If you must use a read-only switch, you can add visual or non-visual labels to make its state clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copying and pasting a label does not seem to be of that much value but that's how we are selling read-only.

What is more valuable is a focusable control being announced by the screen reader, whilst a disabled control cannot be focused and so will not be seen by the screenreader.

  • Lack of adequate support for aria-readonly, we are not clear enough hereon the impact, I guess you browser and screenreader combination.

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 believe generally we're not trying to sell the readonly switch because of it's limitations including limited support for screen readers, but it's something we have just to be consistent with similar components. Guidance here is similar to what we have for checkbox but @karlgoldstraw and @jake-costa can help expand on that

### 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
12 changes: 12 additions & 0 deletions site/src/examples/switch/Readonly.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { FormField, FormFieldLabel, StackLayout, Switch } from "@salt-ds/core";
import type { ReactElement } from "react";

export const Readonly = (): ReactElement => (
<StackLayout>
<FormField>
<FormFieldLabel>Reminders (Read-only)</FormFieldLabel>
<Switch readOnly label="Off" />
</FormField>
<Switch readOnly checked label="Reminders" />
</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