Skip to content

Commit

Permalink
fix: [Rules > Rule Detail][SCREEN READER]: Abbreviations must be read…
Browse files Browse the repository at this point in the history
…aloud correctly (#182417)

Closes: elastic/security-team#8649
Closes: elastic/security-team#8658

## Description

The Schedule description list on Rule Detail views is announcing hours
and minutes incorrectly to screen readers. VoiceOver announced `5m` as
"Five meters" and `1h` as "1 eche". This confusion can be remedied by
spelling out the whole word and hiding it visually. Screen shot and code
sample attached.

### Steps to recreate

1. Open [Detection Rules
(SIEM)](https://kibana.siem.estc.dev/app/security/rules/management)
2. Click on a rule name to land on the detail view
3. Start your preferred screen reader
4. Jump down to the Schedule module and listen to the description list
items

### What was done?: 
The `IntervalAbbrScreenReader` component was developed, and it was
integrated into
`/rule_creation_ui/components/description_step/index.tsx` and
`/rule_details/rule_schedule_section.tsx `to handle the `interval` and
`from` fields.

### Screen:
<img width="1508" alt="image"
src="https://github.com/elastic/kibana/assets/20072247/06e54e68-0dcf-45e3-95df-b3c09cb56db5">
 
#### DOM:
<img width="769" alt="image"
src="https://github.com/elastic/kibana/assets/20072247/b0ba88f2-79a2-4fd2-9246-ad5639090bec">
  • Loading branch information
alexwizp authored May 10, 2024
1 parent f3a04a2 commit 99813cf
Show file tree
Hide file tree
Showing 17 changed files with 200 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
*/

export * from './tooltip_with_keyboard_shortcut';
export * from './interval_abbr_screen_reader';
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 { render, screen } from '@testing-library/react';
import { TestProviders } from '../../../mock';
import { IntervalAbbrScreenReader } from '.';

describe('IntervalAbbrScreenReader', () => {
test('should add screen reader text for 35s', () => {
render(
<TestProviders>
<IntervalAbbrScreenReader interval="35s" />
</TestProviders>
);
expect(screen.getByText('35 seconds')).toBeDefined();
});

test('should add screen reader text for 1m', () => {
render(
<TestProviders>
<IntervalAbbrScreenReader interval="1m" />
</TestProviders>
);
expect(screen.getByText('1 minute')).toBeDefined();
});

test('should add screen reader text for 2h', () => {
render(
<TestProviders>
<IntervalAbbrScreenReader interval="2h" />
</TestProviders>
);
expect(screen.getByText('2 hours')).toBeDefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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, { useMemo } from 'react';
import { EuiScreenReaderOnly } from '@elastic/eui';

import * as i18n from './translations';

interface IntervalAbbrScreenReaderProps {
interval: string;
}

export const IntervalAbbrScreenReader = ({ interval }: IntervalAbbrScreenReaderProps) => {
const screenReaderInterval: string | undefined = useMemo(() => {
if (interval) {
const number = parseInt(interval.slice(0, -1), 10);
const unit = interval.charAt(interval.length - 1);

if (Number.isFinite(number)) {
switch (unit) {
case 's': {
return i18n.SECONDS_SCREEN_READER(number);
}
case 'm': {
return i18n.MINUTES_SCREEN_READER(number);
}
case 'h': {
return i18n.HOURS_SCREEN_READER(number);
}
}
}
}

return undefined;
}, [interval]);

return (
<>
<span data-test-subj="interval-abbr-value" aria-hidden={Boolean(screenReaderInterval)}>
{interval}
</span>
{screenReaderInterval && (
<EuiScreenReaderOnly>
<p>{screenReaderInterval}</p>
</EuiScreenReaderOnly>
)}
</>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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 { i18n } from '@kbn/i18n';

export const SECONDS_SCREEN_READER = (value: number) =>
i18n.translate('xpack.securitySolution.accessibility.intervalAbbrScreenReader.seconds', {
defaultMessage: '{value} {value, plural, one { second } other { seconds }}',
values: {
value,
},
});

export const MINUTES_SCREEN_READER = (value: number) =>
i18n.translate('xpack.securitySolution.accessibility.intervalAbbrScreenReader.minutes', {
defaultMessage: '{value} {value, plural, one { minute } other { minutes }}',
values: {
value,
},
});

export const HOURS_SCREEN_READER = (value: number) =>
i18n.translate('xpack.securitySolution.accessibility.intervalAbbrScreenReader.hours', {
defaultMessage: '{value} {value, plural, one { hour } other { hours }}',
values: {
value,
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { FieldIcon } from '@kbn/react-field';

import type { ThreatMapping, Type, Threats } from '@kbn/securitysolution-io-ts-alerting-types';
import { FilterBadgeGroup } from '@kbn/unified-search-plugin/public';
import { IntervalAbbrScreenReader } from '../../../../common/components/accessibility';
import type {
RequiredFieldArray,
Threshold,
Expand Down Expand Up @@ -676,3 +677,12 @@ export const buildSetupDescription = (label: string, setup: string): ListItems[]
}
return [];
};

export const buildIntervalDescription = (label: string, value: string): ListItems[] => {
return [
{
title: label,
description: <IntervalAbbrScreenReader interval={value} />,
},
];
};
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
buildHighlightedFieldsOverrideDescription,
buildSetupDescription,
getQueryLabel,
buildIntervalDescription,
} from './helpers';
import * as i18n from './translations';
import { buildMlJobsDescription } from './build_ml_jobs_description';
Expand Down Expand Up @@ -342,6 +343,8 @@ export const getDescriptionItem = (
return get('isBuildingBlock', data)
? [{ title: i18n.BUILDING_BLOCK_LABEL, description: i18n.BUILDING_BLOCK_DESCRIPTION }]
: [];
} else if (['interval', 'from'].includes(field)) {
return buildIntervalDescription(label, get(field, data));
} else if (field === 'maxSignals') {
const value: number | undefined = get(field, data);
return value ? [{ title: label, description: value }] : [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React from 'react';
import { EuiDescriptionList, EuiText } from '@elastic/eui';
import type { EuiDescriptionListProps } from '@elastic/eui';
import { IntervalAbbrScreenReader } from '../../../../common/components/accessibility';
import type { RuleResponse } from '../../../../../common/api/detection_engine/model/rule_schema';
import { getHumanizedDuration } from '../../../../detections/pages/detection_engine/rules/helpers';
import { DEFAULT_DESCRIPTION_LIST_COLUMN_WIDTHS } from './constants';
Expand All @@ -19,7 +20,7 @@ interface IntervalProps {

const Interval = ({ interval }: IntervalProps) => (
<EuiText size="s" data-test-subj="intervalPropertyValue">
{interval}
<IntervalAbbrScreenReader interval={interval} />
</EuiText>
);

Expand All @@ -30,7 +31,7 @@ interface FromProps {

const From = ({ from, interval }: FromProps) => (
<EuiText size="s" data-test-subj={`fromPropertyValue-${from}`}>
{getHumanizedDuration(from, interval)}
<IntervalAbbrScreenReader interval={getHumanizedDuration(from, interval)} />
</EuiText>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
TIMELINE_TEMPLATE_DETAILS,
DATA_VIEW_DETAILS,
EDIT_RULE_SETTINGS_LINK,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';
import { GLOBAL_SEARCH_BAR_FILTER_ITEM } from '../../../../screens/search_bar';

Expand Down Expand Up @@ -143,12 +144,16 @@ describe('Custom query rules', { tags: ['@ess', '@serverless'] }, () => {
});
cy.get(DEFINITION_DETAILS).should('not.contain', INDEX_PATTERNS_DETAILS);
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(
rule.from ?? 'now-6m',
rule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
SEVERITY_DETAILS,
TAGS_DETAILS,
TIMELINE_TEMPLATE_DETAILS,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';

import { getDetails, waitForTheRuleToBeExecuted } from '../../../../tasks/rule_details';
Expand Down Expand Up @@ -125,12 +126,16 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => {
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(
rule.from ?? 'now-6m',
rule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
INDICATOR_INDEX_QUERY,
INDICATOR_MAPPING,
INDICATOR_PREFIX_OVERRIDE,
INTERVAL_ABBR_VALUE,
INVESTIGATION_NOTES_MARKDOWN,
INVESTIGATION_NOTES_TOGGLE,
MITRE_ATTACK_DETAILS,
Expand Down Expand Up @@ -478,12 +479,16 @@ describe('indicator match', { tags: ['@ess', '@serverless'] }, () => {
});

cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(
rule.from ?? 'now-6m',
rule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
SEVERITY_DETAILS,
TAGS_DETAILS,
TIMELINE_TEMPLATE_DETAILS,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';

import { getDetails } from '../../../../tasks/rule_details';
Expand Down Expand Up @@ -114,12 +115,16 @@ describe('Machine Learning rules', { tags: ['@ess', '@serverless'] }, () => {
cy.get(MACHINE_LEARNING_JOB_ID).should('have.text', machineLearningJobsArray.join(''));
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${mlRule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${mlRule.interval}`);
const humanizedDuration = getHumanizedDuration(
mlRule.from ?? 'now-6m',
mlRule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
SUPPRESS_BY_DETAILS,
SUPPRESS_FOR_DETAILS,
SUPPRESS_MISSING_FIELD,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';

import { getDetails, waitForTheRuleToBeExecuted } from '../../../../tasks/rule_details';
Expand Down Expand Up @@ -136,12 +137,16 @@ describe(
getDetails(NEW_TERMS_HISTORY_WINDOW_DETAILS).should('have.text', '51000h');
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(
rule.from ?? 'now-6m',
rule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
TAGS_DETAILS,
TIMELINE_TEMPLATE_DETAILS,
TIMESTAMP_OVERRIDE_DETAILS,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';

import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common';
Expand Down Expand Up @@ -139,9 +140,13 @@ describe('Rules override', { tags: ['@ess', '@serverless'] }, () => {
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(rule.from ?? 'now-6m', rule.interval ?? '5m');
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
THRESHOLD_DETAILS,
TIMELINE_TEMPLATE_DETAILS,
SUPPRESS_FOR_DETAILS,
INTERVAL_ABBR_VALUE,
} from '../../../../screens/rule_details';
import { expectNumberOfRules, goToRuleDetailsOf } from '../../../../tasks/alerts_detection_rules';
import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common';
Expand Down Expand Up @@ -134,12 +135,16 @@ describe(
assertDetailsNotExist(SUPPRESS_FOR_DETAILS);
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', `${rule.interval}`);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${rule.interval}`);
const humanizedDuration = getHumanizedDuration(
rule.from ?? 'now-6m',
rule.interval ?? '5m'
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should('have.text', `${humanizedDuration}`);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', `${humanizedDuration}`);
});

waitForTheRuleToBeExecuted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
CUSTOM_QUERY_DETAILS,
DEFINITION_DETAILS,
INDEX_PATTERNS_DETAILS,
INTERVAL_ABBR_VALUE,
INVESTIGATION_NOTES_TOGGLE,
RISK_SCORE_DETAILS,
RULE_NAME_HEADER,
Expand Down Expand Up @@ -149,7 +150,9 @@ describe('Custom query rules', { tags: ['@ess', '@serverless'] }, () => {
});
if (getEditedRule().interval) {
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', getEditedRule().interval);
getDetails(RUNS_EVERY_DETAILS)
.find(INTERVAL_ABBR_VALUE)
.should('have.text', getEditedRule().interval);
});
}
});
Expand Down
Loading

0 comments on commit 99813cf

Please sign in to comment.