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

fix: [Obs Alerts > Rule Detail][SCREEN READER]: H1 tag should not include secondary information: 0001 #193961

Merged
merged 5 commits into from
Sep 27, 2024

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Sep 25, 2024

Closes: https://github.com/elastic/observability-accessibility/issues/61

Description

Observability has a few pages that wrap related information like alert counts in the H1 tag. This presents a challenge to screen readers because all of that information now becomes the heading level one. It clogs up the Headings menu and makes it harder to reason about the page and what's primary information vs. secondary.

What was changed?:

  • pageTitle was renamed to pageTitleContent. The title portion was moved out of that component.
  • ObservabilityPageTemplate.pageHeader for the Alert Detail page was updated to separate the title from the other content.

Note

Related PR: #193958 for Alerts Detail

Screen:

image

@alexwizp
Copy link
Contributor Author

/ci

@alexwizp
Copy link
Contributor Author

/ci

@alexwizp
Copy link
Contributor Author

/ci

@alexwizp alexwizp marked this pull request as ready for review September 25, 2024 16:05
@alexwizp alexwizp requested a review from a team as a code owner September 25, 2024 16:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Sep 25, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 27, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 8acaaa5
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-193961-8acaaa5d998a

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
observability 467.5KB 467.4KB -115.0B

History

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

Copy link
Member

@maryam-saeidi maryam-saeidi 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!

I just wondering if we can have <EuiSpacer size="m" /> between these sections instead of l:

State Compoennt Spacer
Now image image
Before image image

Or @maciejforcone do you think having l space is fine?

@maciejforcone
Copy link

Code LGTM!

I just wondering if we can have <EuiSpacer size="m" /> between these sections instead of l:

State Compoennt Spacer
Now image image
Before image image
Or @maciejforcone do you think having l space is fine?

After quick check, it looks like "L" space is our default, we use it in Alert details view as well.

@alexwizp alexwizp merged commit 89f6438 into elastic:main Sep 27, 2024
27 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11069791169

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 27, 2024
…lude secondary information: 0001 (elastic#193961)

Closes: elastic/observability-accessibility#61

# Description

Observability has a few pages that wrap related information like alert
counts in the H1 tag. This presents a challenge to screen readers
because all of that information now becomes the heading level one. It
clogs up the Headings menu and makes it harder to reason about the page
and what's primary information vs. secondary.

# What was changed?:

- `pageTitle` was renamed to `pageTitleContent`. The title portion was
moved out of that component.
- `ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page was
updated to separate the title from the other content.

> [!NOTE]
> Related PR: elastic#193958 for `Alerts
Detail`

# Screen:

<img width="1274" alt="image"
src="https://github.com/user-attachments/assets/4974a669-67e0-447d-9013-c675299ed75c">

(cherry picked from commit 89f6438)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 27, 2024
…d not include secondary information: 0001 (#193961) (#194288)

# Backport

This will backport the following commits from `main` to `8.x`:
- [fix: [Obs Alerts &gt; Rule Detail][SCREEN READER]: H1 tag should not
include secondary information: 0001
(#193961)](#193961)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alexey
Antonov","email":"alexwizp@gmail.com"},"sourceCommit":{"committedDate":"2024-09-27T11:27:27Z","message":"fix:
[Obs Alerts > Rule Detail][SCREEN READER]: H1 tag should not include
secondary information: 0001 (#193961)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/61\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193958
for `Alerts\r\nDetail`\r\n\r\n# Screen: \r\n\r\n<img width=\"1274\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/4974a669-67e0-447d-9013-c675299ed75c\">","sha":"89f64384ef513ae00fcd71a8eb3b797c95a4c36f","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Project:Accessibility","release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version"],"title":"fix:
[Obs Alerts > Rule Detail][SCREEN READER]: H1 tag should not include
secondary information:
0001","number":193961,"url":"https://github.com/elastic/kibana/pull/193961","mergeCommit":{"message":"fix:
[Obs Alerts > Rule Detail][SCREEN READER]: H1 tag should not include
secondary information: 0001 (#193961)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/61\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193958
for `Alerts\r\nDetail`\r\n\r\n# Screen: \r\n\r\n<img width=\"1274\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/4974a669-67e0-447d-9013-c675299ed75c\">","sha":"89f64384ef513ae00fcd71a8eb3b797c95a4c36f"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193961","number":193961,"mergeCommit":{"message":"fix:
[Obs Alerts > Rule Detail][SCREEN READER]: H1 tag should not include
secondary information: 0001 (#193961)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/61\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193958
for `Alerts\r\nDetail`\r\n\r\n# Screen: \r\n\r\n<img width=\"1274\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/4974a669-67e0-447d-9013-c675299ed75c\">","sha":"89f64384ef513ae00fcd71a8eb3b797c95a4c36f"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
alexwizp added a commit that referenced this pull request Sep 30, 2024
…clude secondary information: 0002 (#193958)

Closes: elastic/observability-accessibility#60

# Description 

Observability has a few pages that wrap related information like alert
counts in the H1 tag. This presents a challenge to screen readers
because all of that information now becomes the heading level one. It
clogs up the Headings menu and makes it harder to reason about the page
and what's primary information vs. secondary.

# What was changed?:

- `pageTitle` was renamed to `pageTitleContent`. The title portion was
moved out of that component.
- `ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page was
updated to separate the title from the other content.

> [!NOTE]
> Related PR: #193961 for `Rule
Detail`


# Screen: 

<img width="1226" alt="image"
src="https://github.com/user-attachments/assets/bd33a0b8-3f44-409a-9655-53b739780e4e">
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 30, 2024
…clude secondary information: 0002 (elastic#193958)

Closes: elastic/observability-accessibility#60

# Description

Observability has a few pages that wrap related information like alert
counts in the H1 tag. This presents a challenge to screen readers
because all of that information now becomes the heading level one. It
clogs up the Headings menu and makes it harder to reason about the page
and what's primary information vs. secondary.

# What was changed?:

- `pageTitle` was renamed to `pageTitleContent`. The title portion was
moved out of that component.
- `ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page was
updated to separate the title from the other content.

> [!NOTE]
> Related PR: elastic#193961 for `Rule
Detail`

# Screen:

<img width="1226" alt="image"
src="https://github.com/user-attachments/assets/bd33a0b8-3f44-409a-9655-53b739780e4e">

(cherry picked from commit 4994e41)
kibanamachine added a commit that referenced this pull request Sep 30, 2024
…ld not include secondary information: 0002 (#193958) (#194376)

# Backport

This will backport the following commits from `main` to `8.x`:
- [fix: [Obs Alerts &gt; Alert Detail][SCREEN READER]: H1 tag should not
include secondary information: 0002
(#193958)](#193958)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alexey
Antonov","email":"alexwizp@gmail.com"},"sourceCommit":{"committedDate":"2024-09-30T07:10:23Z","message":"fix:
[Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not include
secondary information: 0002 (#193958)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/60\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193961
for `Rule\r\nDetail`\r\n\r\n\r\n# Screen: \r\n\r\n<img width=\"1226\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/bd33a0b8-3f44-409a-9655-53b739780e4e\">","sha":"4994e4141d45db4c404bafa034589b14262ef497","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Project:Accessibility","release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version"],"title":"fix:
[Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not include
secondary information:
0002","number":193958,"url":"https://github.com/elastic/kibana/pull/193958","mergeCommit":{"message":"fix:
[Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not include
secondary information: 0002 (#193958)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/60\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193961
for `Rule\r\nDetail`\r\n\r\n\r\n# Screen: \r\n\r\n<img width=\"1226\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/bd33a0b8-3f44-409a-9655-53b739780e4e\">","sha":"4994e4141d45db4c404bafa034589b14262ef497"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193958","number":193958,"mergeCommit":{"message":"fix:
[Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not include
secondary information: 0002 (#193958)\n\nCloses:
https://github.com/elastic/observability-accessibility/issues/60\r\n\r\n#
Description \r\n\r\nObservability has a few pages that wrap related
information like alert\r\ncounts in the H1 tag. This presents a
challenge to screen readers\r\nbecause all of that information now
becomes the heading level one. It\r\nclogs up the Headings menu and
makes it harder to reason about the page\r\nand what's primary
information vs. secondary.\r\n\r\n# What was changed?:\r\n\r\n-
`pageTitle` was renamed to `pageTitleContent`. The title portion
was\r\nmoved out of that component.\r\n-
`ObservabilityPageTemplate.pageHeader` for the `Alert Detail` page
was\r\nupdated to separate the title from the other content.\r\n\r\n>
[!NOTE]\r\n> Related PR: #193961
for `Rule\r\nDetail`\r\n\r\n\r\n# Screen: \r\n\r\n<img width=\"1226\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/bd33a0b8-3f44-409a-9655-53b739780e4e\">","sha":"4994e4141d45db4c404bafa034589b14262ef497"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants