From c091b4d7ee33c870d24d731c9f377cf7ed1cb19a Mon Sep 17 00:00:00 2001 From: Stephanie Eckles Date: Tue, 12 Nov 2024 15:55:57 -0600 Subject: [PATCH] fix(radio): correct display of read-only state (#3350) --- .changeset/hot-timers-count.md | 5 +++ components/fieldgroup/stories/fieldgroup.mdx | 12 +++++-- .../fieldgroup/stories/fieldgroup.stories.js | 36 ++++++++++++------- components/fieldgroup/stories/template.js | 6 ++-- components/radio/index.css | 14 ++------ components/radio/metadata/metadata.json | 3 +- components/radio/stories/radio.stories.js | 11 +++--- components/radio/stories/template.js | 16 +++++---- 8 files changed, 62 insertions(+), 41 deletions(-) create mode 100644 .changeset/hot-timers-count.md diff --git a/.changeset/hot-timers-count.md b/.changeset/hot-timers-count.md new file mode 100644 index 00000000000..3bba157af0c --- /dev/null +++ b/.changeset/hot-timers-count.md @@ -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. diff --git a/components/fieldgroup/stories/fieldgroup.mdx b/components/fieldgroup/stories/fieldgroup.mdx index 5d07c8d9c60..4ceef8fd355 100644 --- a/components/fieldgroup/stories/fieldgroup.mdx +++ b/components/fieldgroup/stories/fieldgroup.mdx @@ -97,11 +97,17 @@ An invalid group of radio buttons or checkboxes is signified by negative help te -### Read-only checkboxes +### Read-only Checkbox - + - + + +### Read-only Radio + + + + ## Properties diff --git a/components/fieldgroup/stories/fieldgroup.stories.js b/components/fieldgroup/stories/fieldgroup.stories.js index 0284a5e73dd..9217f3b3176 100644 --- a/components/fieldgroup/stories/fieldgroup.stories.js +++ b/components/fieldgroup/stories/fieldgroup.stories.js @@ -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"], } @@ -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 }, @@ -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 }, @@ -167,7 +164,7 @@ InvalidRadio.tags = ["!dev"]; InvalidRadio.args = { layout: "horizontal", inputType: "radio", - isInvalid: true, + isInvalid: true }; InvalidRadio.parameters = { chromatic: { disableSnapshot: true }, @@ -231,7 +228,7 @@ VerticalSideLabelRadio.tags = ["!dev"]; VerticalSideLabelRadio.args = { labelPosition: "side", inputType: "radio", - layout: "vertical", + layout: "vertical" }; VerticalSideLabelRadio.parameters = { chromatic: { disableSnapshot: true }, @@ -242,7 +239,7 @@ HorizontalSideLabelRadio.tags = ["!dev"]; HorizontalSideLabelRadio.args = { labelPosition: "side", inputType: "radio", - layout: "horizontal", + layout: "horizontal" }; HorizontalSideLabelRadio.parameters = { chromatic: { disableSnapshot: true }, @@ -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 }, }; diff --git a/components/fieldgroup/stories/template.js b/components/fieldgroup/stories/template.js index a657149eaa5..3c74a2fbdcf 100644 --- a/components/fieldgroup/stories/template.js +++ b/components/fieldgroup/stories/template.js @@ -16,6 +16,7 @@ export const Template = ( customClasses = [], layout = "vertical", inputType = "radio", + name = getRandomId(), isReadOnly = false, isRequired = false, label, @@ -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({ diff --git a/components/radio/index.css b/components/radio/index.css index 62f72e68366..37e128bdb45 100644 --- a/components/radio/index.css +++ b/components/radio/index.css @@ -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))); } } diff --git a/components/radio/metadata/metadata.json b/components/radio/metadata/metadata.json index 31fd8171138..9fd6219975e 100644 --- a/components/radio/metadata/metadata.json +++ b/components/radio/metadata/metadata.json @@ -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", diff --git a/components/radio/stories/radio.stories.js b/components/radio/stories/radio.stories.js index 4fa1306a520..bb2181ea408 100644 --- a/components/radio/stories/radio.stories.js +++ b/components/radio/stories/radio.stories.js @@ -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"; diff --git a/components/radio/stories/template.js b/components/radio/stories/template.js index ef93d33510d..50ba8484b2f 100644 --- a/components/radio/stories/template.js +++ b/components/radio/stories/template.js @@ -38,7 +38,6 @@ export const Template = ({ ...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}), })} style=${styleMap(customStyles)} - id=${ifDefined(id)} > { + if (isDisabled || isReadOnly) return; + updateArgs?.({ isChecked: e.target.checked }); + }} + @click=${(e) => { + if (!isReadOnly) return; + + // Make checked value immutable for read-only. + e.preventDefault(); }} /> @@ -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({ @@ -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)} `,