-
Notifications
You must be signed in to change notification settings - Fork 27.5k
docs(step_15): create new step about accessibility #14779
base: master
Are you sure you want to change the base?
Conversation
fyi @marcysutton |
<div doc-tutorial-reset="15"></div> | ||
|
||
## Why accessibility matters | ||
|
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.
Could you add a paragraph on what is "accessibility" and "aria" and how it affects people. Just a brief summary and then list the links for more info.
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.
ok, I'm on it.
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.
I've added some definitions.
I left a few comments. Overall looks good 👍 A couple more necessary changes:
|
For this example, I think is better the directives rather to use a component. These directives are needed to intercept the repeater and return the values to the aria regions. Maybe this can do it in another way. Another option would be to remove the section "How to use the directives" and add a link to the Directives webpage. |
add definitions about accessibility and WAI-ARIA ngAria module now is not neccesary, so the section has been moved to explain how to use it if you need it explain how to fix the problem with the keyboard in the detail page.
@gkalpak I've made some updates. I will ping you, when I complete all of them (limit of lines, explanation of protractor accessibility plugin). |
* [Accessibility in AngularJS and Beyond](http://marcysutton.com/slides/fluent2015/) | ||
* [Introduction to Web Accessibility](https://www.w3.org/WAI/intro/accessibility.php) | ||
* [Apps For All: Coding Accessible Web Applications](https://shop.smashingmagazine.com/products/apps-for-all) | ||
|
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.
Here's one more article that was written in response to Angular a11y: https://www.smashingmagazine.com/2015/05/client-rendered-accessibility/
@gkalpak the limit of 100 characters is only for the code examples or all the document? |
@felixzapata, the limit is for the whole document (where ever possible, e.g. if a URL is too long it's OK). |
@gkalpak I have to review about the lines and I have to think what to do with the directives. Have you seen my previous comment about that? I mean, remove the section "How to use the directives" and add a link to the Directives webpage, instead of copy the code of the directives. |
remove information about directives. Now the tutorial links to the guide. remove information about how to install ngAria because is not used in this step.
@gkalpak it is ready for another review:
When the step is approved, I will squash the commits. |
@gkalpak it is ready for another review. |
@gkalpak hi, any news about this pull request? |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
1 similar comment
CLAs look good, thanks! |
Sorry it is taking so long, @felixzapata. It is still on my todo list. I'll get to it after 1.5.9 and 1.6.0-rc.0 are released. |
ok @gkalpak |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Docs update to add a new step about accessibility
What is the current behavior? (You can also link to an open issue here)
The current tutorial is not accessible. With this new step you can learn how to fix the issues.
What is the new behavior (if this is a feature change)?
Introduce the accessibility into the tutorial.
Does this PR introduce a breaking change?
No, it does'nt. Only affects to the tutorial.
Please check if the PR fulfills these requirements
Other information:
My opinion is this step should be a process inside of the tutorial. Not a separate step. This way the user can understand that is optional but it should be mandatory.