Skip to content

Commit

Permalink
fix: [Rules > Detection rules][AXE-CORE]: Buttons must have discernib…
Browse files Browse the repository at this point in the history
…le text (elastic#177273)

Closes: elastic/security-team#8566
Closes: elastic/security-team#8569

## Description
The `<RuleSwitch />` component is currently flagged by the axe browser
plugin for lacking text or an accessible label in its button switch.
This pull request introduces support for the addition of the
`aria-label` attribute to address this issue. Additionally, adjustments
are made in two instances where this component is utilized within the
codebase.

## Screens 

### Axe report 

![image](https://github.com/elastic/kibana/assets/20072247/36287d4f-fd98-4b26-b313-a39a72aefb81)

### A11y label 

![image](https://github.com/elastic/kibana/assets/20072247/c61c9d0d-dd6e-4af2-9d43-04e86ad21954)
  • Loading branch information
alexwizp authored and fkanout committed Mar 4, 2024
1 parent 636c3a7 commit 90b6cf5
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
enabled={isExistingRule && (rule?.enabled ?? false)}
startMlJobsIfNeeded={startMlJobsIfNeeded}
onChange={handleOnChangeEnabledRule}
ruleName={rule?.name}
/>
<EuiFlexItem>{i18n.ENABLE_RULE}</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const useEnabledColumn = ({ hasCRUDPermissions, startMlJobs }: ColumnsProps): Ta
(isMlRule(rule.type) && !hasMlPermissions)
}
isLoading={loadingIds.includes(rule.id)}
ruleName={rule.name}
/>
</EuiToolTip>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,33 @@ describe('RuleSwitch', () => {
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props().checked).toBeFalsy();
});

test('it sets the undefined aria-label for switch if ruleName not passed', () => {
const wrapper = mount(<RuleSwitchComponent enabled={true} id={'7'} />, {
wrappingComponent: TestProviders,
});
expect(
wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props()['aria-label']
).toBeUndefined();
});

test('it sets the correct aria-label for switch if "enabled" is true', () => {
const wrapper = mount(<RuleSwitchComponent enabled={true} id={'7'} ruleName={'test'} />, {
wrappingComponent: TestProviders,
});
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props()['aria-label']).toBe(
'Switch off "test"'
);
});

test('it sets the correct aria-label for switch if "enabled" is false', () => {
const wrapper = mount(<RuleSwitchComponent enabled={false} id={'7'} ruleName={'test'} />, {
wrappingComponent: TestProviders,
});
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props()['aria-label']).toBe(
'Switch on "test"'
);
});

test('it dispatches error toaster if "enableRules" call rejects', async () => {
const mockError = new Error('uh oh');
(performBulkAction as jest.Mock).mockRejectedValue(mockError);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { SINGLE_RULE_ACTIONS } from '../../../../common/lib/apm/user_actions';
import { useStartTransaction } from '../../../../common/lib/apm/use_start_transaction';
import { useExecuteBulkAction } from '../../../../detection_engine/rule_management/logic/bulk_actions/use_execute_bulk_action';
import { useRulesTableContextOptional } from '../../../../detection_engine/rule_management_ui/components/rules_table/rules_table/rules_table_context';
import { ruleSwitchAriaLabel } from './translations';

const StaticSwitch = styled(EuiSwitch)`
.euiSwitch__thumb,
Expand All @@ -31,6 +32,7 @@ export interface RuleSwitchProps {
isLoading?: boolean;
startMlJobsIfNeeded?: () => Promise<void>;
onChange?: (enabled: boolean) => void;
ruleName?: string;
}

/**
Expand All @@ -43,8 +45,10 @@ export const RuleSwitchComponent = ({
enabled,
startMlJobsIfNeeded,
onChange,
ruleName,
}: RuleSwitchProps) => {
const [myIsLoading, setMyIsLoading] = useState(false);
const ariaLabel = ruleName ? ruleSwitchAriaLabel(ruleName, enabled) : undefined;
const rulesTableContext = useRulesTableContextOptional();
const { startTransaction } = useStartTransaction();
const { executeBulkAction } = useExecuteBulkAction({ suppressSuccessToast: !rulesTableContext });
Expand Down Expand Up @@ -93,6 +97,7 @@ export const RuleSwitchComponent = ({
disabled={isDisabled}
checked={enabled}
onChange={onRuleStateChange}
aria-label={ariaLabel}
/>
)}
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* 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 ruleSwitchAriaLabel = (name: string, isActive: boolean) =>
i18n.translate('xpack.securitySolution.ruleDetails.ruleSwitch.ariaLabel', {
values: {
name,
action: isActive
? i18n.translate('xpack.securitySolution.ruleDetails.ruleSwitch.ariaLabel.switchOff', {
defaultMessage: 'Switch off',
})
: i18n.translate('xpack.securitySolution.ruleDetails.ruleSwitch.ariaLabel.switchOn', {
defaultMessage: 'Switch on',
}),
},
defaultMessage: '{action} "{name}"',
});

0 comments on commit 90b6cf5

Please sign in to comment.