Initial version of a11y guide #13390

timroes · 2017-08-08T16:24:16Z

The first draft of the discussed a11y guidelines, that can be extended over time.

cjcenizal

Awesome! Had a few comments. Could you also migrate over the accessibility content from the HTML style guide: https://github.com/elastic/kibana/blob/master/style_guides/html_style_guide.md#accessibility

cjcenizal · 2017-08-08T23:20:33Z

style_guides/accessibility_guide.md

+
+## Interactive elements
+
+### Use `<button>` and `<a>`


I think we should also specify that the <a> tag needs an href attribute.

Can we change the title of this to "Use <button> and <a href>"? I think we should call out the importance of href as often as possible, because otherwise it will be very easy for people to think that just <a> by itself is sufficient.

cjcenizal · 2017-08-08T23:29:54Z

style_guides/accessibility_guide.md

+
+If you want to make an element clickable, use a `<button>` or `<a>` element for it.
+Use a *button* whenever it causes an action on the current page, and an *a* if it
+navigates to a different page. You can use click listeners just fine on these elements.


You can use click listeners just fine on these elements.

I think we should advise using a click handler on button only, and an href with the anchor tag. If we use an anchor with a click handler, then we'll need to apply kbn-accessible-click or KuiKeyboardAccessible, because browsers won't trigger the click handler when you hit ENTER.

This information also duplicates https://github.com/elastic/kibana/blob/master/style_guides/html_style_guide.md#prefer-button-and-a-when-making-interactive-elements-keyboard-accessible, so we should probably remove it from the HTML style guide.

They trigger a click as long as the a has an href. So I guess this issue will collapse with the one above, about mentioning usage of href. But I will try to make this more clear.

Oh I see. I guess that raises the question, why would you want a click handler and an href?

Isn't it non-semantic to use a link as a button? I think links should be used for navigating to different URLs, and buttons should be used for triggering event handlers. That way you know that you can open links in new windows or tabs, or just reference the URL it points to.

I would totally agree with you in that. But I think there might be some cases, where the "navigation" is actually implemented in JS (like dynamically determine parts of the URL) in which this would be valid. That's why I left it in there.

I agree, but if I understand correctly, here are the scenarios we can have:

 <a href="hardcoded/url"> <a href={dynamicUrl}> <button onClick={doSomething}> <button className="lookLikeALink" onClick={doSomething}>  <a href onClick={doSomething}>

The last example is what I'm advocating against because it just doesn't make sense... you should just use a <button> element styled to look like a link. The href is supposed to provide a hint of where you're going, and the anchor tag should allow you to open the href in a new tab, but none of that functionality is being used.

cjcenizal · 2017-08-08T23:33:15Z

style_guides/accessibility_guide.md

+  * You cannot focus it by pressing <kbd>tab</kbd>. You can add this behavior by
+    adding `tabindex="0"` to the element.
+  * You cannot trigger the click by pressing <kbd>Enter</kbd> or <kbd>Space</kbd>.
+    We have a `kbn-accessible-click` directive for AngularJS to add that behavior.


And we have the analogous KuiKeyboardAccessible React component.

cjcenizal · 2017-08-08T23:34:06Z

style_guides/accessibility_guide.md

+
+Those kind of elements, require a special interaction model. A [code editor](https://github.com/elastic/kibana/pull/13339)
+could require an <kbd>Enter</kbd> keypress before starting editing mode, and
+could leave that mode on <kbd>Escape</kbd> again.


Should we mention using role="application" here, and some reasons why?

cjcenizal · 2017-08-08T23:34:54Z

style_guides/accessibility_guide.md

+
+If you place it on the `input` you will overwrite the implicit `textbox` or `searchbox`
+role, and as such confuse the user, since it loses it meaning as in input element.
+If you place it on the `form` element you will also overwrite it's role and


Typo: "it's role" should be "its role".

timroes · 2017-08-09T09:16:28Z

Ready for another review :-)

aphelionz · 2017-08-09T14:57:17Z

style_guides/accessibility_guide.md

+be a good `role` (also see below) for that element. It is meant for elements providing
+their own interaction schemes.
+
+## Roles


Should we add role=main, section tags and revisit role=application here as well?

I am not sure if we can and should make this a complete developer guideline or just pointing out most common pitfalls. I fear if we start documenting role=main, section, etc. we would also need to start explaining aria-label/aria-labelledby and aria-describedby, etc.?

Which way do you and @cjcenizal think we should go with this document for now?

There's a very ambiguous line between "complete guideline" and "common pitfalls" because it's so relative to the experience level of the developer. I think we might as well err on the side of comprehensiveness and describe those roles, as well as aria-label/aria-labelledby and aria-describedby. We can use #11837 (comment) for the latter.

To elaborate on my line of reasoning: if we try to keep the guide limited to common pitfalls, then we'll run into this same question over and over, so it seems more maintainable to side-step the question entirely.

Yeah, I see your point there, and will add an explanation tomorrow.

cjcenizal

Sorry, I found a bunch of other really nitpicky things! 😄

cjcenizal · 2017-08-09T15:59:08Z

style_guides/accessibility_guide.md

+
+### Use `<button>` and `<a>`
+
+**TL;DR** *Use `<button>` and `<a>` (with `hred`) instead of click listeners on other elements


Typo: hred should be href

cjcenizal · 2017-08-09T16:00:48Z

style_guides/accessibility_guide.md

+
+## Interactive elements
+
+### Use `<button>` and `<a>`


Can we change the title of this to "Use <button> and <a href>"? I think we should call out the importance of href as often as possible, because otherwise it will be very easy for people to think that just <a> by itself is sufficient.

cjcenizal · 2017-08-09T16:01:03Z

style_guides/accessibility_guide.md

+**TL;DR** *Use `<button>` and `<a>` (with `hred`) instead of click listeners on other elements
+and style it whatever way you need.*
+
+If you want to make an element clickable, use a `<button>` or `<a>` element for it.


Can we also change this to <a href>?

cjcenizal · 2017-08-09T16:01:50Z

style_guides/accessibility_guide.md

+and style it whatever way you need.*
+
+If you want to make an element clickable, use a `<button>` or `<a>` element for it.
+Use a *button* whenever it causes an action on the current page, and an *a* if it


I think this can be slightly improved by consistently referencing these elements using codeblocks, e.g. <button> and <a href> instead of button and a.

cjcenizal · 2017-08-09T16:02:14Z

style_guides/accessibility_guide.md

+
+An `a` element must have an `href` attribute, so that (a) it will be correctly perceived
+as a link by a screen reader and (b) that registered click listener will correctly
+trigger on pressing <kbd>Enter</kbd>. If you don't need it, make it `hred="#"`


Typo: hred=# should be href=#.

Holy cow, how often can I make the same mistake 😓

cjcenizal · 2017-08-09T16:03:40Z

style_guides/accessibility_guide.md

+
+If you want to make an element clickable, use a `<button>` or `<a>` element for it.
+Use a *button* whenever it causes an action on the current page, and an *a* if it
+navigates to a different page. You can use click listeners just fine on these elements.


Isn't it non-semantic to use a link as a button? I think links should be used for navigating to different URLs, and buttons should be used for triggering event handlers. That way you know that you can open links in new windows or tabs, or just reference the URL it points to.

cjcenizal · 2017-08-09T16:04:00Z

style_guides/accessibility_guide.md

+
+*Why not use other elements?*
+
+If you create e.g. a *div* with a click listener (like `<div ng-click="ctrl.doSomething()">...</div>`)


Can we change div to <div>?

cjcenizal · 2017-08-09T16:04:22Z

style_guides/accessibility_guide.md

+Use a *button* whenever it causes an action on the current page, and an *a* if it
+navigates to a different page. You can use click listeners just fine on these elements.
+
+An `a` element must have an `href` attribute, so that (a) it will be correctly perceived


Can we change a to <a>?

timroes · 2017-08-11T16:57:50Z

Added some role information, and created the "Naming elements" section. Would appreciate another round of feedback :-)

cjcenizal

I found a couple last little things that can be fixed before merging, but overall this looks great man!!! 🍰

cjcenizal · 2017-08-11T17:27:54Z

style_guides/accessibility_guide.md

+visual text representation (e.g. an icon button):
+
+```html
+<button aria-label="Add filter"><i class="fa fa-plus"></i></button>


Can we change <i> to <span>, which is more semantic?

Does font awesome not use <i> tags anymore?

Font awesome will work on any inline tag, and possibly any block-level tag (haven't tested).

cjcenizal · 2017-08-11T17:28:01Z

style_guides/accessibility_guide.md

+
+```html
+<div id="datepicker">Date Picker</div>
+<button aria-labelledby="datepicker"><i class="fa fa-calendar"></i></button>


cjcenizal · 2017-08-11T17:28:11Z

style_guides/accessibility_guide.md

+#### `<section>`
+
+The `<section>` element, can be used to mark a region on the page, so that it
+appears in the landmark navigation. The section element therefor needs to have


Typo: therefor -> therefore

Just wanted to sound archaic - no kidding, will fix it.

* Initial version of a11y guide * Move a11y content from HTML guide to a11y guide * Add PR feedback * Add PRs feedback * Add more roles * Refactor labeling elements section * Rename "labeling elements" -> "naming elements" * Use span instead of i for icon examples * Correct typo

timroes · 2017-08-14T05:43:51Z

Backport:

6.x: 1495e2e

Initial version of a11y guide

e893d6d

timroes added the Project:Accessibility label Aug 8, 2017

timroes requested review from epixa, aphelionz and cjcenizal August 8, 2017 16:24

cjcenizal suggested changes Aug 8, 2017

View reviewed changes

Tim Roes added 2 commits August 9, 2017 09:38

Move a11y content from HTML guide to a11y guide

397d6d3

Add PR feedback

9dc78d8

aphelionz suggested changes Aug 9, 2017

View reviewed changes

cjcenizal suggested changes Aug 9, 2017

View reviewed changes

Tim Roes and others added 4 commits August 9, 2017 21:56

Add PRs feedback

0a4accb

Add more roles

11f13ec

Refactor labeling elements section

75b3545

Rename "labeling elements" -> "naming elements"

ff3cae4

cjcenizal approved these changes Aug 11, 2017

View reviewed changes

timroes added 2 commits August 11, 2017 19:48

Use span instead of i for icon examples

9b98b37

Correct typo

ec43752

aphelionz approved these changes Aug 12, 2017

View reviewed changes

timroes merged commit 99f1c65 into elastic:master Aug 14, 2017

timroes deleted the a11y-guide branch August 14, 2017 05:42

timroes added v7.0.0 v6.1.0 labels Aug 14, 2017


		### Use `<button>` and `<a>`

		TL;DR *Use `<button>` and `<a>` (with `hred`) instead of click listeners on other elements


		Why not use other elements?

		If you create e.g. a div with a click listener (like `<div ng-click="ctrl.doSomething()">...</div>`)

Initial version of a11y guide #13390

Initial version of a11y guide #13390

Conversation

timroes commented Aug 8, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes commented Aug 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes commented Aug 11, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes commented Aug 14, 2017

cjcenizal Aug 9, 2017 •

edited

Loading

cjcenizal Aug 9, 2017 •

edited

Loading

cjcenizal Aug 9, 2017 •

edited

Loading

cjcenizal Aug 9, 2017 •

edited

Loading