Skip to content

Variety of quick a11y fixes#46569

Merged
myasonik merged 6 commits intoelastic:masterfrom
myasonik:axe-a11y-fixes
Oct 3, 2019
Merged

Variety of quick a11y fixes#46569
myasonik merged 6 commits intoelastic:masterfrom
myasonik:axe-a11y-fixes

Conversation

@myasonik
Copy link
Copy Markdown
Contributor

@myasonik myasonik commented Sep 25, 2019 •
edited
Loading

(## Summary

A slew of accessibility fixes for a wide range of issues, mostly around document structure. Focusing on things that are easily detectable by automated tools (which we keep getting flagged for in audits) and things that are easy to fix.

Not all solutions are perfect but they're improvements on what we have today.

Addresses some pages of #37539 (Home, Discover, Dashboard, Visualize). Probably more can be done but this is a good start.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [ ] Unit or functional tests were updated or added to match the most common scenarios

For maintainers

@myasonik myasonik requested a review from bhavyarm September 25, 2019 11:06
@myasonik myasonik requested a review from a team as a code owner September 25, 2019 11:06
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 25, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 25, 2019

💔 Build Failed

@myasonik myasonik requested a review from a team September 25, 2019 14:38
@timroes timroes self-requested a review September 25, 2019 14:41
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 25, 2019

💔 Build Failed

@myasonik myasonik requested a review from a team as a code owner September 26, 2019 11:13
@sulemanof
Copy link
Copy Markdown
Contributor

sulemanof commented Sep 26, 2019

Hi @myasonik !
I would suggest to revert all changes in src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/heatmap_options.html because I have an opened PR #45766 with EUI-fying of this stuff. I've added you as a reviewer. I also tried to follow the general structure you've done in your PR and replaced all the headings to h3 tag.
And I'd also like you to explain the necessity of such a replacement!
Thanks in advance.

@myasonik
Copy link
Copy Markdown
Contributor Author

myasonik commented Sep 26, 2019

@sulemanof Previously I suggested using an h2 because of the existing heading structure. I think the sidebar title was set to be the h1 on the page which would make the headings inside of the sidebar h2s.

Now, I've gone and added a proper h1 to the page (though it's visually hidden) that describes the whole page. The sidebar header is now an h2 which makes all of the sections inside of the sidebar h3 headers.

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 26, 2019

💚 Build Succeeded

sulemanof
sulemanof approved these changes Sep 26, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@sulemanof sulemanof left a comment

Choose a reason for hiding this comment

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

LGTM

timroes
timroes suggested changes Sep 27, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Overall looks really good, got a couple of minor code style comments.

src/legacy/core_plugins/data/public/query/query_bar/components/query_bar_input.tsx Outdated
Copy link
Copy Markdown
Contributor

@timroes timroes Sep 27, 2019

Choose a reason for hiding this comment

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

If you change the default message of a string, you should remove all existing translations for this string, so it will be retranslated by our translators again. Just search for that id and remove it from all internationalization files.

Copy link
Copy Markdown
Contributor Author

@myasonik myasonik Oct 1, 2019

Choose a reason for hiding this comment

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

Looks like all of them were already removed... Am I missing anything?

@myasonik myasonik requested a review from timroes October 1, 2019 09:38
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 1, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 1, 2019

💚 Build Succeeded

timroes
timroes approved these changes Oct 1, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, looked at a couple of changes and checked it didn't break styling.

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 2, 2019

💚 Build Succeeded

bhavyarm
bhavyarm approved these changes Oct 2, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@bhavyarm bhavyarm left a comment

Choose a reason for hiding this comment

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

LGTM

joshdover
joshdover approved these changes Oct 2, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Platform changes LGTM

@kertal kertal self-requested a review October 2, 2019 13:24
kertal
kertal approved these changes Oct 2, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Change in Discover LGTM 👍

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 2, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 2, 2019

💚 Build Succeeded

@myasonik myasonik merged commit ea18949 into elastic:master Oct 3, 2019
@myasonik myasonik deleted the axe-a11y-fixes branch October 3, 2019 09:30
@myasonik myasonik mentioned this pull request Oct 3, 2019
Merged
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 3, 2019
@jloleysens
Merge branch 'console-fix-menu-actions-alignment' of github.com:jlole…
db42ef0 
…ysens/kibana into console-fix-menu-actions-alignment

* 'console-fix-menu-actions-alignment' of github.com:jloleysens/kibana: (59 commits)
  [Canvas] i18n for page manager, page preview, and shape preview (elastic#46865)
  [SIEM] hide siem data on empty string or _all (elastic#47166)
  [Uptime] Shim UI exports for new platform (elastic#44722)
  [Monitoring] Metricbeat Migration Wizard Tests (elastic#47139)
  [Logs UI] Support partitioned log rate results in API (elastic#46751)
  [APM] Remove beta badge from transaction breakdown (elastic#47163)
  test: 💍 convert Interpreter font function tests to Jest (elastic#47193)
  visualizations 👉 NP-ready (elastic#47142)
  [Uptime] Update pings chart colors (elastic#46780)
  Add scroll into view to avoid tooltip overlap (elastic#47106)
  Variety of quick a11y fixes (elastic#46569)
  [Graph] Empty workspace overlay (elastic#45547)
  [ML] Converts index and saved search selection to React (elastic#47117)
  [Uptime] Change default status filter in ping list to all on monitor page  (elastic#47108)
  refactor: 💡 convert Interpreter .js -> .ts (elastic#44545)
  Add TypeScript rules to STYLEGUIDE [skip ci] (elastic#47125)
  chore(NA): fix logic behind cleaning x-pack node modules on build (elastic#47091)
  [SIEM] Update Settings Text (elastic#47147)
  Add KQL functionality in the find function of the saved objects (elastic#41136)
  [Maps] Add 'InjectedData' class and revise so File Upload Features are assigned to new InjectedData instances (elastic#46381)
  ...
myasonik pushed a commit that referenced this pull request Oct 3, 2019
[7.x] Variety of quick a11y fixes (#46569) (#47194)
c3ef55d 
Focusing on heading structure and page layout for Home, Discover, Dashboard, and Visualize.

This is progress on #37539
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pheyos pheyos pheyos left review comments

@peteharverson peteharverson peteharverson left review comments

@kertal kertal kertal approved these changes

@bhavyarm bhavyarm bhavyarm approved these changes

@maryia-lapata maryia-lapata Awaiting requested review from maryia-lapata

+3 more reviewers

@timroes timroes timroes approved these changes

@joshdover joshdover joshdover approved these changes

@sulemanof sulemanof sulemanof approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Project:Accessibility release_note:fix v7.5.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@myasonik @elasticmachine @sulemanof @kertal @timroes @joshdover @pheyos @bhavyarm @peteharverson @rayafratkina