-
Notifications
You must be signed in to change notification settings - Fork 7
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
improves keyboard nav #204
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* by putting inputs and associated button in form, we get 'submit/click on enter' by default; no need for extra JS to make that happen (see code commented out in app.js) * by using buttons instead of <a> links, we get tab navigation for free (e.g. see results list when you search for 'sydney boys' and get multiple results) * first rule of ARIA - don't use it if you have something standard instead https://www.w3.org/TR/aria-in-html/#first-rule-of-aria-use * see also https://www.smashingmagazine.com/2015/12/reimagining-single-page-applications-progressive-enhancement/
* secondary has to do with secondary schools and is used in the JS (.secondary buttons had an event handler attached which shouldn't have been attached to the new search button). * this helps make using that button work w/o clicking on it
* uncomment the script tag in index.html to enable this tool which lets you visually see accessibility issues on the page * it's also available as a chrome extension, but that extension doesn't yet have the screen-reader viewer feature which tells you what a screen reader would read
* padding on some elements so the outline isn't squashed against element text
* so that when we need to use rgba(), we can clearly see how colors are related
prevent screen readers from reading funny characters by using aria-hidden=true
not currently used but might be useful later
* improved focus states * note that we can also set tabindex="-1" on attribution elements later since users probably don't want to be bothered with those every time they try using the map
* uses role=group aria attribute (see bootstrap for similar examples)
* when we scrollAndCenter() we also use setTabFocus() to move the tab focus to the newly scrolled-to content * we set tabindex="-1" on elements containing or just before the content we want to tab to. We use tabindex="0" for elements like inputs we want to be focused for both keyboard or mouse users (see the effect with .block-location input for example - if you click the primary school button, you'd expect you can just start typing an address and now you can). * there's a special case for the map: when we scroll there, we want next tab click to bring focus to the selected school's popup so that keyboard users can then select that popup link and be taken to the school info * there's an added effect here: when you select a school from the results list, we pause briefly so you can observe what is happening (one button being selected while another is deselected). * [tabindex="-1"] styling for setTabFocus() - when focusing on programattically focusable item (unlike real focusable item), we don't want to highlight that we've focused on it - we're only doing it to move the focus position so the *next* tab lands on the focusable item.
* multiple buttons might have .search class, so this line was adding an unnecessary event handler to buttons we didn't want that added to.
* relates to some html structure changes / how we do containers etc
prevent screen readers from reading image names
* tabindex=-2 so we definitely never send focus there
* If using a chrome extension like "Landmarks", you can navigate through the aria landmarks we have on the page * should also help screen reader users
Rustuma
approved these changes
Sep 23, 2016
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. Go forth my son. These changes work well with a keyboard. I can't see any issues
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #190 - improves accessibility for keyboard users.
A few things to note:
<a role="button" ...>
but if we use<button>
we don't need to use ARIA attributes to tell the browser this is a button.setTabFocus()
which is the equivalent for tabbing asscrollAndCenter()
. We settabindex="-1"
attribute on an element previous or containing the first element a user would want to focus on in any given section we'd be scrolling to. By setting tab focus on that element (which might be, for example, a<div>
or<h1>
), when the user clicks tab again, tab focus will land on the first item that can take focus (like a<button>
). See this commit: e8ec295 ("add setTabFocus() to help tab through interface").tota11y
and can be enabled by uncommenting one line in index.html - see a9aea83 ("add tota11y tool for checking accessibility").For more details, see commit messages.