Skip to content
This repository was 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
Member

@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

mdc-web-bot commented Dec 19, 2019

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

mdc-web-bot commented Dec 20, 2019

All 728 screenshot tests passed for commit e91ae6f vs. master! 💯🎉
@mdc-web-bot
Copy link
Collaborator

mdc-web-bot commented Dec 23, 2019

All 728 screenshot tests passed for commit 186537e vs. master! 💯🎉
@mdc-web-bot
Copy link
Collaborator

mdc-web-bot commented Dec 23, 2019

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
setNativeControlDisabled(disabled: boolean): void;

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

@abhiomkar abhiomkar Dec 23, 2019

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
Member Author

@yavanosta yavanosta Dec 24, 2019

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
Member Author

@yavanosta yavanosta Dec 24, 2019

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

mdc-web-bot commented Dec 24, 2019

All 728 screenshot tests passed for commit 2659b45 vs. master! 💯🎉
@mdc-web-bot
Copy link
Collaborator

mdc-web-bot commented Dec 24, 2019

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.

Labels

cla: yes

7 participants

@yavanosta @codecov-io @mdc-web-bot @abhiomkar @allan-chen @googlebot