Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Many Select / Menu Fixes #6151

Closed
wants to merge 5 commits into from
Closed

Many Select / Menu Fixes #6151

wants to merge 5 commits into from

Conversation

rschmukler
Copy link
Contributor

This branch fixes many of the issues introduced by the side-effects caused by the accessibility enhancements in RC6, as well as some issues that came to light while debugging the issues reported in RC6.

  1. Fixes an issue where menu contents could go behind the menubar if menu's position was clamped (@jelbourn saw this as the "cropping" issue we experienced while debugging accessibility) (089e33b)

  2. Fix menubar breaking after a menu has been opened and closed with keyboard controls. The first open works as intended, but the following one will break. (d034e14)

  3. Fix placing open selects and menus inside their parent selects. They are now moved to the body element when they are opened. This resolves positioning inside of dialogs, toolbars, etc. (2ea692b, cd75b22)

  4. Fix accessibility issues for menubar menus. Turns out ngAria was setting a bunch of aria-related attributes because of our md-menu-item having a ngModel binding. This could lead to weird situations such as a menuitemcheckbox inside of a role=checkbox. This is now resolved and tested in Chrome w/ NVDA. (9fe3a57) @jelbourn if you can please test w/ Firefox / JAWS

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

}
};

MenuItemController.prototype.clearNgAria = function() {
Copy link
Member

Choose a reason for hiding this comment

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

Could use a comment here explaining what ngAria does and why it's a problem. (which is my understanding of what's happening here)

@jelbourn
Copy link
Member

jelbourn commented Dec 8, 2015

Overall LGTM aside from minor nitpicks

@rschmukler rschmukler added this to the 1.0-rc7 milestone Dec 8, 2015
@ThomasBurleson ThomasBurleson added the pr: merge ready This PR is ready for a caretaker to review label Dec 8, 2015
@rschmukler rschmukler closed this in eb1695a Dec 8, 2015
@EladBezalel EladBezalel deleted the wip-select-menu-fixes branch January 15, 2016 23:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants