Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(switch): handle aria-checked correctly. (#5202) #5357

Merged
merged 4 commits into from
Jan 7, 2020
Merged

fix(switch): handle aria-checked correctly. (#5202) #5357

merged 4 commits into from
Jan 7, 2020

Conversation

Yavanosta
Copy link
Contributor

@Yavanosta Yavanosta commented Dec 18, 2019 •
edited
Loading

Handle aria-checked attribute on MDC Web level, so developers should not implement this logic.

Aria checked is required for role="switch" elements.

In addition after this fix MDC Web catalog website should be updated.

Closes #5202

BREAKING CHANGE: Added setNativeControlAttr method in mdc-switch adapter.

@codecov-io
Copy link

codecov-io commented Dec 18, 2019 •
edited
Loading

Codecov Report

Merging #5357 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5357      +/-   ##
==========================================
+ Coverage   97.63%   97.71%   +0.08%     
==========================================
  Files         163      163              
  Lines        6293     6299       +6     
  Branches      857      857              
==========================================
+ Hits         6144     6155      +11     
+ Misses        149      144       -5
Impacted Files Coverage Δ
packages/mdc-switch/constants.ts 100% <ø> (ø) ⬆️
packages/mdc-switch/component.ts 98.3% <100%> (+6.92%) ⬆️
packages/mdc-switch/foundation.ts 100% <100%> (ø) ⬆️
packages/mdc-auto-init/index.ts 96% <0%> (+4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6808c5...0c0fc2c. Read the comment docs.

allan-chen
allan-chen suggested changes Dec 18, 2019
View reviewed changes
@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit e91ae6f vs. master! 💯🎉

allan-chen
allan-chen approved these changes Dec 19, 2019
View reviewed changes
Copy link
Collaborator

@allan-chen allan-chen left a comment

Choose a reason for hiding this comment

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

LGTM!

@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit e91ae6f vs. master! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit 186537e vs. master! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit 98e2827 vs. master! 💯🎉

abhiomkar
abhiomkar approved these changes Dec 23, 2019
View reviewed changes
Copy link
Collaborator

@abhiomkar abhiomkar left a comment

Choose a reason for hiding this comment

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

Since this change introduces new adapter method this is a breaking changes.

You would need to add BREAKING CHANGE: <description_of_breaking_change> line to PR description and also in commit description for CHANGELOG.

Thanks!

packages/mdc-switch/adapter.ts Outdated
@@ -48,4 +48,9 @@ export interface MDCSwitchAdapter {
* Sets the disabled state of the native HTML control underlying the switch.
*/
setNativeControlDisabled(disabled: boolean): void;

/**
* Set an attribute value of the native HTML control underlying the switch.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure you sync-in all the changes from your CL. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I remember that MDC Web is not synced yet, so added commit with typo fix. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to PR description and to commit.

@Yavanosta Yavanosta force-pushed the fix/switch/aria-checked branch from 2659b45 to 0c0fc2c Compare December 24, 2019 11:02
@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit 2659b45 vs. master! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 728 screenshot tests passed for commit 0c0fc2c vs. master! 💯🎉

@Yavanosta Yavanosta merged commit d245a1a into master Jan 7, 2020
@Yavanosta Yavanosta deleted the fix/switch/aria-checked branch January 7, 2020 17:55
@andrewseguin andrewseguin mentioned this pull request Jan 7, 2020
Closed
This was referenced Feb 13, 2020
Merged
Closed
@snyk-bot snyk-bot mentioned this pull request Mar 7, 2020
Open
@adrianschmidt adrianschmidt mentioned this pull request Dec 10, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@abhiomkar abhiomkar abhiomkar approved these changes

@allan-chen allan-chen allan-chen approved these changes

@bonniezhou bonniezhou Awaiting requested review from bonniezhou

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[mdc-switch] add required aria-checked
6 participants
@Yavanosta @codecov-io @mdc-web-bot @abhiomkar @allan-chen @googlebot