From 4994e4141d45db4c404bafa034589b14262ef497 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 30 Sep 2024 10:10:23 +0300 Subject: [PATCH] fix: [Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not include secondary information: 0002 (#193958) Closes: https://github.com/elastic/observability-accessibility/issues/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: https://github.com/elastic/kibana/pull/193961 for `Rule Detail` # Screen: image --- .../alert_details/alert_details.test.tsx | 30 +++- .../pages/alert_details/alert_details.tsx | 24 +++- .../components/page_title.stories.tsx | 30 ++-- .../alert_details/components/page_title.tsx | 128 ------------------ ...e.test.tsx => page_title_content.test.tsx} | 51 +------ .../components/page_title_content.tsx | 101 ++++++++++++++ 6 files changed, 163 insertions(+), 201 deletions(-) delete mode 100644 x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.tsx rename x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/{page_title.test.tsx => page_title_content.test.tsx} (58%) create mode 100644 x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.tsx diff --git a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.test.tsx b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.test.tsx index 1089bb52b7ed4..877b0b965d1ce 100644 --- a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.test.tsx +++ b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.test.tsx @@ -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( + + {getPageTitle(ruleCategory)} + , + 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]); diff --git a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.tsx b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.tsx index 9561dc37b3e7f..e7533c226df58 100644 --- a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.tsx +++ b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/alert_details.tsx @@ -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 ( + ), + children: ( + = (props: PageTitleProps) => ( +const Template: ComponentStory = (props: PageTitleContentProps) => ( ); -const TemplateWithPageTemplate: ComponentStory = (props: PageTitleProps) => ( +const TemplateWithPageTemplate: ComponentStory = ( + props: PageTitleContentProps +) => ( - } bottomBorder={false} /> + } bottomBorder={false} /> ); @@ -33,21 +34,8 @@ 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 = { diff --git a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.tsx b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.tsx deleted file mode 100644 index 91ba564df7216..0000000000000 --- a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.tsx +++ /dev/null @@ -1,128 +0,0 @@ -/* - * 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 moment from 'moment'; -import React from 'react'; -import { - EuiFlexGroup, - EuiFlexItem, - EuiLoadingSpinner, - EuiSpacer, - 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, - ALERT_RULE_CATEGORY, - TIMESTAMP, -} from '@kbn/rule-data-utils'; -import { css } from '@emotion/react'; -import { asDuration } from '../../../../common/utils/formatters'; -import { TopAlert } from '../../../typings/alerts'; - -export interface PageTitleProps { - alert: TopAlert | null; - alertStatus?: AlertStatus; - dataTestSubj: string; -} - -export function pageTitleContent(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 PageTitle({ alert, alertStatus, dataTestSubj }: PageTitleProps) { - const { euiTheme } = useEuiTheme(); - - if (!alert) return ; - - return ( -
- - {pageTitleContent(alert.fields[ALERT_RULE_CATEGORY])} - - - - - {alertStatus && ( - - )} - - - - - - :  - - - {moment(Number(alert.start)).locale(i18n.getLocale()).fromNow()} - - - - - - - - :  - - - {asDuration(Number(alert.fields[ALERT_DURATION]))} - - - - - - - - :  - - - {moment(alert.fields[TIMESTAMP]?.toString()).locale(i18n.getLocale()).fromNow()} - - - - -
- ); -} diff --git a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.test.tsx b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.test.tsx similarity index 58% rename from x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.test.tsx rename to x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.test.tsx index d22e60eeef4d0..d65c4f84c99dd 100644 --- a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title.test.tsx +++ b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.test.tsx @@ -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( - + ); }; - 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(); diff --git a/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.tsx b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.tsx new file mode 100644 index 0000000000000..11fc6d0a476bb --- /dev/null +++ b/x-pack/plugins/observability_solution/observability/public/pages/alert_details/components/page_title_content.tsx @@ -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 ( + + + {alertStatus && ( + + )} + + + + + + :  + + + {moment(Number(alert.start)).locale(i18n.getLocale()).fromNow()} + + + + + + + + :  + + + {asDuration(Number(alert.fields[ALERT_DURATION]))} + + + + + + + + :  + + + {moment(alert.fields[TIMESTAMP]?.toString()).locale(i18n.getLocale()).fromNow()} + + + + + ); +}