Skip to content

Commit

Permalink
fix(radio): correct display of read-only state (#3350)
Browse files Browse the repository at this point in the history
  • Loading branch information
5t3ph authored Nov 12, 2024
1 parent 5d2cc5d commit c091b4d
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .changeset/hot-timers-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@spectrum-css/radio": patch
---

Corrects the styles of the read-only state to show the radio inputs and allow visible focus. Also adds `aria-disabled` since `aria-readonly` isn't well supported, and story demonstrates scripting to make selection for read-only radios immutable.
12 changes: 9 additions & 3 deletions components/fieldgroup/stories/fieldgroup.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,17 @@ An invalid group of radio buttons or checkboxes is signified by negative help te

<Canvas of={FieldGroupStories.HorizontalSideLabelCheckbox} />

### Read-only checkboxes
### Read-only Checkbox

<Description of={FieldGroupStories.ReadOnly} />
<Description of={FieldGroupStories.ReadOnlyCheckbox} />

<Canvas of={FieldGroupStories.ReadOnly} />
<Canvas of={FieldGroupStories.ReadOnlyCheckbox} />

### Read-only Radio

<Description of={FieldGroupStories.ReadOnlyRadio} />

<Canvas of={FieldGroupStories.ReadOnlyRadio} />

## Properties

Expand Down
36 changes: 24 additions & 12 deletions components/fieldgroup/stories/fieldgroup.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,14 @@ export default {
helpText: "Select an option.",
items: [
{
id: "apple",
label: "Apples are best",
customClasses: ["spectrum-FieldGroup-item"],
},
{
id: "banana",
label: "Bananas forever",
customClasses: ["spectrum-FieldGroup-item"],
},
{
id: "pear",
label: "Pears or bust",
customClasses: ["spectrum-FieldGroup-item"],
}
Expand Down Expand Up @@ -126,7 +123,7 @@ export const VerticalRadio = Template.bind({});
VerticalRadio.tags = ["!dev"];
VerticalRadio.args = {
layout: "vertical",
inputType: "radio",
inputType: "radio"
};
VerticalRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -146,7 +143,7 @@ export const HorizontalRadio = Template.bind({});
HorizontalRadio.tags = ["!dev"];
HorizontalRadio.args = {
layout: "horizontal",
inputType: "radio",
inputType: "radio"
};
HorizontalRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -167,7 +164,7 @@ InvalidRadio.tags = ["!dev"];
InvalidRadio.args = {
layout: "horizontal",
inputType: "radio",
isInvalid: true,
isInvalid: true
};
InvalidRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand Down Expand Up @@ -231,7 +228,7 @@ VerticalSideLabelRadio.tags = ["!dev"];
VerticalSideLabelRadio.args = {
labelPosition: "side",
inputType: "radio",
layout: "vertical",
layout: "vertical"
};
VerticalSideLabelRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -242,7 +239,7 @@ HorizontalSideLabelRadio.tags = ["!dev"];
HorizontalSideLabelRadio.args = {
labelPosition: "side",
inputType: "radio",
layout: "horizontal",
layout: "horizontal"
};
HorizontalSideLabelRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand Down Expand Up @@ -275,13 +272,28 @@ HorizontalSideLabelCheckbox.parameters = {
* - Read-only checkboxes are immutable, i.e. their checked state cannot be changed.
* - Unlike disabled checkbox groups, the normally focusable elements of a checkbox group should remain focusable.
*/
export const ReadOnly = Template.bind({});
ReadOnly.tags = ["!dev"];
ReadOnly.args = {
export const ReadOnlyCheckbox = Template.bind({});
ReadOnlyCheckbox.tags = ["!dev"];
ReadOnlyCheckbox.args = {
isReadOnly: true,
inputType: "checkbox",
helpText: undefined,
};
ReadOnly.parameters = {
ReadOnlyCheckbox.parameters = {
chromatic: { disableSnapshot: true },
};

/**
* A group of read-only radio buttons.
*
* Review the individual story for more features of [read-only radio buttons](?path=/docs/components-radio--docs#read-only).
*/
export const ReadOnlyRadio = Template.bind({});
ReadOnlyRadio.tags = ["!dev"];
ReadOnlyRadio.args = {
isReadOnly: true,
inputType: "radio"
};
ReadOnlyRadio.parameters = {
chromatic: { disableSnapshot: true },
};
6 changes: 4 additions & 2 deletions components/fieldgroup/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const Template = (
customClasses = [],
layout = "vertical",
inputType = "radio",
name = getRandomId(),
isReadOnly = false,
isRequired = false,
label,
Expand Down Expand Up @@ -61,13 +62,14 @@ export const Template = (
})}
>
${inputType === "radio" ?
items.map((item) =>
items.map((item, i) =>
Radio({
...item,
isReadOnly,
isRequired,
name: "field-group-example",
name: `field-group-example-${name}`,
customClasses: [`${rootClass}-item`],
...(isReadOnly ? {isChecked: i === 1} : {}),
}, context))
: items.map((item, i) =>
CheckBox({
Expand Down
14 changes: 2 additions & 12 deletions components/radio/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -239,24 +239,14 @@
}

&.is-readOnly {
.spectrum-Radio-input:read-only {
cursor: initial;
}

/* hide selection indicator */
& .spectrum-Radio-button {
position: fixed;
inset-inline-end: 100%;
inset-block-end: 100%;
clip: rect(1px, 1px, 1px, 1px);
clip-path: inset(50%);
.spectrum-Radio-input {
pointer-events: none;
}

.spectrum-Radio-label,
/* ensure disabled readonly has normal text color */
& .spectrum-Radio-input:disabled ~ .spectrum-Radio-label,
& .spectrum-Radio-input:checked:disabled ~ .spectrum-Radio-label {
margin-inline-start: 0;
color: var(--highcontrast-radio-neutral-content-color, var(--mod-radio-neutral-content-color, var(--spectrum-radio-neutral-content-color)));
}
}
Expand Down
3 changes: 1 addition & 2 deletions components/radio/metadata/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@
".spectrum-Radio-label:lang(ja)",
".spectrum-Radio-label:lang(ko)",
".spectrum-Radio-label:lang(zh)",
".spectrum-Radio.is-readOnly .spectrum-Radio-button",
".spectrum-Radio.is-readOnly .spectrum-Radio-input",
".spectrum-Radio.is-readOnly .spectrum-Radio-input:checked:disabled ~ .spectrum-Radio-label",
".spectrum-Radio.is-readOnly .spectrum-Radio-input:disabled ~ .spectrum-Radio-label",
".spectrum-Radio.is-readOnly .spectrum-Radio-input:read-only",
".spectrum-Radio.is-readOnly .spectrum-Radio-label",
".spectrum-Radio:active .spectrum-Radio-button:before",
".spectrum-Radio:active .spectrum-Radio-input:checked + .spectrum-Radio-button:before",
Expand Down
11 changes: 7 additions & 4 deletions components/radio/stories/radio.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,14 @@ Disabled.parameters = {
};

/**
* A radio group has a read-only option for when it's in the disabled state but still needs to be shown.
* This allows for content to be copied, but not interacted with or changed.
* A radio group has a read-only option for when it's functionally disabled but still needs to be shown.
* This allows for label content to be copied, but prevents the input from being interacted with or changed.
*
* - Read-only radio buttons are disabled, but not all disabled radio buttons are read-only.
* - Read-only radio buttons do not have a focus ring, but the button should be focusable.
* Read-only radio buttons:
* - prevent interaction like disabled, but not all disabled radio buttons are read-only
* - are immutable, i.e. their checked state cannot be changed
* - are keyboard focusable and communicate state to assistive technology
* - use `aria-disabled` since the `readonly` attribute [is not valid](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly#overview) and `aria-readonly` is not currently announced by the majority of screen readers.
*/
export const ReadOnly = BasicGroupTemplate.bind({});
ReadOnly.storyName = "Read-only";
Expand Down
16 changes: 10 additions & 6 deletions components/radio/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export const Template = ({
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
style=${styleMap(customStyles)}
id=${ifDefined(id)}
>
<input
type="radio"
Expand All @@ -47,9 +46,16 @@ export const Template = ({
id=${inputId}
?checked=${isChecked}
?disabled=${isDisabled}
@change=${function() {
if (isDisabled) return;
updateArgs({ isChecked: !isChecked });
aria-disabled=${ifDefined(isReadOnly ? "true" : undefined)}
@change=${(e) => {
if (isDisabled || isReadOnly) return;
updateArgs?.({ isChecked: e.target.checked });
}}
@click=${(e) => {
if (!isReadOnly) return;
// Make checked value immutable for read-only.
e.preventDefault();
}}
/>
<span class="${rootClass}-button ${rootClass}-button--sizeS"></span>
Expand All @@ -74,7 +80,6 @@ export const BasicGroupTemplate = (args, context) => Container({
${Template({
...args,
label: "Example label",
id: "radio-1-" + (args?.id ?? "default"),
name: "radio-example-" + (args?.name ?? "default"),
}, context)}
${Template({
Expand All @@ -84,7 +89,6 @@ export const BasicGroupTemplate = (args, context) => Container({
customStyles: {
"max-width": "220px",
},
id: "radio-2-" + (args?.id ?? "default"),
name: "radio-example-" + (args?.name ?? "default"),
}, context)}
`,
Expand Down

0 comments on commit c091b4d

Please sign in to comment.