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

fix(menu): Switch from aria-selected to aria-checked for selected menu item. #4779

Merged
merged 20 commits into from
Jun 3, 2019

Conversation

joyzhong
Copy link
Contributor

Matt Goo and others added 16 commits May 28, 2019 15:05
Remove the top and bottom padding from the components. Remove the top and bottom margins from the button content. Button content should be center aligned. Use a <button> tag for the chip.

BREAKING CHANGE: Update mdc-chip-leading-icon-margin and mdc-chip-trailing-icon-margin mixins signatures to take only left and right margin values.
…dleClick/#handleKeydown. (#4655)

BREAKING CHANGE: Dialog `Foundation#handleInteraction` has been split into two methods: `#handleClick` and `#handleKeydown`.
…on group (#4620)

BREAKING CHANGE: Replaced adapter methods getParentElement, getSelectedElementIndex with getSelectedSiblingOfItemAtIndex, isSelectableItemAtIndex.
…4697)

BREAKING CHANGE: Removed `$mdc-checkbox-ui-pct` sass variable from `MDCCheckbox`
Add Adapter#getInitialFocusEl API. initialFocusEl will be the element passed in to Adapter#trapFocus. This formalizes the a11y-aligned idea of adding focus to an initial element (in #trapFocus) into the API.

BREAKING CHANGE: Dialog Adapter#getInitialFocusEl has been added and Adapter#trapFocus first argument is now the initialFocusEl.
…dapters (#4701)

BREAKING CHANGE: Replaced adapter methods getParentElement, getSelectedElementIndex with getSelectedSiblingOfItemAtIndex, isSelectableItemAtIndex.
Add MDCChipAdapter#setAttr method. Update screenshot tests with appropriate roles.

BREAKING CHANGE: Add the setAttr method to the chip adapter.
Use 100vw as it doesn't have the same browser incompatibilities that 100vh does (see PR #4746 for context). 100% width on the other hand breaks on mobile/Safari.
@joyzhong joyzhong changed the base branch from master to develop May 31, 2019 20:55
@joyzhong joyzhong requested a review from abhiomkar May 31, 2019 20:55
@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #4779 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4779   +/-   ##
========================================
  Coverage    98.95%   98.95%           
========================================
  Files          129      129           
  Lines         6332     6332           
  Branches       820      820           
========================================
  Hits          6266     6266           
  Misses          65       65           
  Partials         1        1
Impacted Files Coverage Δ
packages/mdc-menu/foundation.ts 96.25% <100%> (ø) ⬆️

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 f8c561c...cf097a2. Read the comment docs.

@mdc-web-bot
Copy link
Collaborator

All 693 screenshot tests passed for commit 6c2f583 vs. develop! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 690 screenshot tests passed for commit cf097a2 vs. develop! 💯🎉

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.

LGTM.

Please also update examples in README & screenshot test files.

@joyzhong
Copy link
Contributor Author

joyzhong commented Jun 3, 2019

LGTM.

Please also update examples in README & screenshot test files.

No README/screenshot test updates needed, as there were no references to aria-selected.

@joyzhong joyzhong merged commit f4b0bf5 into develop Jun 3, 2019
@joyzhong joyzhong deleted the fix/menu_ariachecked branch June 3, 2019 19:25
abhiomkar pushed a commit that referenced this pull request Jun 11, 2019
…u item. (#4779)

aria-checked is advised in ARIA spec for selectable menu items:
* w3.org/TR/wai-aria-1.1/#menuitemcheckbox
* w3.org/TR/wai-aria-1.1/#menuitemradio
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants