Skip to content

Commit

Permalink
fix: [Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not in…
Browse files Browse the repository at this point in the history
…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">
alexwizp authored Sep 30, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 1a39b13 commit 4994e41
Showing 6 changed files with 163 additions and 201 deletions.
Original file line number Diff line number Diff line change
@@ -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', () => ({
@@ -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]);

Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ import {
EuiPanel,
EuiSpacer,
EuiTabbedContent,
EuiLoadingSpinner,
EuiTabbedContentTab,
useEuiTheme,
} from '@elastic/eui';
@@ -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';
@@ -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: {
@@ -154,7 +165,7 @@ export function AlertDetails() {
},
{
text: alertDetail
? pageTitleContent(alertDetail.formatted.fields[ALERT_RULE_CATEGORY])
? getPageTitle(alertDetail.formatted.fields[ALERT_RULE_CATEGORY])
: defaultBreadcrumb,
},
]);
@@ -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'}
Original file line number Diff line number Diff line change
@@ -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 = {

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import moment from 'moment';
import { EuiFlexGroup, EuiFlexItem, EuiText, useEuiTheme } from '@elastic/eui';
import { AlertLifecycleStatusBadge } from '@kbn/alerts-ui-shared';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { AlertStatus, ALERT_DURATION, ALERT_FLAPPING, TIMESTAMP } from '@kbn/rule-data-utils';
import { css } from '@emotion/react';
import { asDuration } from '../../../../common/utils/formatters';
import { TopAlert } from '../../../typings/alerts';

export interface PageTitleContentProps {
alert: TopAlert | null;
alertStatus?: AlertStatus;
dataTestSubj: string;
}

export function PageTitleContent({ alert, alertStatus, dataTestSubj }: PageTitleContentProps) {
const { euiTheme } = useEuiTheme();

if (!alert) {
return null;
}

return (
<EuiFlexGroup direction="row" alignItems="center" gutterSize="xl" data-test-subj={dataTestSubj}>
<EuiFlexItem grow={false}>
{alertStatus && (
<AlertLifecycleStatusBadge
alertStatus={alertStatus}
flapping={alert.fields[ALERT_FLAPPING]}
/>
)}
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="none">
<EuiText size="s" color="subdued">
<FormattedMessage
id="xpack.observability.pages.alertDetails.pageTitle.triggered"
defaultMessage="Triggered"
/>
:&nbsp;
</EuiText>
<EuiText
css={css`
font-weight: ${euiTheme.font.weight.semiBold};
`}
size="s"
>
{moment(Number(alert.start)).locale(i18n.getLocale()).fromNow()}
</EuiText>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="none">
<EuiText size="s" color="subdued">
<FormattedMessage
id="xpack.observability.pages.alertDetails.pageTitle.duration"
defaultMessage="Duration"
/>
:&nbsp;
</EuiText>
<EuiText
css={css`
font-weight: ${euiTheme.font.weight.semiBold};
`}
size="s"
>
{asDuration(Number(alert.fields[ALERT_DURATION]))}
</EuiText>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="none">
<EuiText size="s" color="subdued">
<FormattedMessage
id="xpack.observability.pages.alertDetails.pageTitle.lastStatusUpdate"
defaultMessage="Last status update"
/>
:&nbsp;
</EuiText>
<EuiText
css={css`
font-weight: ${euiTheme.font.weight.semiBold};
`}
size="s"
>
{moment(alert.fields[TIMESTAMP]?.toString()).locale(i18n.getLocale()).fromNow()}
</EuiText>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
);
}

0 comments on commit 4994e41

Please sign in to comment.