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

Accessibility: Fix the accessibility of the menu items in the ellipsis menu #4124

Merged
merged 1 commit into from Dec 21, 2017

Conversation

Projects
None yet
2 participants
@youknowriad
Contributor

youknowriad commented Dec 21, 2017

closes #4123

This PR adds an aria-labelledby attribute to these menus and changes the color of the label to match the A11y guidelines.

@youknowriad youknowriad self-assigned this Dec 21, 2017

@youknowriad youknowriad requested a review from afercia Dec 21, 2017

@afercia

@youknowriad thanks! Seems to me the second menu is now an empty div? Not sure why.

screen shot 2017-12-21 at 16 34 30

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Dec 21, 2017

Contributor

@afercia I think it has always been like that but it should be fixed now.

Contributor

youknowriad commented Dec 21, 2017

@afercia I think it has always been like that but it should be fixed now.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Dec 21, 2017

Contributor

@youknowriad yep, deleted the branch, checked out again and rebuilt: seems fine now. Maybe I've messed up something. Thanks! 🎉

Contributor

afercia commented Dec 21, 2017

@youknowriad yep, deleted the branch, checked out again and rebuilt: seems fine now. Maybe I've messed up something. Thanks! 🎉

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Dec 21, 2017

Contributor

@afercia Actually, you were right. We had the bug (on master as well) and I pushed a fix after your comment

Contributor

youknowriad commented Dec 21, 2017

@afercia Actually, you were right. We had the bug (on master as well) and I pushed a fix after your comment

@youknowriad youknowriad merged commit 5aabd70 into master Dec 21, 2017

3 checks passed

codecov/project 39.1% (+0.01%) compared to d980bdb
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/menu-items-accessibility branch Dec 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment