Skip to content

Commit

Permalink
[8.x] fix: [Obs Alerts > Alert Detail][SCREEN READER]: H1 tag shou…
Browse files Browse the repository at this point in the history
…ld not include secondary information: 0002 (#193958) (#194376)

# Backport

This will backport the following commits from `main` to `8.x`:
- [fix: [Obs Alerts > 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>
  • Loading branch information
kibanamachine and alexwizp authored Sep 30, 2024
1 parent 1ee91ba commit c5d2f18
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { Subset } from '../../typings';
import { useKibana } from '../../utils/kibana_react';
import { kibanaStartMock } from '../../utils/kibana_react.mock';
import { render } from '../../utils/test_helper';
import { AlertDetails } from './alert_details';
import { AlertDetails, getPageTitle } from './alert_details';
import { alertDetail, alertWithNoData } from './mock/alert';

jest.mock('react-router-dom', () => ({
Expand Down Expand Up @@ -128,6 +128,34 @@ describe('Alert details', () => {
config
);

describe('getPageTitle', () => {
const renderPageTitle = (ruleCategory: string) =>
render(
<IntlProvider locale="en">
<span data-test-subj="title">{getPageTitle(ruleCategory)}</span>
</IntlProvider>,
config
);

it('should display Log threshold title', () => {
const { getByTestId } = renderPageTitle('Log threshold');

expect(getByTestId('title').textContent).toContain('Log threshold breached');
});

it('should display Anomaly title', () => {
const { getByTestId } = renderPageTitle('Anomaly');

expect(getByTestId('title').textContent).toContain('Anomaly detected');
});

it('should display Inventory title', () => {
const { getByTestId } = renderPageTitle('Inventory');

expect(getByTestId('title').textContent).toContain('Inventory threshold breached');
});
});

it('should show the alert detail page with all necessary components', async () => {
useFetchAlertDetailMock.mockReturnValue([false, alertDetail]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EuiPanel,
EuiSpacer,
EuiTabbedContent,
EuiLoadingSpinner,
EuiTabbedContentTab,
useEuiTheme,
} from '@elastic/eui';
Expand All @@ -36,7 +37,7 @@ import { useKibana } from '../../utils/kibana_react';
import { useFetchRule } from '../../hooks/use_fetch_rule';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { AlertData, useFetchAlertDetail } from '../../hooks/use_fetch_alert_detail';
import { PageTitle, pageTitleContent } from './components/page_title';
import { PageTitleContent } from './components/page_title_content';
import { HeaderActions } from './components/header_actions';
import { AlertSummary, AlertSummaryField } from './components/alert_summary';
import { CenterJustifiedSpinner } from '../../components/center_justified_spinner';
Expand Down Expand Up @@ -68,6 +69,16 @@ const RELATED_ALERTS_TAB_ID = 'related_alerts';
const ALERT_DETAILS_TAB_URL_STORAGE_KEY = 'tabId';
type TabId = typeof OVERVIEW_TAB_ID | typeof METADATA_TAB_ID | typeof RELATED_ALERTS_TAB_ID;

export const getPageTitle = (ruleCategory: string) => {
return i18n.translate('xpack.observability.pages.alertDetails.pageTitle.title', {
defaultMessage:
'{ruleCategory} {ruleCategory, select, Anomaly {detected} Inventory {threshold breached} other {breached}}',
values: {
ruleCategory,
},
});
};

export function AlertDetails() {
const {
cases: {
Expand Down Expand Up @@ -154,7 +165,7 @@ export function AlertDetails() {
},
{
text: alertDetail
? pageTitleContent(alertDetail.formatted.fields[ALERT_RULE_CATEGORY])
? getPageTitle(alertDetail.formatted.fields[ALERT_RULE_CATEGORY])
: defaultBreadcrumb,
},
]);
Expand Down Expand Up @@ -274,8 +285,13 @@ export function AlertDetails() {
return (
<ObservabilityPageTemplate
pageHeader={{
pageTitle: (
<PageTitle
pageTitle: alertDetail?.formatted ? (
getPageTitle(alertDetail.formatted.fields[ALERT_RULE_CATEGORY])
) : (
<EuiLoadingSpinner />
),
children: (
<PageTitleContent
alert={alertDetail?.formatted ?? null}
alertStatus={alertStatus}
dataTestSubj={rule?.ruleTypeId || 'alertDetailsPageTitle'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,34 @@
import React from 'react';
import { ComponentStory } from '@storybook/react';
import { EuiPageTemplate } from '@elastic/eui';
import { ALERT_RULE_CATEGORY } from '@kbn/rule-data-utils';

import { PageTitle as Component, PageTitleProps } from './page_title';
import { PageTitleContent as Component, PageTitleContentProps } from './page_title_content';
import { alert } from '../mock/alert';

export default {
component: Component,
title: 'app/AlertDetails/PageTitle',
title: 'app/AlertDetails/PageTitleContent',
alert,
};

const Template: ComponentStory<typeof Component> = (props: PageTitleProps) => (
const Template: ComponentStory<typeof Component> = (props: PageTitleContentProps) => (
<Component {...props} />
);

const TemplateWithPageTemplate: ComponentStory<typeof Component> = (props: PageTitleProps) => (
const TemplateWithPageTemplate: ComponentStory<typeof Component> = (
props: PageTitleContentProps
) => (
<EuiPageTemplate>
<EuiPageTemplate.Header pageTitle={<Component {...props} />} bottomBorder={false} />
<EuiPageTemplate.Header children={<Component {...props} />} bottomBorder={false} />
</EuiPageTemplate>
);

const defaultProps = {
alert,
};

export const PageTitle = Template.bind({});
PageTitle.args = defaultProps;

export const PageTitleForAnomaly = Template.bind({});
PageTitleForAnomaly.args = {
...{
alert: {
...defaultProps.alert,
fields: {
...defaultProps.alert.fields,
[ALERT_RULE_CATEGORY]: 'Anomaly',
},
},
},
};
export const PageTitleContent = Template.bind({});
PageTitleContent.args = defaultProps;

export const PageTitleUsedWithinPageTemplate = TemplateWithPageTemplate.bind({});
PageTitleUsedWithinPageTemplate.args = {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,72 +10,29 @@ import { render } from '@testing-library/react';
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import {
AlertStatus,
ALERT_RULE_CATEGORY,
ALERT_STATUS,
ALERT_STATUS_ACTIVE,
ALERT_STATUS_RECOVERED,
ALERT_STATUS_UNTRACKED,
} from '@kbn/rule-data-utils';
import { PageTitle, PageTitleProps } from './page_title';
import { PageTitleContent, PageTitleContentProps } from './page_title_content';
import { alert } from '../mock/alert';

describe('Page Title', () => {
describe('Page Title Content', () => {
const defaultProps = {
alert,
alertStatus: ALERT_STATUS_ACTIVE as AlertStatus,
dataTestSubj: 'ruleTypeId',
};

const renderComp = (props: PageTitleProps) => {
const renderComp = (props: PageTitleContentProps) => {
return render(
<IntlProvider locale="en">
<PageTitle {...props} />
<PageTitleContent {...props} />
</IntlProvider>
);
};

it('should display Log threshold title', () => {
const { getByTestId } = renderComp(defaultProps);

expect(getByTestId('ruleTypeId').textContent).toContain('Log threshold breached');
});

it('should display Anomaly title', () => {
const props: PageTitleProps = {
alert: {
...defaultProps.alert,
fields: {
...defaultProps.alert.fields,
[ALERT_RULE_CATEGORY]: 'Anomaly',
},
},
alertStatus: defaultProps.alertStatus as AlertStatus,
dataTestSubj: defaultProps.dataTestSubj,
};

const { getByTestId } = renderComp(props);

expect(getByTestId('ruleTypeId').textContent).toContain('Anomaly detected');
});

it('should display Inventory title', () => {
const props: PageTitleProps = {
alert: {
...defaultProps.alert,
fields: {
...defaultProps.alert.fields,
[ALERT_RULE_CATEGORY]: 'Inventory',
},
},
alertStatus: defaultProps.alertStatus as AlertStatus,
dataTestSubj: defaultProps.dataTestSubj,
};

const { getByTestId } = renderComp(props);

expect(getByTestId('ruleTypeId').textContent).toContain('Inventory threshold breached');
});

it('should display an active badge when alert is active', async () => {
const { getByText } = renderComp(defaultProps);
expect(getByText('Active')).toBeTruthy();
Expand Down
Loading

0 comments on commit c5d2f18

Please sign in to comment.