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.

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

[Docs] Add accessibility to tabs in installation documentation #9431

Merged
merged 4 commits into from
Apr 19, 2017

Conversation

AlmeroSteyn
Copy link
Contributor

For your consideration:

After seeing the mention of the awesome addition of tabs in the docs, I added some accessibility features so that all users will be able to use these tabs easily.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@AlmeroSteyn AlmeroSteyn mentioned this pull request Apr 14, 2017
13 tasks
Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Do you think we should change those to be <button>s as well? I find it confusing they don’t select on Spacebar press. It is also annoying to see the text cursor over what essentially are buttons.

I would also increase the padding to be 10px on all sides because the buttons are getting too crowdy now that the font is bold.

@AlmeroSteyn
Copy link
Contributor Author

@gaearon Thank you for taking the time to review and you are quite correct.

Firstly I added the style to get the mouse pointer going. Was so busy testing it with keyboard only...

As for the tab control itself, neither a button nor a link is quite right in this case. The ARIA specification for the tab control specifies the expected interactions for this kind of control. They are actually operated with the arrow keys instead of space or enter. Only the selected tab should be in the focus flow of the page, with the tabindex rolling over as the user interacts with it.

Initially I was not sure just how far to go with the change as I respect the effort of those who have worked on this bit before me, but it's clear it creates more confusion than it solves. And using buttons here would create a kind of toggle button hybrid thing. Or you'd have to strip the role from the li and mess with the button itself or assistive tech will get confused.

So now I added the logic to make it navigable with arrow keys as expected, giving the following result in a screen reader. In this case I used NVDA in Firefox:
image

I have also added a text underline on focus (following the style of the tab like links on the home page) as the standard browser focus outline was getting lost around the tabs, which makes it nasty for sighted users who have to use a keyboard.

Please let me know what you think. If you like what you see and if this is a way you would like to go with accessibility in the React docs (both actual docs about a11y and making the docs themselves more accessible) I would be happy to help where I can.

@gaearon
Copy link
Collaborator

gaearon commented Apr 18, 2017

I think the text underline feels odd. It's okay on links because they get it both on focus and on hover, but it's odd to have an underline instead of an outline. I also do get the browser outline, but maybe it depends on the browser (mine is Chrome on OS X).

@AlmeroSteyn
Copy link
Contributor Author

AlmeroSteyn commented Apr 18, 2017

@gaearon Love love that you say that!

Okay, here is my tab with focus set in Firefox on Windows:
image
and in Chrome on Windows:
image

In Chrome I can see the outline happening but the contrast is terrible. In Firefox I can see no change happening (aside from the underline I made).

In order to be visible to users with low vision or color blindness, there would need to be at least a significant colour contrast difference between the button itself and the focus outline or some sort of shape change. I opted for the underline (although I would be the first to argue to style only links as links) but also considered making the button colour very dark on focus.

The second option is usually extremely accessible but I know a lot of folks find it ugly. Is there a very dark blue in the docs style guide you'd be happy for me to use in this case?

@AlmeroSteyn
Copy link
Contributor Author

I have removed the underline already and added a darker blue focus style to the selected tab for illustration.

@gaearon
Copy link
Collaborator

gaearon commented Apr 19, 2017

I changed the color to be less dark so that it doesn’t look as weird (compromise).
LGTM!

@gaearon gaearon merged commit 9526174 into facebook:master Apr 19, 2017
@gaearon
Copy link
Collaborator

gaearon commented Apr 19, 2017

Also thanks for caring. If you find more issues like this let us know.

gaearon pushed a commit that referenced this pull request Apr 19, 2017
* Add accessibility to tabs in installation documentation

* Change color and fix styling

(cherry picked from commit 9526174)
@AlmeroSteyn
Copy link
Contributor Author

It's a pleasure!!

However I see that on mobile it freaks out a bit as the tabs aren't wrapping well. Will fix it when I get home later today.

@gaearon
Copy link
Collaborator

gaearon commented Apr 19, 2017

Cool, thanks!

flarnie pushed a commit to flarnie/react that referenced this pull request Jun 7, 2017
…ook#9431)

* Add accessibility to tabs in installation documentation

* Change color and fix styling

(cherry picked from commit 9526174)
flarnie added a commit that referenced this pull request Jun 7, 2017
In order to ensure consistency between the 15-stable and 15.6 branches, we found all the diffs present only in 15-stable and cherry-picked them onto 15.6-dev.

These are 100% documentation updates, which is why we didn't bother cherry-picking them in the first place, but having them on the 15.6-dev branch makes the two branches more consistent and that is better.

Nobody will accidentally see outdated docs on the 15.6-dev branch
these diffs won't somehow be lost or overwritten when we combine the branches to release 15.6.
Test Plan:
Manually tested the docs, looking at many of the updated pages. Also ran all tests etc.
Changelog below:
---

* Fix the proptypes deprecation warning url on the "Don't Call PropTypes Warning" doc page (#9419)

* Use the same prop-types link on the warning docs page as the main proptypes doc page

* Link to repo instead

* Docs: Clarification of setState() behavior (#9329)

* Clarification of setState() behavior

`setState()` is a frequent source of confusion for people new to React, and I believe part of that is due to minimization of the impact of the asynchronous behavior of `setState()` in the documentation. This revision is an attempt to clarify that behavior. For motivation and justification, see [setState Gate](https://medium.com/javascript-scene/setstate-gate-abc10a9b2d82).

* Update reference-react-component.md

* Signature fix

* Update to address @acdlite concerns

* Add more details

* Update proptypes doc (#9391)

* Update proptypes doc

* Removed note

* Add tabs to installation page (#9275, #9277) (#9401)

* Add tabs to installation page (#9275, #9277)

This adds tabs for create-react-app and existing apps to the installation section of the docs. The tab implementation is a simplified version of React Native's installation page.

Fixes #9275.

* Use classList instead of className

* Use same implementation as in RN

* Refractor docs to indicate that state set to props in constructor will not recieve the updated props (#9404)

* Switch Installation to a tab when hash is present (#9422)

* Updated the Good First Bug section in readme (#9429)

* Updated the Good First Bug section in readme

* Inconsistent use of quotes. Prefered single quotes instead of double quotes

* Updated Good first bug link in how_to_contribute doc.

* Undo JSX attribute quote change

* don't capitalize "beginner friendly issue"

(cherry picked from commit 36c935c)

* [Documentation] Impreove the react-component section of doc (#9349)

* Impreove react-component of doc

[#9304](#9304)

* update description

* add missing space

(cherry picked from commit 359f5d2)

* Unique headings for linking purposes (#9259)

Previously two headings were 'Javascript Expressions' - now 'Javascript
Expressions as Props' and 'Javascript Expressions as Children'
(cherry picked from commit 363f6cb)

* Update jsx-in-depth.md (#9178)

* Update jsx-in-depth.md

Line 9 isn't changed

* Move selection down

* Fix

(cherry picked from commit ccb38a9)

* Sort out conferences by date (#9172)

(cherry picked from commit 37f9e35)

* Lift state up - Updating the documentation to mention that onClick is a synthetic event handler (#9427)

* Lift state up - Updating the documentation to mention that onClick is a synthetic event handler

* Review comments - Rephrase to handle synthetic events and event handler patterns

* Tweak

(cherry picked from commit 53a3939)

* Fixed grammar (#9432)

* Update codebase-overview.md

* Some more fixes

(cherry picked from commit d724115)

* [Tutorial] ES6, installation, and button closing tag (#9441)

* adds notes to tutorial on es6 and installation

* fixes tutorial mention of opening button tag

* More writing

* Update

* Minor tweaks to tutorial

* Fix duplicate sentence

* Minor tutorial nits

* Add missing tutorial sidebar links

* Tweak tutorial structure

* FIX: Move CRA build info under it's tab page (#9452)

* FIX: Move CRA build info under it's tab page

* Add some links

* Reorganize the "following along" instructions (#9453)

* Reorganize the "following along" instructions

* Minor tweaks

(cherry picked from commit 1ce562e)

* [Docs] Add accessibility to tabs in installation documentation (#9431)

* Add accessibility to tabs in installation documentation

* Change color and fix styling

(cherry picked from commit 9526174)

* [Docs: Installation] Fix tabs responsive layout - Resubmit (#9458)

* [Docs: Installation] Fix tabs responsive layout

* Move tabs a pixel down

* Remove left margin on first tab

* Remove the long line

* Fix mobile styles

(cherry picked from commit a92128e)

* Add more details in jsx-in-depth.md (#9006)

* jsx-in-depth.md add ternary statement for javascript expressions section

* jsx-in-depth.md add explanation to get falsey values for props

* update jsx-in-depth.md

* ensure links work locally, remove section about falsey prop values

* Fix links

* Add link to 'Typechecking with PropTypes' under 'Advanced Guides' (#9472)

This should have been retained in our docs, since PropTypes are only
moved and not deprecated.

Partially handles #9467, and I'll make a separate PR to
https://github.com/reactjs/prop-types to add more docs to the README
there.
(cherry picked from commit 39ca8aa)

* Updates how-to-contribute.md to use JSFiddle referenced in submit Git issue template (#9503)

(cherry picked from commit c8a64e2)

* Describe fixtures dir in the codebase overview (#9516)

* Describe fixtures dir in overview

* Fix folder name

(cherry picked from commit d12c41c)

* adds indirect refs to docs (#9528)

* adds indirect refs to docs

* Add more info

* Explain clearer

* Rephrase

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

* Update refs-and-the-dom.md

(cherry picked from commit 14fa8a5)

* Add reference to the Hyperscript libraries (#9517)

* Add reference to the Hyperscript libraries

I feel these should be mentioned as they provide terser syntax than using `R.createElement` directly, even with a shorthand.

* Rephrase

(cherry picked from commit a8c223a)

* [Docs] Fix confusing description for the <script>...</script> usage (#9502)

* Fix confusing description for the <script>...</script> usage

* Update jsx-in-depth.md

* Update reference-react-dom-server.md

* Update reference-react-dom.md

* Update reference-react.md

(cherry picked from commit 3d60d4c)

* [site] Load libraries from unpkg (#9499)

* [site] Load libraries from unpkg

* Revert Gemfile changes

(cherry picked from commit cf24d87)

* fixed error formatting in live editor (#9497)

(cherry picked from commit 7ccfb07)

* React.createElement syntax (#9459)

* React.createElement syntax

Added React.createElement syntax.
I think this is required for this tutorial.

* Reword

(cherry picked from commit 9824d52)

* Add guide on integrating with non-react code (#9316)

* Add guide on integrating with non-react code

* Capitalize guide title

* Make links to other docs relative

* Rephrase 'What it does do'

* Remove experimental syntax

* Capitalize Backbone

* Remove empty lifecycle method in generic jQuery example

* Use shouldComponentUpdate() not componentWillUpdate()

* Prefer single quotes

* Add cleanup to generic jQuery example

* Capitalize React

* Generalize the section on Backbone Views

* Generalize the section on Backbone Models, a little

* Add introduction

* Adjust wording

* Simplify ref callbacks

* Fix typo in generic jQuery example

* Fix typos in Backbone models in React components

* Fix more typos in Backbone models in React components

* Add generic section on integrating with other view libraries

* Stress the benefits of an unchanging React element

* Small changes to introduction

* Add missing semicolon

* Revise generic jQuery wrapper section

Moved the section on using empty elements to prevent conflicts above the
code example and added brief introduction to that example.

* Add usage example for Chosen wrapper

* Prevent Chosen wrapper from updating

* Note that sharing the DOM with plugins is not recommended

* Mention how React is used at Facebook

* Mention React event system in template rendering section

* Remove destructuring from function parameters

* Do not name React components Component

* Elaborate on unmountComponentAtNode()

* Mention preference for unidirectional data flow

* Rename backboneModelAdapter

* Replace rest syntax

* Respond to updated model in connectToBackboneModel

* Rewrite connectToBackboneModel example

* Rework connectToBackboneModel example

* Misc changes

* Misc changes

* Change wording

* Tweak some parts

(cherry picked from commit 1816d06)

* Don't build gh-pages branch on CircleCI (#9442)
flarnie pushed a commit that referenced this pull request Jun 12, 2017
* Add accessibility to tabs in installation documentation

* Change color and fix styling

(cherry picked from commit 9526174)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants