Skip to content

Conversation

5t3ph
Copy link
Contributor

@5t3ph 5t3ph commented Oct 29, 2024

Description

Same motivation as CSS-1004 for read-only checkbox, as being updated in PR #3328

Field group:

  • modifications to Storybook templates to support custom names for radio groups for enabling screen reader testing
  • added story for read-only radios

Radio:

  • Modifies read-only CSS to enable a visible UI input
  • Template (similar modifications as for checkbox):
    • JS added to make radio choice immutable as a group to prevent changing via pointer or keyboard
    • disabled attribute removed when read-only. Read-only still needs to be focusable / in the tab order
    • added aria-disabled="true" to not announce as interactive by screen readers (ex. "dimmed" in VO)
      • confirmed that aria-readonly had no effect on radio buttons, or even when set on role="radiogroup" at least in VO, but left in case other AT make use of it

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

@jawinn

  • field group docs page: read-only radio shows full input UI
  • radio docs page: read-only radio shows full input UI
  • review docs updates for both examples
  • read-only radio groups are immutable via both pointer (click) and keyboard access
  • read-only radios are keyboard focusable
  • field group of radios are announced as a group by screen reader (VO)
  • read-only radios are announced as dimmed/disabled by screen reader

Regression testing

Expected changes:

  • read-only radio now displays the input UI

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

Before:
image

After:
image

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

Copy link

changeset-bot bot commented Oct 29, 2024

🦋 Changeset detected

Latest commit: 2261d0f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@spectrum-css/radio Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@5t3ph 5t3ph added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Oct 29, 2024
Copy link
Contributor

github-actions bot commented Oct 29, 2024

🚀 Deployed on https://pr-3350--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented Oct 29, 2024

File metrics

Summary

Total size: 4.26 MB*
Total change (Δ): 🟢 ⬇ 0.62 KB (-0.01%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
radio 17.58 KB 🟢 ⬇ 0.21 KB

Details

radio

Filename Head Compared to base
index-base.css 16.52 KB 🟢 ⬇ 0.21 KB (-1.20%)
index-theme.css 1.67 KB -
index-vars.css 17.58 KB 🟢 ⬇ 0.21 KB (-1.13%)
index.css 17.58 KB 🟢 ⬇ 0.21 KB (-1.13%)
themes/express.css 1.35 KB -
themes/spectrum.css 1.35 KB -
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Collaborator

@jawinn jawinn left a comment

Choose a reason for hiding this comment

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

This is looking really good! I just had a few suggestions after wrapping up with the related checkbox work:

@5t3ph 5t3ph requested a review from jawinn November 12, 2024 16:19
@5t3ph 5t3ph marked this pull request as ready for review November 12, 2024 16:19
@5t3ph 5t3ph added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. ready-for-review s1 labels Nov 12, 2024
@jawinn jawinn added run_vrt For use on PRs looking to kick off VRT and removed skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Nov 12, 2024
@5t3ph 5t3ph merged commit c091b4d into main Nov 12, 2024
14 checks passed
@5t3ph 5t3ph deleted the seckles/css-1005-radio-readonly-refactor branch November 12, 2024 21:56
@github-actions github-actions bot mentioned this pull request Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge run_vrt For use on PRs looking to kick off VRT size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants