Skip to content

Commit

Permalink
fix: [Obs Alerts > Rules][KEYBOARD]: State dropdown menu is not keybo…
Browse files Browse the repository at this point in the history 
…ard accessible (elastic#183509)

Closes: elastic/observability-dev#3358

## Description

The Obs Alert Rules view has a `State` dropdown menu that cannot be
accessed by keyboard. I've included a MOV file that shows the keypress
events I tried to interact with the menu.

### Steps to recreate

1. Open the [Obs Alerts
Rules](https://keepserverless-qa-oblt-b4ba07.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts/rules)
table
2. Tab to the `State` button
3. Press `Enter` to open the menu
4. Click `Tab` and `Down_Arrow` to verify no action is being taken
5. Hover over the menu options and click one with a mouse to verify
action is being taken

### What was changed?: 
1. EuiSelectableListItem was replace to EuiSelectable

### Screen:


https://github.com/elastic/kibana/assets/20072247/befc9c75-9313-416a-be64-cc0b67f97a84
  • Loading branch information
@alexwizp
alexwizp authored Jun 20, 2024
1 parent 1989a60 commit 963a178
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 73 deletions.
124 changes: 73 additions & 51 deletions ...ggers_actions_ui/public/application/sections/rules_list/components/rule_status_filter.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React, { useState, useCallback } from 'react';
import React, { useState, useCallback, useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiFilterButton, EuiPopover, EuiFilterGroup, EuiSelectableListItem } from '@elastic/eui';
import { RuleStatus } from '../../../../types';

const statuses: RuleStatus[] = ['enabled', 'disabled', 'snoozed'];

const getOptionDataTestSubj = (status: RuleStatus) => `ruleStatusFilterOption-${status}`;
import {
EuiFilterButton,
EuiPopover,
EuiFilterGroup,
EuiSelectable,
type EuiSelectableOption,
type EuiSelectableProps,
} from '@elastic/eui';
import type { RuleStatus } from '../../../../types';

export interface RuleStatusFilterProps {
selectedStatuses: RuleStatus[];
Expand All @@ -22,6 +26,38 @@ export interface RuleStatusFilterProps {
onChange: (selectedStatuses: RuleStatus[]) => void;
}

const ruleStateFilterOptions: Array<{ key: RuleStatus; label: string }> = [
{
key: 'enabled',
label: i18n.translate(
'xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.enabledOptionText',
{
defaultMessage: 'Rule is enabled',
}
),
},
{
key: 'disabled',
label: i18n.translate(
'xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.disabledOptionText',
{
defaultMessage: 'Rule is disabled',
}
),
},
{
key: 'snoozed',
label: i18n.translate(
'xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.snoozedOptionText',
{
defaultMessage: 'Rule has snoozed',
}
),
},
];

const getOptionDataTestSubj = (status: RuleStatus) => `ruleStatusFilterOption-${status}`;

export const RuleStatusFilter = (props: RuleStatusFilterProps) => {
const {
selectedStatuses = [],
Expand All @@ -34,45 +70,31 @@ export const RuleStatusFilter = (props: RuleStatusFilterProps) => {

const [isPopoverOpen, setIsPopoverOpen] = useState<boolean>(false);

const onFilterItemClick = useCallback(
(newOption: RuleStatus) => () => {
if (selectedStatuses.includes(newOption)) {
onChange(selectedStatuses.filter((option) => option !== newOption));
return;
}
onChange([...selectedStatuses, newOption]);
const onFilterItemChange: EuiSelectableProps['onChange'] = useCallback(
(newOptions: EuiSelectableOption[]) => {
const selected = newOptions
.filter(({ checked }) => checked === 'on')
.map(({ key }) => key as RuleStatus);

onChange(selected);
},
[selectedStatuses, onChange]
[onChange]
);

const onClick = useCallback(() => {
setIsPopoverOpen((prevIsOpen) => !prevIsOpen);
}, [setIsPopoverOpen]);

const renderRuleStateOptions = (status: 'enabled' | 'disabled' | 'snoozed') => {
if (status === 'enabled') {
return (
<FormattedMessage
id="xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.enabledOptionText"
defaultMessage="Rule is enabled"
/>
);
} else if (status === 'disabled') {
return (
<FormattedMessage
id="xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.disabledOptionText"
defaultMessage="Rule is disabled"
/>
);
} else if (status === 'snoozed') {
return (
<FormattedMessage
id="xpack.triggersActionsUI.sections.ruleDetails.ruleStateFilter.snoozedOptionText"
defaultMessage="Rule has snoozed"
/>
);
}
};
const selectableOptions = useMemo<EuiSelectableOption[]>(
() =>
ruleStateFilterOptions.map(({ key, label }) => ({
key,
label,
'data-test-subj': optionDataTestSubj(key),
checked: selectedStatuses.includes(key) ? 'on' : undefined,
})),
[optionDataTestSubj, selectedStatuses]
);

return (
<EuiFilterGroup data-test-subj={dataTestSubj}>
Expand All @@ -96,18 +118,18 @@ export const RuleStatusFilter = (props: RuleStatusFilterProps) => {
}
>
<div data-test-subj={selectDataTestSubj}>
{statuses.map((status) => {
return (
<EuiSelectableListItem
key={status}
data-test-subj={optionDataTestSubj(status)}
onClick={onFilterItemClick(status)}
checked={selectedStatuses.includes(status) ? 'on' : undefined}
>
{renderRuleStateOptions(status)}
</EuiSelectableListItem>
);
})}
<EuiSelectable
options={selectableOptions}
onChange={onFilterItemChange}
listProps={{
bordered: false,
style: {
minWidth: 300,
},
}}
>
{(list) => list}
</EuiSelectable>
</div>
</EuiPopover>
</EuiFilterGroup>
Expand Down
5 changes: 3 additions & 2 deletions ...triggers_actions_ui/public/application/sections/rules_list/components/rules_list.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,13 +397,14 @@ describe('rules_list ', () => {
);
fireEvent.click((await screen.findAllByTestId('ruleStatusFilterButton'))[0]);
fireEvent.click((await screen.findAllByTestId('ruleStatusFilterOption-enabled'))[0]);

expect(loadRulesWithKueryFilter).toHaveBeenLastCalledWith(
expect.objectContaining({
ruleStatusesFilter: ['disabled', 'enabled'],
ruleStatusesFilter: ['enabled', 'disabled'],
})
);
expect(onStatusFilterChangeMock).toHaveBeenCalled();
expect(onStatusFilterChangeMock).toHaveBeenLastCalledWith(['disabled', 'enabled']);
expect(onStatusFilterChangeMock).toHaveBeenLastCalledWith(['enabled', 'disabled']);
});

it('can filter by last response', async () => {
Expand Down
53 changes: 33 additions & 20 deletions x-pack/test_serverless/functional/test_suites/observability/rules/rules_list.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
const find = getService('find');
const retry = getService('retry');
const toasts = getService('toasts');
const log = getService('log');
const svlCommonApi = getService('svlCommonApi');
const svlUserManager = getService('svlUserManager');
const supertestWithoutAuth = getService('supertestWithoutAuth');
Expand Down Expand Up @@ -711,6 +712,24 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
});

it('should filter rules by the rule status', async () => {
const setRuleStatus = async (testSubj: string, checked: boolean) => {
const readAriaChecked = async (): Promise<boolean> => {
const statusItem = await testSubjects.find(testSubj);
return statusItem
? JSON.parse((await statusItem.getAttribute('aria-checked')) || 'null')
: null;
};

await retry.try(async () => {
if ((await readAriaChecked()) !== checked) {
await testSubjects.click(testSubj);
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
}

expect(await readAriaChecked()).toEqual(checked);
});
};

// Enabled alert
const rule1 = await createRule({
supertest,
Expand Down Expand Up @@ -750,39 +769,33 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
await refreshRulesList();
await assertRulesLength(4);

// Select only enabled
log.debug('ruleStatusFilterButton: Select only enabled');
await testSubjects.click('ruleStatusFilterButton');
await testSubjects.click('ruleStatusFilterOption-enabled');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await setRuleStatus('ruleStatusFilterOption-enabled', true);
await assertRulesLength(2);

// Select enabled or disabled (e.g. all)
await testSubjects.click('ruleStatusFilterOption-disabled');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
log.debug('ruleStatusFilterButton: Select enabled or disabled (e.g. all)');
await setRuleStatus('ruleStatusFilterOption-disabled', true);
await assertRulesLength(4);

// Select only disabled
await testSubjects.click('ruleStatusFilterOption-enabled');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
log.debug('ruleStatusFilterButton: Select only disabled');
await setRuleStatus('ruleStatusFilterOption-enabled', false);
await assertRulesLength(2);

// Select only snoozed
await testSubjects.click('ruleStatusFilterOption-disabled');
await testSubjects.click('ruleStatusFilterOption-snoozed');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
log.debug('ruleStatusFilterButton: Select only snoozed');
await setRuleStatus('ruleStatusFilterOption-disabled', false);
await setRuleStatus('ruleStatusFilterOption-snoozed', true);
await assertRulesLength(2);

// Select disabled or snoozed
await testSubjects.click('ruleStatusFilterOption-disabled');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
log.debug('ruleStatusFilterButton: Select disabled or snoozed');
await setRuleStatus('ruleStatusFilterOption-disabled', true);
await assertRulesLength(3);

// Select enabled or disabled or snoozed
await testSubjects.click('ruleStatusFilterOption-enabled');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
log.debug('ruleStatusFilterButton: Select enabled or disabled or snoozed');
await setRuleStatus('ruleStatusFilterOption-enabled', true);
await assertRulesLength(4);

// Clear it again because it is still selected
log.debug('ruleStatusFilterButton: Clear it again because it is still selected');
await testSubjects.click('rules-list-clear-filter');
await find.waitForDeletedByCssSelector('.euiBasicTable-loading');
await assertRulesLength(4);
Expand Down

0 comments on commit 963a178

Please sign in to comment.