Skip to content
This repository has been archived by the owner on Dec 4, 2017. It is now read-only.

docs(a11y-sweep): Apply baseline accessibility to the examples #1613

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlmeroSteyn
Copy link
Contributor

** DO NOT MERGE **

Starting with a11y sweep on the examples.

Background: The examples need to, where possible, comply with at least a minimum of accessibility features.

To kick off the sweep and discussion I have made a few subtle changes to the TOH-1 example app.

Violations corrected:

  1. HTML needs a language tag
  2. Labels should be associated with their related form controls.
  3. Labels should be used with form controls, otherwise use something like a <dl>.
  4. Textual elements should have a contrast ratio of 4.5:1 for normal text and 3:1 for large text (14 point and bold or larger, or 18 point or larger).

These kinds of accessibility issues can be fixed purely in HTML and CSS.

@mattdistefano
Copy link

mattdistefano commented Jun 8, 2016

TOH also has a number of issues with keyboard operability. [Edit: I should say, later TOH examples.]

<div>
<label>name: </label>
<input [(ngModel)]="hero.name" placeholder="name">
<label>name:

Choose a reason for hiding this comment

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

Is there a project preference for nested inputs vs. for/id-based association?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No specific preference yet but I would prefer implicit labelling where possible as it removes the need to generate/bind unique IDs to controls in components. Although both options lead to valid accessible form controls.

Choose a reason for hiding this comment

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

I think that makes sense, but I'd maybe like to see both techniques demonstrated at some point, simply because the explicit/non-nested labelling is pretty popular and also much easier to mess up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fully agree, but don't think this is the place. I'm also working on an a11y cookbook that should see the light in the near future. That has a whole section on labelling, including the explicit labelling version. Here, however, the purpose is to get folks started with Angular2 so I think adding the extra stuff to generate the IDs right off the bat will distract from what the TOH docs are all about.

Choose a reason for hiding this comment

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

Yup, I looked through the a11y cookbook PR a while back. Good stuff! Is it worth commenting on that PR now or are you making big changes?

I guess my concern with the implicit labelling is just that devs who're accustomed to doing explicit labelling incorrectly might learn something from seeing it done correctly here, whereas devs who are accustomed to doing implicit labelling are probably already doing it correctly. But I may be over-thinking it, and this is definitely an improvement in any case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah that one is outdated. Did that, then life happened, with some changes and edits. I just created a new one at #1625

Agree to a certain extent but what we don't want are devs copying and pasting it from TOH because they think that is what is needed to make Angular work

I think most devs looking here will be mostly concerned with getting their first app off the ground and we should not confuse them with extra code. So in the TOH examples I think we should look at the big wins that require little to no extra code. The rest will come later.

Choose a reason for hiding this comment

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

Yeah I think as long as it's accessible in some manner I'm OK. I can appreciate not wanting to confuse people but I also think it's important to position accessibility as a first-class concern and embed it throughout the documentation.

Copy link
Contributor Author

@AlmeroSteyn AlmeroSteyn Jun 8, 2016

Choose a reason for hiding this comment

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

There I fully agree with you. This exercise is exactly about giving a11y that position but to find the balance where the lessons of the separate docs are not diluted. Stuff not added to to the examples due to complexity could eventually be added as comments/notes in the examples or docs.

@AlmeroSteyn
Copy link
Contributor Author

@mattdistefano Thanks for the input! Agreed there are keyboard issues. Will be working through the TOH examples in the coming days with suggested changes.

@mattdistefano
Copy link

Sure thing. I was going to tackle some of this myself but have been too busy the last couple weeks. Let me know if you want some assistance, though.

@AlmeroSteyn
Copy link
Contributor Author

That sounds great. For one, please keep your eye on this PR and keep commenting. And just give me a day or three to get my thoughts in a row, then we see what makes sense.

@@ -0,0 +1,9 @@
body, input[text], button {

Choose a reason for hiding this comment

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

This looks like it's present in the current TOH as well, but is the input[text] selector doing anything? AFAIK this will target an input element with an attribute named 'text'. I'm guessing input[type="text"] was the intent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm it is certainly not doing anything here. Result of copy&paste without looking. Already got it fixed locally, will push with next batch of changes. Not messing with it in the main css file though, as that is used by all the examples.

@Foxandxss Foxandxss changed the title [WIP] docs(a11y-sweep): Apply baseline accessibility to the examples docs(a11y-sweep): Apply baseline accessibility to the examples Jun 9, 2016
@AlmeroSteyn
Copy link
Contributor Author

AlmeroSteyn commented Jun 10, 2016

Added a11y suggestions to the TOH-2 application.

Summary:

  1. All the changes for TOH-1.
  2. Added tabindex and click event for keyboard accessibility.
  3. Added button role and aria-pressed state to inform screen readers of selection.
  4. Also apply hover styles as focus styles.
  5. Extra CSS changes to fix contrast errors.
  6. Fixed header level structure.

NOT FIXED

  1. Feeding back changes in selection of detail section to screen reader.
  2. Implementing button/link instead of span for first rule of ARIA.
  3. Button toggle logic to fully implement aria-pressed.

These would either conflict with the purpose of the example or make it overly complex. Recommendation to add a note on this later.

@AlmeroSteyn
Copy link
Contributor Author

Added a11y suggestions to the TOH-3 application.

Applied all changes from TOH-2 with same exclusions.

@AlmeroSteyn
Copy link
Contributor Author

Added a11y suggestions to the TOH-4 application.

Applied all changes from TOH-2 with same exclusions.

@AlmeroSteyn
Copy link
Contributor Author

Added a11y suggestions to the TOH-5 application.

Applied all changes mentioned with TOH-2 where applicable in the new layout. Added similar fixes to the menu dashboard.

Also similar exclusions. With the addition of no focus management.

@AlmeroSteyn
Copy link
Contributor Author

Added a11y suggestions to the TOH-6 application.

Applied all changes mentioned with TOH-2 where applicable in the new layout. Added similar fixes to the menu dashboard.

Also similar exclusions.

Also did not add focus management and notifications to the screen reader for asynchronous content/actions

As suggested before these could be added as notes/comments in the docs.

Header levels on heroes page not technically correct but as the same component is being used I also skipped the complexity of displaying different h tags per use.

@AlmeroSteyn
Copy link
Contributor Author

AlmeroSteyn commented Jun 10, 2016

TOH application refactor complete.

Based on this I would make the following recommendations.

The following actions I recommend as MUST HAVE:
The examples must be keyboard navigable to a degree where a user can use the full function with a keyboard alone. This means:

  1. Adding tabindex="0" as well as a (keydown.enter) event for every mouse clickable element when a natively clickable element is not used.
  2. All hover styles used to functionally show mouse interaction should also be implemented as focus styles to give the same interaction for a keyboard user.

The examples must play reasonably well with screen readers. This means:

  1. Supplying a lang attribute to indicate the page language.
  2. All form controls should have associated labels. Either implicit labeling where the form control is embedded in the label, explicit labeling where the for/id construct is used or css hidden label or aria-label method for visually hidden labels.
  3. The <label> tag should NOT be used without a related form control. Rather use the <dl> tag for non-interactive data.
  4. Non-native clickable elements (ie using a <li> instead of a button) should be given a role and some ARIA to indicate to the screen reader that a certain state exists. If this is considered too confusing, use the native elements instead.
  5. Page structure is important so at least do not have any skipped levels in the h1-h6 structure.

The following action I recommend as SHOULD HAVE:
Good contract is important for many users, even mouse users. The color ratio rule of 4.5:1 for normal text and 3:1 for large text should be implemented throughout when selecting styles. [This one has an impact on refactor of previous docs as any screen caps may have be recreated due to the visual changes, however this does not add complexity to the examples]

Further accessibility functions such as complex focus management and other ARIA function should be implemented where it does not obscure the purpose of the document example with too much complexity. For example aria-live sections, etc. When not implemented, we should provide notes/comments about their importance to guide developers.

Also implementation of full HTML sematics will probably make many examples overly complex but again should be considered where possible.

IMPORTANT: All these points are MUST HAVE in real world applications.

Lets make a call on what is in and what is out.

@Foxandxss
Copy link
Member

I like all of that.

What I would personally do is: Add all those things that doesn't really change the game. Changing invalid <labels> for something else, removing non clickable elements from where they don't belong. Etc.

For big game changers, say adding new events for keyboard, the hover and focus thing... I would definitely add that too.

What do you think (both @wardbell and @AlmeroSteyn) if we create a new kind of "section" (you know, a colored block) for a11y? A11y is a real thing, we should promote it everywhere and we could add this colored section (in green, I like green) with extra information.

For example in this case, we can add a bit of text + snippet to say that we add a keydown.enter event here for accessibility purposes, bla bla.

At least for the tutorial, we need to teach the best practices there.

@marcysutton
Copy link

This stuff really needs to be merged! Every day people go to learn about Angular 2, they are learning anti-patterns.

@wardbell
Copy link
Contributor

wardbell commented Oct 7, 2016

We agree! We had to postpone on our road to final. We're rev'ing up to review this, clean it up, and get it done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants