Skip to content

Commit

Permalink
fix(radio): correct display of read-only state
Browse files Browse the repository at this point in the history
  • Loading branch information
5t3ph committed Oct 29, 2024
1 parent db450d8 commit 6d59af1
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 36 deletions.
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 fields:

<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
38 changes: 30 additions & 8 deletions components/fieldgroup/stories/fieldgroup.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,17 @@ export default {
helpText: "Select an option.",
items: [
{
id: "apple",
// id: getRandomId("apple"),
label: "Apples are best",
customClasses: ["spectrum-FieldGroup-item"],
},
{
id: "banana",
// id: getRandomId("banana"),
label: "Bananas forever",
customClasses: ["spectrum-FieldGroup-item"],
},
{
id: "pear",
// id: getRandomId("pear"),
label: "Pears or bust",
customClasses: ["spectrum-FieldGroup-item"],
}
Expand Down Expand Up @@ -123,6 +123,7 @@ VerticalRadio.tags = ["!dev"];
VerticalRadio.args = {
layout: "vertical",
inputType: "radio",
name: "vertical"
};
VerticalRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -143,6 +144,7 @@ HorizontalRadio.tags = ["!dev"];
HorizontalRadio.args = {
layout: "horizontal",
inputType: "radio",
name: "horizontal"
};
HorizontalRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -164,6 +166,7 @@ InvalidRadio.args = {
layout: "horizontal",
inputType: "radio",
isInvalid: true,
name: "invalid"
};
InvalidRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -189,7 +192,8 @@ export const Required = Template.bind({});
Required.tags = ["!dev"];
Required.args = {
inputType: "radio",
fieldLabel: "Radio group label (required)"
fieldLabel: "Radio group label (required)",
name: "required"
};
Required.parameters = {
chromatic: { disableSnapshot: true },
Expand Down Expand Up @@ -229,6 +233,7 @@ VerticalSideLabelRadio.args = {
labelPosition: "side",
inputType: "radio",
layout: "vertical",
name: "vertical-side-label"
};
VerticalSideLabelRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand All @@ -240,6 +245,7 @@ HorizontalSideLabelRadio.args = {
labelPosition: "side",
inputType: "radio",
layout: "horizontal",
name: "horizontal-side-label"
};
HorizontalSideLabelRadio.parameters = {
chromatic: { disableSnapshot: true },
Expand Down Expand Up @@ -270,12 +276,28 @@ HorizontalSideLabelCheckbox.parameters = {
/**
* A group of read-only checkboxes that have been checked. In U.S. English, use commas to delineate items within read-only checkbox groups. In other languages, use the locale-specific formatting.
*/
export const ReadOnly = Template.bind({});
ReadOnly.tags = ["!dev"];
ReadOnly.args = {
export const ReadOnlyCheckbox = Template.bind({});
ReadOnlyCheckbox.tags = ["!dev"];
ReadOnlyCheckbox.args = {
isReadOnly: true,
inputType: "checkbox",
};
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",
name: "read-only"
};
ReadOnlyRadio.parameters = {
chromatic: { disableSnapshot: true },
};
11 changes: 9 additions & 2 deletions components/fieldgroup/stories/template.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Template as CheckBox } from "@spectrum-css/checkbox/stories/template.js";
import { Template as FieldLabel } from "@spectrum-css/fieldlabel/stories/template.js";
import { Template as HelpText } from "@spectrum-css/helptext/stories/template.js";
import { getRandomId } from "@spectrum-css/preview/decorators";
import { Template as Radio } from "@spectrum-css/radio/stories/template.js";
import { html } from "lit";
import { classMap } from "lit/directives/class-map.js";
Expand All @@ -15,6 +16,7 @@ export const Template = (
customClasses = [],
layout = "vertical",
inputType = "radio",
name = "default",
isReadOnly = false,
isRequired = false,
label,
Expand All @@ -25,6 +27,8 @@ export const Template = (
} = {},
context = {},
) => {
const groupLabelId = getRandomId("group-label");

return html`
<div
class=${classMap({
Expand All @@ -38,6 +42,7 @@ export const Template = (
aria-invalid=${ifDefined(isInvalid ? "true" : undefined)}
aria-readonly=${ifDefined(isReadOnly && inputType == "radio" ? "true" : undefined)}
aria-required=${ifDefined(isRequired ? "true" : undefined)}
aria-labelledby=${ifDefined(label ? groupLabelId : undefined)}
>
${when(label, () =>
FieldLabel(
Expand All @@ -46,6 +51,7 @@ export const Template = (
label,
isRequired,
alignment: labelPosition === "side" ? "right" : "top",
id: groupLabelId
},
context,
),
Expand All @@ -56,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`],
isChecked: isReadOnly && i === 1
}, context))
: items.map((item) =>
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
12 changes: 8 additions & 4 deletions components/radio/stories/radio.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,15 @@ 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)
* - use `preventDefault()` on click to prevent changing the selection via pointer or keyboard interaction
*/
export const ReadOnly = BasicGroupTemplate.bind({});
ReadOnly.storyName = "Read-only";
Expand Down
18 changes: 11 additions & 7 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,8 +89,7 @@ export const BasicGroupTemplate = (args, context) => Container({
customStyles: {
"max-width": "220px",
},
id: "radio-2-" + (args?.id ?? "default"),
name: "radio-example-" + (args?.name ?? "default"),
}, context)}
`,
});
});

0 comments on commit 6d59af1

Please sign in to comment.