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

Adds a11y docs #2357

Merged
merged 15 commits into from
Nov 25, 2019
Merged

Adds a11y docs #2357

merged 15 commits into from
Nov 25, 2019

Conversation

myasonik
Copy link
Contributor

@myasonik myasonik commented Sep 20, 2019

Summary

Adds a bunch of accessibility guidelines. Hopefully people will read them.

The docs here are the same as the Accessibility Guidelines Google doc.

Checklist

  • Added documentation examples
  • A changelog entry exists and is marked appropriately
    - [ ] Checked in dark mode
    - [ ] Checked in mobile
    - [ ] Checked in IE11 and Firefox
    - [ ] Props have proper autodocs
    - [ ] Added or updated jest tests
    - [ ] Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes

@myasonik myasonik marked this pull request as ready for review September 20, 2019 12:00
package.json Outdated Show resolved Hide resolved
Copy link

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

I added comments, questions and suggestions to the Google doc.

@cchaos
Copy link
Contributor

cchaos commented Sep 23, 2019

We might need to consider how we're highlighting the sidebar. There are now 2 sections named "Accessibility" and so they're both in the selected state.

Screen Shot 2019-09-23 at 16 03 00 PM

@cchaos
Copy link
Contributor

cchaos commented Sep 23, 2019

Further, we should really get some sub-navigation headers into the sidebar for Guidelines pages.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Overall we should add some Do/Don't panels to accompany and illustrate each section. There's just a lot of text and no "Do this, not that" type of examples that I think most devs will look for.

src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
src-docs/src/views/guidelines/accessibility.js Outdated Show resolved Hide resolved
@myasonik
Copy link
Contributor Author

myasonik commented Oct 3, 2019

I feel like these guidelines are held together with hot glue in the design department. It's mostly ok if you just look at the rendered page but I use a lot of inline styles and the spacing around the code examples is a little wonky.

If anyone has ideas on how to clean this up, I'm all ears.

CC @cchaos @snide

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Hey @myasonik I don't have time to go through this today and will be out tomorrow. We definitely shouldn't be changing sass for a guidelines page that only live in docs.

There's also other issues that you've pointed out. With its text heavy format this feels really close to an education-focused blog post rather than an example-driven guideline as we've generally provided. We can probably take what's here and move it in that direction, but it'll require some design work and some cycles.

I can help next week.

@snide
Copy link
Contributor

snide commented Oct 22, 2019

Jumping back in here now data grid is mostly done. Gimme a day or two to do a pass.

@snide
Copy link
Contributor

snide commented Nov 22, 2019

OK. This took too long, my apologies, but happy Thanksgiving? I did a fairly large cleanup of the layout that were mostly visual. The following changes were needed:

  • Added a minHeight prop on GuidRuleExample to better layout grids of these.
  • Changed @myasonik's callout changes to have some easier to grok prop names and types.
  • Removed all of the custom style stuff. Most of it wasn't needed and could be fixed with more strict EUI usage.
  • Modified some of the code content to better reference EUI rather than straight HTML. Specifically this felt necessary when discussing the styling of titles.
  • Removed the 50 minute video. I think more video would be good here, but the recording needs to be both shorter and have better quality audio.

I think the content here is much improved and is likely good enough to merge. I still think it could use with some trimming, but I appreciate all the examples that are now in the document and it's much more readable.

This should get a review from @cchaos or another designer before merge since I'll be away.

@myasonik myasonik added the documentation Issues or PRs that only affect documentation - will not need changelog entries label Nov 22, 2019
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I'm finding mostly layout issues and others that are just easier to fix on my end.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I've pushed mostly layout fixes. The other major change was some heading shifts for better links in the side nav. It will help consumers find what they're looking for via search.

Before
Screen Shot 2019-11-22 at 18 39 29 PM

After
Screen Shot 2019-11-22 at 18 16 43 PM

CHANGELOG.md Outdated Show resolved Hide resolved
@myasonik
Copy link
Contributor Author

@cchaos Do you think this is good to merge?

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Yep, LGTM, thanks @myasonik !!

@myasonik myasonik merged commit cc1da42 into elastic:master Nov 25, 2019
@myasonik myasonik deleted the a11y-docs branch November 25, 2019 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility documentation Issues or PRs that only affect documentation - will not need changelog entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants