Skip to content

Commit

Permalink
fix: [Obs Alert Rules > Rule Detail][KEYBOARD]: N Alerts and N Active…
Browse files Browse the repository at this point in the history
… Now elems must both be keyboard focsuable (#186529)

Closes: elastic/observability-dev#3371

## Description

The Obs Alert Rule Detail view has a card that is clickable with a
focusable element inside it. This is a confusing paradigm and prevents
keyboard users from filtering by all alerts because it's not focusable.
It would be better to make the two alert number widgets the focusable
elements. Screenshot attached below.

PR is based on the following comment posted by @1Copenut in
elastic/observability-dev#3371 (comment)

> @alexwizp Agreed, panels should not be focusable. The highlighted
panel is clickable, and that was unexpected. I could click the entire
panel, and click the "1 Active now" text to filter by all alerts or
active alerts in the table below.
> 
> It would be better to have the "All alerts" text be clickable and
focusable, and keep the "1 Active now" clickable and focusable. That way
the two text blocks have the interactive behavior, while the panel
(card) is just a container.

### Steps to recreate

1. Open the [Obs
Alerts](https://keepserverless-qa-oblt-b4ba07.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts)
table
2. Click the "Manage Rules" link
3. Create a new rule and verify it appears in the Rules table
4. Click on the rule name to load the Rule Detail view
6. Verify the `1 Active Now`


### What was done?:
1. The click event was **REMOVED** from the panel and has been moved to
`All alerts.`
2. `aria-describedby` attributes were added for `AllAlertCounts` and
`ActiveAlertCounts`
3. `h3` attributes were replaced to `EuiTitle` in `AllAlertCounts` and
`ActiveAlertCounts`
  • Loading branch information
alexwizp authored Jul 25, 2024
1 parent 7b4e251 commit 631baa3
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 128 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,48 @@
* 2.0.
*/

import React, { MouseEvent } from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiLink } from '@elastic/eui';
import React, { type MouseEvent } from 'react';
import { EuiFlexGroup, useEuiTheme } from '@elastic/eui';
import { ALERT_STATUS_ACTIVE, AlertStatus } from '@kbn/rule-data-utils';
import { ActiveAlertCounts } from './active_alert_counts';
import { AllAlertCounts } from './all_alert_counts';
import {
ACTIVE_ALERT_COUNT_DATA_TEST_SUBJ,
ACTIVE_NOW_LABEL,
ALERTS_LABEL,
ALL_ALERT_COLOR,
TOTAL_ALERT_COUNT_DATA_TEST_SUBJ,
} from './constants';
import { AlertItem } from './alert_item';

interface Props {
activeAlertCount: number;
recoveredAlertCount: number;
onActiveClick?: (
handleClick?: (
event: MouseEvent<HTMLAnchorElement | HTMLDivElement>,
status?: AlertStatus
) => void;
}

export const AlertCounts = ({ activeAlertCount, recoveredAlertCount, onActiveClick }: Props) => {
export const AlertCounts = ({ activeAlertCount, recoveredAlertCount, handleClick }: Props) => {
const { euiTheme } = useEuiTheme();

return (
<EuiFlexGroup gutterSize="l" responsive={false}>
<EuiFlexItem style={{ minWidth: 50, wordWrap: 'break-word' }} grow={false}>
<AllAlertCounts count={activeAlertCount + recoveredAlertCount} />
</EuiFlexItem>
<EuiFlexItem style={{ minWidth: 50, wordWrap: 'break-word' }} grow={false}>
{!!onActiveClick ? (
<EuiLink
onClick={(event: React.MouseEvent<HTMLAnchorElement>) =>
onActiveClick(event, ALERT_STATUS_ACTIVE)
}
data-test-subj="activeAlerts"
>
<ActiveAlertCounts activeAlertCount={activeAlertCount} />
</EuiLink>
) : (
<ActiveAlertCounts activeAlertCount={activeAlertCount} />
)}
</EuiFlexItem>
<AlertItem
label={ALERTS_LABEL}
count={activeAlertCount + recoveredAlertCount}
color={ALL_ALERT_COLOR}
data-test-subj={TOTAL_ALERT_COUNT_DATA_TEST_SUBJ}
handleClick={handleClick}
/>
<AlertItem
label={ACTIVE_NOW_LABEL}
count={activeAlertCount}
color={activeAlertCount > 0 ? euiTheme.colors.dangerText : euiTheme.colors.successText}
alertType={ALERT_STATUS_ACTIVE}
handleClick={handleClick}
showWarningIcon={true}
data-test-subj={ACTIVE_ALERT_COUNT_DATA_TEST_SUBJ}
/>
</EuiFlexGroup>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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, { type MouseEvent, type ReactNode } from 'react';
import numeral from '@elastic/numeral';
import {
EuiIcon,
EuiTitle,
EuiText,
EuiTextColor,
EuiFlexItem,
EuiLink,
type EuiTextColorProps,
} from '@elastic/eui';
import { type AlertStatus } from '@kbn/rule-data-utils';
import { ALERT_COUNT_FORMAT } from './constants';

interface AlertItemProps {
label: string | ReactNode;
count: number;
color: EuiTextColorProps['color'];
alertType?: AlertStatus;
handleClick?: (
event: MouseEvent<HTMLAnchorElement | HTMLDivElement>,
status?: AlertStatus
) => void;
showWarningIcon?: true;
'data-test-subj'?: string;
}

export const AlertItem = ({
label,
count,
handleClick,
alertType,
color,
showWarningIcon,
'data-test-subj': dataTestSubj,
}: AlertItemProps) => {
const content = (
<>
<EuiTitle size="s">
<EuiTextColor data-test-subj={dataTestSubj} color={color}>
{numeral(count).format(ALERT_COUNT_FORMAT)}
{count > 0 && showWarningIcon ? (
<>
&nbsp;
<EuiIcon type="warning" ascent={10} />
</>
) : null}
</EuiTextColor>
</EuiTitle>
<EuiText size="s" color="subdued">
{label}
</EuiText>
</>
);

return (
<EuiFlexItem style={{ minWidth: 50, wordWrap: 'break-word' }} grow={false}>
{handleClick ? (
<EuiLink
onClick={(event: React.MouseEvent<HTMLAnchorElement>) => {
handleClick(event, alertType);
}}
>
{content}
</EuiLink>
) : (
content
)}
</EuiFlexItem>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,7 @@ export const AlertSummaryWidgetCompact = ({
};

return (
<EuiPanel
element="div"
data-test-subj="alertSummaryWidgetCompact"
hasShadow={false}
hasBorder
onClick={handleClick}
>
<EuiPanel element="div" data-test-subj="alertSummaryWidgetCompact" hasShadow={false} hasBorder>
<EuiFlexGroup direction="column">
{!!timeRangeTitle && (
<EuiFlexItem>
Expand All @@ -78,7 +72,7 @@ export const AlertSummaryWidgetCompact = ({
<AlertCounts
activeAlertCount={activeAlertCount}
recoveredAlertCount={recoveredAlertCount}
onActiveClick={handleClick}
handleClick={handleClick}
/>
</EuiFlexItem>

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { FtrProviderContext } from '../../../ftr_provider_context';

const COMPACT_COMPONENT_SELECTOR = 'alertSummaryWidgetCompact';
const COMPACT_TIME_RANGE_TITLE_SELECTOR = 'timeRangeTitle';
const COMPACT_ACTIVE_ALERTS_SELECTOR = 'activeAlerts';

const FULL_SIZE_COMPONENT_SELECTOR = 'alertSummaryWidgetFullSize';

Expand All @@ -23,37 +22,42 @@ export function ObservabilityAlertSummaryWidgetProvider({ getService }: FtrProvi
return await testSubjects.existOrFail(COMPACT_COMPONENT_SELECTOR);
};

const getCompactTimeRangeTitle = async () => {
return (await testSubjects.find(COMPACT_TIME_RANGE_TITLE_SELECTOR)).getVisibleText();
const getFullSizeComponentSelectorOrFail = async () => {
return await testSubjects.existOrFail(FULL_SIZE_COMPONENT_SELECTOR);
};

const getCompactActiveAlertSelector = async () => {
return await testSubjects.find(COMPACT_ACTIVE_ALERTS_SELECTOR);
const getCompactTimeRangeTitle = async () => {
return (await testSubjects.find(COMPACT_TIME_RANGE_TITLE_SELECTOR)).getVisibleText();
};

const getCompactWidgetSelector = async () => {
return await testSubjects.find(COMPACT_COMPONENT_SELECTOR);
};

const getFullSizeComponentSelectorOrFail = async () => {
return await testSubjects.existOrFail(FULL_SIZE_COMPONENT_SELECTOR);
const getActiveAlertSelector = async () => {
return await testSubjects.find(ACTIVE_ALERT_SELECTOR);
};

const getTotalAlertSelector = async () => {
return await testSubjects.find(TOTAL_ALERT_SELECTOR);
};

const getActiveAlertCount = async () => {
return (await testSubjects.find(ACTIVE_ALERT_SELECTOR)).getVisibleText();
return (await getActiveAlertSelector()).getVisibleText();
};

const getTotalAlertCount = async () => {
return (await testSubjects.find(TOTAL_ALERT_SELECTOR)).getVisibleText();
return (await getTotalAlertSelector()).getVisibleText();
};

return {
getCompactActiveAlertSelector,
getCompactComponentSelectorOrFail,
getCompactTimeRangeTitle,
getCompactWidgetSelector,
getCompactTimeRangeTitle,
getFullSizeComponentSelectorOrFail,
getActiveAlertCount,
getTotalAlertSelector,
getActiveAlertSelector,
getTotalAlertCount,
getActiveAlertCount,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ export default ({ getService }: FtrProviderContext) => {
expect(timeRangeTitle).to.be('Last 30 days');
});

it('handles clicking on active correctly', async () => {
it('handles clicking on active alerts correctly', async () => {
const activeAlerts =
await observability.components.alertSummaryWidget.getCompactActiveAlertSelector();
await observability.components.alertSummaryWidget.getActiveAlertSelector();
await activeAlerts.click();

const url = await browser.getCurrentUrl();
Expand All @@ -178,10 +178,10 @@ export default ({ getService }: FtrProviderContext) => {
expect(url.includes(to.replaceAll(':', '%3A'))).to.be(true);
});

it('handles clicking on widget correctly', async () => {
const compactWidget =
await observability.components.alertSummaryWidget.getCompactWidgetSelector();
await compactWidget.click();
it('handles clicking on total alerts correctly', async () => {
const totalAlerts =
await observability.components.alertSummaryWidget.getTotalAlertSelector();
await totalAlerts.click();

const url = await browser.getCurrentUrl();
const from = 'rangeFrom:now-30d';
Expand Down

0 comments on commit 631baa3

Please sign in to comment.