Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adds Voice Over Accessibility support and Keyboard Nav #3850

Merged
merged 11 commits into from
Jul 27, 2015
Merged

Adds Voice Over Accessibility support and Keyboard Nav #3850

merged 11 commits into from
Jul 27, 2015

Conversation

JLLeitschuh
Copy link
Contributor

Accessibility

  • Adds semantically correct role tags to the grid according to the W3 Aria Standards. This allows a screen reader to accurately describe the content on the screen auditorily.
  • Adds focus management and controls on all menus and inputs. When opening a menu the focus automatically moves into that menu. This means that most of the core grid is fully keyboard accessible.
  • Pagination button controls now show the focus cursor on OSX with Chrome. They also move the focus when they are disabled when hitting the page limits. The pagination controls are also semantically labeled as a menu for Screen Readers.
  • All aria labels use i18n.
  • Adds focus helpers to the gridUtil to make setting focus on elements easier inside of directives.

Requirements

In order to take full advantage of the accessibility and keyboard support angular aria must be included as a dependency to the app. Aria adds the tabindex to dom elements that need them and adds keypress handlers so that the enter key triggers the ng-click action.

Review Specifics

Before merging it would be nice if you were to review the following.

  • Headers:
    • The header is now a button that can be clicked using the OSX Voice Over cursor (because it simulates a click. However, it can not be clicked by using the enter key (because the header doesn't use an ng-click directive). Instead it uses complex logic to determine if a click sort is happening or if a click and drag is happening.
    • When you hide a column the focus is automatically moved to the gird menu button. However, if the grid menu is not enabled then focus is lost and a warning is printed to the console. I don't know where else to set the focus when you remove a column using the menus.
  • Pagination:
    • Does the new CSS on the buttons work for the rest of the grid?
      • Does this break on any other browsers? eg. not an OSX browser?
    • Any edge cases for focus movement that I didn't think about causing the document focus to be lost?
  • General focus:
    • The focus highlight stands out quite a bit. You may get complaints from users about this. However there are many people in the accessibility world that say to stop messing with the browser default. Do we want to cater to these users that insist upon hiding the focus highlight or just tell them to work around it themselves.
    • Angular Aria automatically adds keypress events to buttons with ng-click. This means that without aria you will get the focus highlight on elements but you can't use the keyboard to click them. I don't think this is a problem however I think this could be discussed.

Related #2840

Review on Reviewable

@c0bra
Copy link
Contributor

c0bra commented Jun 25, 2015

Looking really good! A couple notes:

  1. I personally don't like the focus highlight, if by that you mean the outline on the elements that were changed to buttons. It makes the UI feel strange to me and I would prefer to prefer to remove it. Users can always set it back to normal if they want that.
  2. The pagination controls are overlapping the bottom of the grid now:

@JLLeitschuh
Copy link
Contributor Author

Response to

  1. I would argue the opposite, there are countless accessibility professionals out there who say, don't remove the focus highlight. Also, by doing so, you are making the entire grid unusable for a very large population of web users. Alternatively, I would suggest styling it. If you really want an option that can be toggled I would prefer it to be something that users are forced to turn off. At least then it forces devs to think about the entire demographic of their users.
  2. I experienced that problem once. I did a grunt clean build and never saw it again. However, I've also seen the same problem on unstable so this may be a bug outside of what I've changed. I'll have a look into seeing if I can fix it though.

@c0bra
Copy link
Contributor

c0bra commented Jun 29, 2015

  1. Ah; I didn't realize the outline was actually required for focus+aria to work properly. Styling it is fine with me. The only real problem I have is that it's a big clunky blue outline in Chrome and it looks rather bad.
    • I did do a clean build but I can try it again and will let you know.

@c0bra
Copy link
Contributor

c0bra commented Jun 30, 2015

@JLLeitschuh OK it must have been some weird caching thing in Chrome. Looks good to me now!

@c0bra
Copy link
Contributor

c0bra commented Jun 30, 2015

I've tried setting up outline CSS to make it more stylistically consistent, but it doesn't seem to want to work in Chrome. When I set outline: 1px solid blue instead of the border going around the entire element it's just a single side, or only the left and right sides. Strange.

@JLLeitschuh
Copy link
Contributor Author

Outline on Chrome in OSX is being incredibly buggy.

@c0bra
Copy link
Contributor

c0bra commented Jun 30, 2015

OK, so... what's our way forward?

@JLLeitschuh
Copy link
Contributor Author

What I'm in the process of finishing up:

  • Add Screen reader support to cell nav.
  • Fix screen reader support in safari (fixed finally thanks to the accessibility inspector built into XCode)
  • Double check on the pagination footer not showing up correctly.
  • Add Small Close button at the top of the menus (focusable only with tab key) so you can close from the top and bottom of the menus.
  • Hiding menus moves focus to neighbor cell header instead of onto the grid men (because the grid menu may not exist).
  • Update docs to reflect these changes.

Hopefully the last three will get completed tomorrow.

@JLLeitschuh JLLeitschuh force-pushed the feature/accessibility branch from ecfcf87 to 96c1d77 Compare July 2, 2015 00:10
@JLLeitschuh JLLeitschuh force-pushed the feature/accessibility branch 2 times, most recently from cc9d5ee to 790ea3a Compare July 20, 2015 22:10
@JLLeitschuh
Copy link
Contributor Author

#3987 Created a conflict that needs to be resolved.
I'm running out of time to work on this project. Hopefully this will be the last time that I need to rebase.

@JLLeitschuh JLLeitschuh force-pushed the feature/accessibility branch from 790ea3a to 2379d51 Compare July 22, 2015 02:07
@JLLeitschuh JLLeitschuh mentioned this pull request Jul 23, 2015
Closed
@JLLeitschuh
feat(i18n): accessibility i18n terms to 'en'
e5c8299 
Adds accessibility tags to the i18n service.
@JLLeitschuh
feat(gridUtil): Focus helper functions
94e50a5 
Adds a few helper functions to assist with focus management.
Makes the focus return promises and queue
Focus methods return promises that resolve themselves when the focus
either suceseeds or fails.
Additionally, the promises queue and cancel eachother when mutiple
focus events are requested before the timeout is purged.
@JLLeitschuh JLLeitschuh force-pushed the feature/accessibility branch from c26795e to 7327182 Compare July 27, 2015 15:57
@JLLeitschuh
feat(core): Basic screen reader accessibility functionality
377485a 
 - Adds aria roles to the grid to allow screen readers to understand
   what text to read out.
 - Adds screen reader only css element
 - Allows you to make a dom element have content exclusively for a
   screen reader.
 - Adds an accessibility demo to help with testing
@JLLeitschuh
feat(pagination): Accessibiliy & Keyboard Support to Pagination Controls
ee04132 
 - Adds appropriate aria labels to the pagination controls.
 - Styles Pagination Buttons using Bootstrap.
 - Adds focus managment on the buttons so that when they are disabled
   focus shifts to the center input box.
@JLLeitschuh
feat(core): Accessibility and keyboard support to the filter controls.
11a1ae5 
The filter controls can now be accessed using the tab keys and the
remove filter button automatically moves the focus to the input when it
is disabled.
They also support OSX Voice over reading.
Additionally all of the google accessibility audit rules pass.
The focus input now provides a default aria label on the input that can
be overridden in the filter settings.
@JLLeitschuh
feat(core): Accessibility and keyboard support to the grid header.
1f1de5a 
All applicable roles have been applied to the header. OSX Screen reader
correctly reads out all of the header information about each column.
@JLLeitschuh
feat(core): grid menus accessible
1d577b1 
 - When the grid opens the focus is automatically set to the first element
in the list. When the menu closes the focus is automatically reutnred to
the menu button that opened that list except for when the column is
hidden. When the column is hidden the focus is set to the master grid
menu.
 - Also adds the ability to have screen reader only buttons in the menus.
The example of this is the 'close' menu item that only appears when the
focus is over it.
 - Updates e2e tests to reflect menu item addition.
 - The menu now has an additional item that is visible to screen readers
only and only becomes visible when focused. This adds one item to all of
the tests that are counting the number of elements that are 'displayed'.
@JLLeitschuh
feat(cellNav): Accessibility Support to Cell Nav
9532de2 
As you navigate the grid using cell nav the browser should now read out
the cell that is currently focused.

Fixes angular-ui#3896
Partially angular-ui#3815
@JLLeitschuh
feat(core): Two new directives to one bind
a9d2f90 
 - Adds `ui-grid-one-bind-aria-labelledby` and
`ui-grid-one-bind-aria-labelledby-grid` directives to one bind.
 - Fixes a one bind test not being initialized correctly
@JLLeitschuh
docs: Adds ngdoc to describe the accessibility support functionality
d0d92b6 
 - Also adds the ability to use the ng-aria in examples
 - Documents Accessibility and adds e2e tests
 - Adds details regarding how to try out the accessibility features using
OSX Voice over. Also adds e2e tests so to ensure that focus is managed
correctly during navigation around the grid.
@JLLeitschuh
test: Enables Protractor's chromeA11YDevTools
cad6b5f 
 - This turns on Protractor's built in chromeA11YDevTools.
 - This also requires a reload to occur after the last e2e test runs in
order to prevent a Stale Element Refrerence.
More info: angular/protractor#2363
@JLLeitschuh JLLeitschuh force-pushed the feature/accessibility branch from 7327182 to cad6b5f Compare July 27, 2015 16:16
@c0bra
Copy link
Contributor

c0bra commented Jul 27, 2015

LGTM

c0bra added a commit that referenced this pull request Jul 27, 2015
@c0bra
Merge pull request #3850 from JLLeitschuh/feature/accessibility
7420b4b 
Adds Voice Over Accessibility support and Keyboard Nav
@c0bra c0bra merged commit 7420b4b into angular-ui:master Jul 27, 2015
@JLLeitschuh JLLeitschuh mentioned this pull request Jul 27, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
a11y grid-core grid-pagination type: feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JLLeitschuh @c0bra @blowsie