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

fix(Checkbox): indeterminate state #51350

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

SpaNb4
Copy link
Contributor

@SpaNb4 SpaNb4 commented Oct 23, 2024

๐Ÿค” This is a ...

  • ๐Ÿ†• New feature
  • ๐Ÿž Bug fix
  • ๐Ÿ“ Site / documentation improvement
  • ๐Ÿ“ฝ๏ธ Demo improvement
  • ๐Ÿ’„ Component style improvement
  • ๐Ÿค– TypeScript definition improvement
  • ๐Ÿ“ฆ Bundle size optimization
  • โšก๏ธ Performance optimization
  • โญ๏ธ Feature enhancement
  • ๐ŸŒ Internationalization
  • ๐Ÿ›  Refactoring
  • ๐ŸŽจ Code style optimization
  • โœ… Test Case
  • ๐Ÿ”€ Branch merge
  • โฉ Workflow
  • โ“ Other (about what?)

๐Ÿ”— Related Issues

๐Ÿ’ก Background and Solution

Checkbox component previously incorrectly set the aria-checked attribute for its indeterminate state, causing accessibility issues. This solution ensures proper use of the indeterminate attribute in the component's logic and includes tests to verify its correct behavior

๐Ÿ“ Change Log

Language Changelog
๐Ÿ‡บ๐Ÿ‡ธ English Checkbox: improved accessibility for indeterminate state
๐Ÿ‡จ๐Ÿ‡ณ Chinese Checkbox: ๆ”นๅ–„ไธๅฎšๆœŸ็ปŸ่ฎก็š„ๆ— ้šœ็ข็Žฏๅขƒ

Copy link

stackblitz bot commented Oct 23, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Contributor

github-actions bot commented Oct 23, 2024

Preview is ready

Copy link
Contributor

github-actions bot commented Oct 23, 2024

๐Ÿ‘ Visual Regression Report for PR #51350 Failed โŒ

๐ŸŽฏ Target branch: master (e0e0cc2)
๐Ÿ“– View Full Report โ†—๏ธŽ

Expected (Branch master) Actual (Current PR) Diff
tree-customized-icon.compact.png tree-customized-icon.compact.png tree-customized-icon.compact.css-var.png tree-customized-icon.compact.css-var.png
tree-customized-icon.dark.png tree-customized-icon.dark.png tree-customized-icon.dark.css-var.png tree-customized-icon.dark.css-var.png
tree-customized-icon.default.png tree-customized-icon.default.png tree-customized-icon.default.css-var.png tree-customized-icon.default.css-var.png
tree-directory.compact.png tree-directory.compact.png tree-directory.compact.css-var.png tree-directory.compact.css-var.png
tree-directory.dark.png tree-directory.dark.png tree-directory.dark.css-var.png tree-directory.dark.css-var.png
tree-directory.default.png tree-directory.default.png tree-directory.default.css-var.png tree-directory.default.css-var.png
tree-drag-debug.compact.png tree-drag-debug.compact.png tree-drag-debug.compact.css-var.png tree-drag-debug.compact.css-var.png
tree-drag-debug.dark.png tree-drag-debug.dark.png tree-drag-debug.dark.css-var.png tree-drag-debug.dark.css-var.png
tree-drag-debug.default.png tree-drag-debug.default.png tree-drag-debug.default.css-var.png tree-drag-debug.default.css-var.png
tree-draggable.compact.png tree-draggable.compact.png tree-draggable.compact.css-var.png tree-draggable.compact.css-var.png

Check Full Report for details


If you think the visual diff is acceptable, please check:

  • Visual diff is acceptable

Copy link

Walkthrough

This pull request addresses a bug in the Checkbox component where the aria-checked attribute was incorrectly set for the indeterminate state, causing accessibility issues. The fix involves proper handling of the indeterminate attribute and includes tests to verify the correct behavior.

Changes

Files Summary
components/checkbox/Checkbox.tsx Fixed the handling of the indeterminate state by using composeRef and updating the indeterminate property on the input element. Removed incorrect aria-checked logic.
components/checkbox/tests/snapshots/demo-extend.test.ts.snap, components/checkbox/tests/snapshots/demo.test.tsx.snap Updated snapshot tests to reflect changes in Checkbox behavior.
components/checkbox/tests/checkbox.test.tsx Added a test case to verify the indeterminate state is correctly reflected in the Checkbox component.

Copy link

pkg-pr-new bot commented Oct 23, 2024

@SpaNb4 SpaNb4 force-pushed the fix-checkbox-indeterminate-state branch from 1d7226e to af863a4 Compare October 23, 2024 06:54
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests โœ…

Project coverage is 100.00%. Comparing base (bb58221) to head (af863a4).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #51350   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          761       761           
  Lines        13495     13499    +4     
  Branches      3507      3507           
=========================================
+ Hits         13495     13499    +4     

โ˜” View full report in Codecov by Sentry.
๐Ÿ“ข Have feedback on the report? Share it here.

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 23, 2024

The visual diffs in this PR are not a result of my changes. I've noticed the same diffs in another PR #51351 (comment)

@afc163 afc163 merged commit ec5e6c4 into ant-design:master Oct 23, 2024
40 checks passed
Copy link
Contributor

๐ŸŽ‰ Thank you for your contribution! If you have not yet joined our DingTalk community group, please feel free to join us (when joining, please provide the link to this PR).

๐ŸŽ‰ ๆ„Ÿ่ฐขๆ‚จ็š„่ดก็Œฎ๏ผๅฆ‚ๆžœๆ‚จ่ฟ˜ๆฒกๆœ‰ๅŠ ๅ…ฅ้’‰้’‰็คพๅŒบ็พค๏ผŒ่ฏทๆ‰ซๆไธ‹ๆ–นไบŒ็ปด็ ๅŠ ๅ…ฅๆˆ‘ไปฌ๏ผˆๅŠ ็พคๆ—ถ่ฏทๆไพ›ๆญค PR ้“พๆŽฅ๏ผ‰ใ€‚

@mellis481
Copy link
Contributor

@SpaNb4 You're awesome. ๐Ÿคœ๐Ÿค›

You da man!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox indeterminate state has bad attribute causing a11y error
3 participants