fix: [Obs Alerts > Alert Detail][SCREEN READER]: H1 tag should not in…

…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">
angorayc · Oct 1, 2024 · dfd2a60 · dfd2a60
1 parent 02ed6e4
commit dfd2a60
Show file tree

Hide file tree

Showing 6 changed files with 163 additions and 201 deletions.
diff --git a/...ns/observability_solution/observability/public/pages/alert_details/alert_details.test.tsx b/...ns/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(
+        <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]);
 

diff --git a/...plugins/observability_solution/observability/public/pages/alert_details/alert_details.tsx b/...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 (
     <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'}

diff --git a/...ility_solution/observability/public/pages/alert_details/components/page_title.stories.tsx b/...ility_solution/observability/public/pages/alert_details/components/page_title.stories.tsx
@@ -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 = {

diff --git a/...observability_solution/observability/public/pages/alert_details/components/page_title.tsx b/...observability_solution/observability/public/pages/alert_details/components/page_title.tsx
diff --git a/...rt_details/components/page_title.test.tsx → ...ls/components/page_title_content.test.tsx b/...rt_details/components/page_title.test.tsx → ...ls/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(
       <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();