[Graph] Fix various a11y issues by flash1293 · Pull Request #54097 · elastic/kibana

flash1293 · 2020-01-07T10:18:13Z

Summary

Fixes #48488

This PR addresses some a11y issues in Graph:

Empty listing
- No h1 heading - changed the prompt to create a new Graph from h2 to h1
Filled listing
- Empty table header (has to be fixed i table list view) - not actually an error - there is no text in the th itself, but the checkbox in there has a label. If this is a problem, it has to be fixed in eui.
Workspace
- No heading structure (only on filled workspace, on empty workspace guidance panel takes over this role) - added a screenreader visible h1 on filled workspace
Other a11y problems that can be fixed in the same batch
- Empty buttons in side bar (no accessible label) - added tooltip as aria-label as well
- Low contrast issue in the side bar - tinted the color a little

flash1293 · 2020-01-07T13:45:38Z

Jenkins, test this. Not sure whether unrelated or not.

elasticmachine · 2020-01-07T16:09:47Z

Pinging @elastic/kibana-app (Team:KibanaApp)

myasonik · 2020-01-07T17:25:19Z

Saw a few more low hanging fruit for a11y fixes here. If you have a chance, could you add these fixes in as well? (If anything's not clear, feel free to reach out! Happy to zoom!)

Can you add aria-descrbiedby={idOfTheH1} to the main on the graph empty page? (EuiPageBody renders as a main and you should be able to pass that prop in...)
When going through the steps to add a graph, can the contents of the page be put inside a main tag? (#graphBasic might be a good option to turn into a main which I'm guessing you'll have control over given its id and should also solve the problem for the workspace in general...) (And, same as item above, can you add aria-describedby the same way to it as well?)
- Should also work for the workspace's h1
Can ul.gphGuidancePanel__list be changed to an <ol>? Also on this element, can you add aria-describedby={idOfTheH1}?
For each li.gphGuidancePanel__item, could you add aria-current="step" to the current one that should be dealt with?
Adding display: contents onto buttons (i.e., .gphGuidancePanel__item button) unfortunately ruins their semantics and so several of their interactions (like focus stats and tabbing). Toggling it off in the inspector I don't see any changes without the property so I'm hoping we can just remove it?
You kill pointer events on buttons for disabled items but I'm still able to activate the button using a screen reader. Could you add disabled={true} to any buttons that are inside disabled list items?

flash1293 · 2020-01-08T08:53:59Z

Hi @myasonik ,

thanks for this!

I went through your points and I think I addressed all of them.

Adding display: contents onto buttons (i.e., .gphGuidancePanel__item button) unfortunately ruins their semantics and so several of their interactions (like focus stats and tabbing). Toggling it off in the inspector I don't see any changes without the property so I'm hoping we can just remove it?

This was originally added to have the button text wrap like ordinary text, but with the current labels it's not necessary anymore because the clickable texts are pretty short. I removed it for now.

lizozom · 2020-01-08T10:25:03Z

src/plugins/kibana_react/public/table_list_view/table_list_view.tsx

  tableListTitle: string;
  toastNotifications: ToastsStart;
  uiSettings: IUiSettingsClient;
+  ariaDescribedby?: string;


TableListView already takes in quite a few textual inputs (entityName, entityNamePlural, tableListTitle).
Could those be used for this?

If the fact they are translated is an issue, I would still pass in a single input, instead of the two added here (ariaDescribedby, headingId)?.

Also, TableListView is used in only two other places. Would you mind adding this new attribute (if eventually needed) there as well?

The reason I introduced new properties is that in some cases the same id has to be used outside of the component - reusing the entity name seems like too much magic. I'm fine with just using a single property - it takes away some flexibility in using the component, but for the current usage it's not relevant.

…dashboard listing

myasonik

Found a couple more things: (sorry this didn't happen in one pass!)

For the no data sources page, set the callout to have an h1 (it takes a heading prop)
Unfortunately, it's not possible to set an ID on that heading so we've got to remove the aria-labelledby on the main#graphBasic tag if we're showing the no data sources page... (If that's not possible, can add a screen reader only p with the right ID so that the main is still labelled.)

src/plugins/kibana_react/public/table_list_view/table_list_view.tsx

x-pack/legacy/plugins/graph/public/angular/templates/index.html

x-pack/legacy/plugins/graph/public/components/guidance_panel/guidance_panel.tsx

lizozom

@elastic/kibana-app-arch changes LGTM

elizabetdev

I just added a suggestion. But all the rest LGTM! 🎉

x-pack/legacy/plugins/graph/public/angular/templates/_sidebar.scss

…w.tsx Co-Authored-By: Michail Yasonik <michail@yasonik.com>

Co-Authored-By: Michail Yasonik <michail@yasonik.com>

…uidance_panel.tsx Co-Authored-By: Michail Yasonik <michail@yasonik.com>

…scss Co-Authored-By: Elizabet Oliveira <elizabet.oliveira@elastic.co>

flash1293 · 2020-01-13T08:07:44Z

@elasticmachine merge upstream

kibanamachine · 2020-01-13T09:21:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e6c1c89

History

💚 Build #19437 succeeded 1cc7a0d
💔 Build #19418 failed 0554990
💔 Build #19217 failed 3cd3ed5
💔 Build #19212 failed 17bff7b
💚 Build #18704 succeeded c460819

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* master: (69 commits) [Graph] Fix various a11y issues (elastic#54097) Add ApplicationService app status management (elastic#50223) logs in one time (elastic#54447) Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392) [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457) Security - Role Mappings UI (elastic#53620) [SIEM] [Detection engine] Permission II (elastic#54292) Allow User to Cleanup Repository from UI (elastic#53047) [Detection engine] Some UX for rule creation (elastic#54471) share specific instances of some ui packages (elastic#54079) [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792) [APM] Delay rendering invalid license notification (elastic#53924) [Graph] Improve error message on graph requests (elastic#54230) [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719) Unit Tests for common/lib (elastic#53736) [Graph] Only show explorable fields (elastic#54101) remove linting rule exception for markdown (elastic#54232) [Monitoring] Fetch shard data more efficiently (elastic#54028) [Maps] Add hiddenLayers option to embeddable map input (elastic#54355) Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391) ...

…age-offset-floating-tooltip * 'master' of github.com:elastic/kibana: [Maps] refactor isPointsOnly, isLinesOnly, and isPolygonsOnly to make synchronous (elastic#54067) Fix icon path in tutorial introduction (elastic#49684) [State Management] State containers improvements (elastic#54436) Fix floating tools rendering logic (elastic#54505) Handle another double quote special case (elastic#54474) [Home][Tutorial] Add data UI for IBM MQ Filebeat module (elastic#54238) fix(package): upgrade transitive dependency elliptic to v6.5.2 (elastic#54476) [Graph] Fix various a11y issues (elastic#54097) # Conflicts: # src/legacy/core_plugins/console/public/np_ready/application/models/legacy_core_editor/legacy_core_editor.ts

fix various a11y issues

7f8244b

flash1293 added Feature:Graph Graph application feature v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Jan 7, 2020

flash1293 mentioned this pull request Jan 7, 2020

[Meta] Graph Technical Plan #44225

Closed

31 tasks

Merge remote-tracking branch 'upstream/master' into graph/a11y

35679b7

flash1293 marked this pull request as ready for review January 7, 2020 16:09

flash1293 requested a review from a team January 7, 2020 16:09

flash1293 requested a review from a team as a code owner January 7, 2020 16:09

flash1293 added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Jan 7, 2020

flash1293 requested a review from barlowm January 7, 2020 16:09

elizabetdev self-requested a review January 7, 2020 16:19

flash1293 added 2 commits January 8, 2020 09:19

Merge remote-tracking branch 'upstream/master' into graph/a11y

bddc98c

fix additional a11y issues

9f6853f

flash1293 requested a review from a team as a code owner January 8, 2020 08:52

lizozom reviewed Jan 8, 2020

View reviewed changes

flash1293 added 2 commits January 8, 2020 13:28

simplify table list view and fix heading structure for visualize and …

2c87d99

…dashboard listing

fix unit tests

c460819

myasonik approved these changes Jan 8, 2020

View reviewed changes

lizozom approved these changes Jan 9, 2020

View reviewed changes

elizabetdev approved these changes Jan 9, 2020

View reviewed changes

x-pack/legacy/plugins/graph/public/angular/templates/_sidebar.scss Outdated Show resolved Hide resolved

flash1293 and others added 4 commits January 10, 2020 11:11

Merge remote-tracking branch 'upstream/master' into graph/a11y

579d769

Update src/plugins/kibana_react/public/table_list_view/table_list_vie…

f1502e2

…w.tsx Co-Authored-By: Michail Yasonik <michail@yasonik.com>

Update src/plugins/kibana_react/public/table_list_view/table_list_vie…

17bff7b

…w.tsx Co-Authored-By: Michail Yasonik <michail@yasonik.com>

Update x-pack/legacy/plugins/graph/public/angular/templates/index.html

18cc05a

Co-Authored-By: Michail Yasonik <michail@yasonik.com>

flash1293 and others added 7 commits January 10, 2020 11:12

Update x-pack/legacy/plugins/graph/public/components/guidance_panel/g…

92e1b17

…uidance_panel.tsx Co-Authored-By: Michail Yasonik <michail@yasonik.com>

Update x-pack/legacy/plugins/graph/public/angular/templates/_sidebar.…

173f8ac

…scss Co-Authored-By: Elizabet Oliveira <elizabet.oliveira@elastic.co>

Merge branch 'graph/a11y' of github.com:flash1293/kibana into graph/a11y

71f7ecc

fix a11y of no data page

3cd3ed5

Merge remote-tracking branch 'upstream/master' into graph/a11y

e9bf311

only show labelled-by if listing content is already fetched

0554990

fix table list view labelledby attribute during loading

1cc7a0d

Merge branch 'master' into graph/a11y

e6c1c89

flash1293 merged commit 204155b into elastic:master Jan 13, 2020

flash1293 deleted the graph/a11y branch January 13, 2020 09:27

flash1293 added a commit to flash1293/kibana that referenced this pull request Jan 13, 2020

[Graph] Fix various a11y issues (elastic#54097)

935c343

flash1293 mentioned this pull request Jan 13, 2020

[7.x] [Graph] Fix various a11y issues (#54097) #54552

Merged

flash1293 added a commit that referenced this pull request Jan 13, 2020

[Graph] Fix various a11y issues (#54097) (#54552)

a64013f

jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020

[Graph] Fix various a11y issues (elastic#54097)

7137a52

Conversation

flash1293 commented Jan 7, 2020

Summary

Uh oh!

flash1293 commented Jan 7, 2020

Uh oh!

elasticmachine commented Jan 7, 2020

Uh oh!

myasonik commented Jan 7, 2020

Uh oh!

flash1293 commented Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lizozom Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

flash1293 Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myasonik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lizozom left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flash1293 commented Jan 13, 2020

Uh oh!

kibanamachine commented Jan 13, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

flash1293 commented Jan 8, 2020 •

edited

Loading

flash1293 Jan 8, 2020 •

edited

Loading