Skip to content

Make breadcrumb a heading for screen readers, fix #12885 - #13734

Merged
timroes merged 2 commits into
elastic:masterfrom
timroes:breadcrumb-heading
Aug 29, 2017
Merged

timroes merged 2 commits into
elastic:masterfrom
timroes:breadcrumb-heading

Conversation

@timroes

@timroes timroes commented Aug 28, 2017

Copy link
Copy Markdown
Contributor

This PR is a fix for #12885.

Instead of changing the visuals and adding a header to all those pages, I just annotated the breadcrumb navigation (or the title that is in the very same place) to be the level 1 header (equal to an <h1>) on Visualize, Dashboards, and the surrounding documents listing. This PR doesn't change any visuals, just applies role=heading and aria-level=1 to that elements, so that a screen reader will detect them as a heading (and they would be added to the landmarks navigation).

@aphelionz aphelionz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@ppisljar ppisljar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@timroes
timroes merged commit d22f4ee into elastic:master Aug 29, 2017
@timroes
timroes deleted the breadcrumb-heading branch August 29, 2017 10:05
timroes added a commit that referenced this pull request Aug 29, 2017
* Make breadcrumb a heading for screen readers, fix #12885

* Use h2 in vis wizard step 2
timroes added a commit that referenced this pull request Aug 29, 2017
* Make breadcrumb a heading for screen readers, fix #12885

* Use h2 in vis wizard step 2
@timroes

timroes commented Aug 29, 2017

Copy link
Copy Markdown
Contributor Author

Backports:

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
…tic#13734)

* Make breadcrumb a heading for screen readers, fix elastic#12885

* Use h2 in vis wizard step 2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants