Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert the Button's disabled state #2340

Merged
merged 3 commits into from
Dec 12, 2023
Merged

Conversation

connor-baer
Copy link
Member

@connor-baer connor-baer commented Dec 12, 2023

Purpose

#2299 improved the behavior of disabled Buttons. The disabled attribute was replaced with the aria-disabled attribute, which enabled the disabled element to receive focus and be perceived by screenreader users.

This is a breaking change since it requires developers to update unit tests that were validating the Button's disabled state using Testing Library's .toBeDisabled() matcher.

We'd like to roll out the new button design in a minor version, so we've decided to postpone this functional change until the next major.

The other potentially major changes were found to be non-breaking, so we're able to include them in a minor release.

Approach and changes

  • Re-add the disabled attribute and pointer-events: none style to Buttons
  • Downgrade non-breaking major changesets to minor

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

Copy link

changeset-bot bot commented Dec 12, 2023

⚠️ No Changeset found

Latest commit: 956fae1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link

vercel bot commented Dec 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
oss-circuit-ui ✅ Ready (Inspect) Visit Preview Dec 12, 2023 10:15am

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Merging #2340 (956fae1) into canary (ef5ed0a) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           canary    #2340      +/-   ##
==========================================
- Coverage   96.97%   96.95%   -0.02%     
==========================================
  Files         252      252              
  Lines       20274    20278       +4     
  Branches     1263     1263              
==========================================
  Hits        19661    19661              
- Misses        600      604       +4     
  Partials       13       13              
Files Coverage Δ
packages/circuit-ui/components/Button/shared.tsx 90.98% <100.00%> (-1.52%) ⬇️

@connor-baer connor-baer marked this pull request as ready for review December 12, 2023 10:40
@connor-baer connor-baer requested a review from a team as a code owner December 12, 2023 10:40
@connor-baer connor-baer merged commit 3361c45 into canary Dec 12, 2023
@connor-baer connor-baer deleted the revert/button-disabled-state branch December 12, 2023 10:50
connor-baer added a commit that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant