Skip to content

Commit

Permalink
### [Security Solution] [Security Assistant] Fixes Security Assistant…
Browse files Browse the repository at this point in the history
… accessibility (a11y) issues

This PR fixes the following Security Assistant accessibility (a11y) issues:

- <elastic#206348> - _The ai assistant settings and actions button is announced wrong_
- <elastic#206362> - _Close button on View in AI assistant is missing discernible text_
- <elastic#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_

### Details

#### [206348](elastic#206348) - The ai assistant settings and actions button is announced wrong

This issue was resolved by adding an `aria-label` to the assistant settings context menu.

This fix was desk tested using Voiceover, as illustrated by the following screenshots:

**Before:**

![voiceover_before_206348](https://github.com/user-attachments/assets/92106bd9-b651-447e-b5dd-f59323288534)

**After:**

![voiceover_after_206348](https://github.com/user-attachments/assets/da580121-fab1-47e8-ae7b-41fd6d0008ca)

Desk testing: see [206348](elastic#206348) for reproduction steps

#### [206362](elastic#206362) - Close button on View in AI assistant is missing discernible text

This issue was resolved by adding an `aria-label` to the assistant close button.

This fix was desk tested using Axe, as illustrated by the following screenshots:

**Before:**

![axe_before_206362](https://github.com/user-attachments/assets/21503311-a9e0-402f-9ee0-333ad6d6171a)

**After:**

![axe_after_206362](https://github.com/user-attachments/assets/54565a48-4285-47f2-b3fd-3709feb9b57c)

Desk testing: see [206362](elastic#206362) for reproduction steps

#### [206875](elastic#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled

Issue [206875](elastic#206875) includes the following statement:

> Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now "show anonymized"

The first part of the statement above:

> Anonymization button doesn't get announced

appears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/>

The second part of the statement above:

> doesn't have enough context in the tooltip about when it gets enabled

is addressed by this PR, though there is still a quirk described in detail below.

In this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below:

**Before:**

![empty_before_206875](https://github.com/user-attachments/assets/682f6269-d3db-40ee-877e-e877e9b1ae31)

_Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized`

**After:**

![empty_after_206875](https://github.com/user-attachments/assets/1eed6a88-c3d2-424a-abc0-ef45b9ee41d5)

_Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields`

Note that there is still a quirk with the button, which is not addressed by this fix:

The current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation.

Alternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR.

Desk testing: see [206875](elastic#206875) for reproduction steps
  • Loading branch information
andrew-goldstein committed Jan 17, 2025
1 parent 032f072 commit aac95fd
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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 { getAnonymizationTooltip } from '.';
import {
SHOW_ANONYMIZED,
SHOW_REAL_VALUES,
THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS,
} from '../translations';

describe('getAnonymizationTooltip', () => {
it('returns the expected tooltip when conversationHasReplacements is false', () => {
const result = getAnonymizationTooltip({
conversationHasReplacements: false, // <-- false
showAnonymizedValuesChecked: false,
});

expect(result).toBe(THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS);
});

it('returns SHOW_REAL_VALUES when showAnonymizedValuesChecked is true', () => {
const result = getAnonymizationTooltip({
conversationHasReplacements: true,
showAnonymizedValuesChecked: true, // <-- true
});

expect(result).toBe(SHOW_REAL_VALUES);
});

it('returns SHOW_ANONYMIZED when showAnonymizedValuesChecked is false', () => {
const result = getAnonymizationTooltip({
conversationHasReplacements: true,
showAnonymizedValuesChecked: false, // <-- false
});

expect(result).toBe(SHOW_ANONYMIZED);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* 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 * as i18n from '../translations';

export const getAnonymizationTooltip = ({
conversationHasReplacements,
showAnonymizedValuesChecked,
}: {
conversationHasReplacements: boolean;
showAnonymizedValuesChecked: boolean;
}): string => {
if (!conversationHasReplacements) {
return i18n.THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS;
}

return showAnonymizedValuesChecked ? i18n.SHOW_REAL_VALUES : i18n.SHOW_ANONYMIZED;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,20 @@
*/

import React from 'react';
import { act, fireEvent, render } from '@testing-library/react';
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import userEvent, { PointerEventsCheckLevel } from '@testing-library/user-event';

import { AssistantHeader } from '.';
import { TestProviders } from '../../mock/test_providers/test_providers';
import { alertConvo, emptyWelcomeConvo, welcomeConvo } from '../../mock/conversation';
import { useLoadConnectors } from '../../connectorland/use_load_connectors';
import { mockConnectors } from '../../mock/connectors';
import {
CLOSE,
SHOW_ANONYMIZED,
SHOW_REAL_VALUES,
THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS,
} from './translations';

const onConversationSelected = jest.fn();
const mockConversations = {
Expand Down Expand Up @@ -139,4 +147,103 @@ describe('AssistantHeader', () => {
cTitle: alertConvo.title,
});
});

it('renders an accessible close button icon', () => {
const onCloseFlyout = jest.fn(); // required to render the close button

render(<AssistantHeader {...testProps} onCloseFlyout={onCloseFlyout} />, {
wrapper: TestProviders,
});

expect(screen.getByRole('button', { name: CLOSE })).toBeInTheDocument();
});

it('disables the anonymization toggle button when there are NO replacements', () => {
render(
<AssistantHeader
{...testProps}
selectedConversation={{ ...emptyWelcomeConvo, replacements: {} }} // <-- no replacements
/>,
{
wrapper: TestProviders,
}
);

expect(screen.getByTestId('showAnonymizedValues')).toBeDisabled();
});

it('displays the expected anonymization toggle button tooltip when there are NO replacements', async () => {
render(
<AssistantHeader
{...testProps}
selectedConversation={{ ...emptyWelcomeConvo, replacements: {} }} // <-- no replacements
/>,
{
wrapper: TestProviders,
}
);

await userEvent.hover(screen.getByTestId('showAnonymizedValues'), {
pointerEventsCheck: PointerEventsCheckLevel.Never,
});

await waitFor(() => {
expect(screen.getByTestId('showAnonymizedValuesTooltip')).toHaveTextContent(
THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS
);
});
});

it('enables the anonymization toggle button when there are replacements', () => {
render(
<AssistantHeader {...testProps} selectedConversation={alertConvo} />, // <-- conversation with replacements
{
wrapper: TestProviders,
}
);

expect(screen.getByTestId('showAnonymizedValues')).toBeEnabled();
});

it('displays the SHOW_ANONYMIZED toggle button tooltip when there are replacements and showAnonymizedValues is false', async () => {
render(
<AssistantHeader
{...testProps}
selectedConversation={alertConvo} // <-- conversation with replacements
showAnonymizedValues={false} // <-- false
/>,
{
wrapper: TestProviders,
}
);

await userEvent.hover(screen.getByTestId('showAnonymizedValues'), {
pointerEventsCheck: PointerEventsCheckLevel.Never,
});

await waitFor(() => {
expect(screen.getByTestId('showAnonymizedValuesTooltip')).toHaveTextContent(SHOW_ANONYMIZED);
});
});

it('displays the SHOW_REAL_VALUES toggle button tooltip when there are replacements and showAnonymizedValues is true', async () => {
render(
<AssistantHeader
{...testProps}
selectedConversation={alertConvo} // <-- conversation with replacements
showAnonymizedValues={true} // <-- true
/>,
{
wrapper: TestProviders,
}
);

await userEvent.hover(screen.getByTestId('showAnonymizedValues'), {
pointerEventsCheck: PointerEventsCheckLevel.Never,
});

await waitFor(() => {
expect(screen.getByTestId('showAnonymizedValuesTooltip')).toHaveTextContent(SHOW_REAL_VALUES);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { FlyoutNavigation } from '../assistant_overlay/flyout_navigation';
import { AssistantSettingsModal } from '../settings/assistant_settings_modal';
import * as i18n from './translations';
import { AIConnector } from '../../connectorland/connector_selector';
import { getAnonymizationTooltip } from './get_anonymization_tooltip';
import { SettingsContextMenu } from '../settings/settings_context_menu/settings_context_menu';

interface OwnProps {
Expand Down Expand Up @@ -104,6 +105,12 @@ export const AssistantHeader: React.FC<Props> = ({
[onConversationSelected]
);

const conversationHasReplacements = !isEmpty(selectedConversation?.replacements);
const anonymizationTooltip = getAnonymizationTooltip({
conversationHasReplacements,
showAnonymizedValuesChecked,
});

return (
<>
<FlyoutNavigation
Expand Down Expand Up @@ -136,6 +143,7 @@ export const AssistantHeader: React.FC<Props> = ({
{onCloseFlyout && (
<EuiFlexItem grow={false}>
<EuiButtonIcon
aria-label={i18n.CLOSE}
data-test-subj="euiFlyoutCloseButton"
iconType="cross"
color="text"
Expand Down Expand Up @@ -184,9 +192,8 @@ export const AssistantHeader: React.FC<Props> = ({
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiToolTip
content={
showAnonymizedValuesChecked ? i18n.SHOW_REAL_VALUES : i18n.SHOW_ANONYMIZED
}
content={anonymizationTooltip}
data-test-subj="showAnonymizedValuesTooltip"
>
<EuiButtonIcon
css={css`
Expand All @@ -195,12 +202,10 @@ export const AssistantHeader: React.FC<Props> = ({
display="base"
data-test-subj="showAnonymizedValues"
isSelected={showAnonymizedValuesChecked}
aria-label={
showAnonymizedValuesChecked ? i18n.SHOW_ANONYMIZED : i18n.SHOW_REAL_VALUES
}
aria-label={anonymizationTooltip}
iconType={showAnonymizedValuesChecked ? 'eye' : 'eyeClosed'}
onClick={onToggleShowAnonymizedValues}
isDisabled={isEmpty(selectedConversation?.replacements)}
disabled={!conversationHasReplacements}
/>
</EuiToolTip>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ export const ANONYMIZATION = i18n.translate(
}
);

export const CLOSE = i18n.translate(
'xpack.elasticAssistant.assistant.assistantHeader.closeButtonLabel',
{
defaultMessage: 'Close',
}
);

export const KNOWLEDGE_BASE = i18n.translate(
'xpack.elasticAssistant.assistant.settings.knowledgeBase',
{
Expand Down Expand Up @@ -56,6 +63,13 @@ export const SHOW_REAL_VALUES = i18n.translate(
}
);

export const THIS_CONVERSATION_DOES_NOT_INCLUDE_ANONYMIZED_FIELDS = i18n.translate(
'xpack.elasticAssistant.assistant.settings.thisConversationDoesNotIncludeAnonymizedFieldsTooltip',
{
defaultMessage: 'This conversation does not include anonymized fields',
}
);

export const CANCEL_BUTTON_TEXT = i18n.translate(
'xpack.elasticAssistant.assistant.resetConversationModal.cancelButtonText',
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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 { render, screen } from '@testing-library/react';
import React from 'react';

import { TestProviders } from '../../../mock/test_providers/test_providers';
import { SettingsContextMenu } from './settings_context_menu';
import { AI_ASSISTANT_MENU } from './translations';

describe('SettingsContextMenu', () => {
it('renders an accessible menu button icon', () => {
render(
<TestProviders>
<SettingsContextMenu />
</TestProviders>
);

expect(screen.getByRole('button', { name: AI_ASSISTANT_MENU })).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { useAssistantContext } from '../../../..';
import * as i18n from '../../assistant_header/translations';
import { AlertsSettingsModal } from '../alerts_settings/alerts_settings_modal';
import { KNOWLEDGE_BASE_TAB } from '../const';
import { AI_ASSISTANT_MENU } from './translations';

interface Params {
isDisabled?: boolean;
Expand Down Expand Up @@ -170,7 +171,7 @@ export const SettingsContextMenu: React.FC<Params> = React.memo(
button={
<KnowledgeBaseTour>
<EuiButtonIcon
aria-label="test"
aria-label={AI_ASSISTANT_MENU}
isDisabled={isDisabled}
iconType="boxesVertical"
onClick={onButtonClick}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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 AI_ASSISTANT_MENU = i18n.translate(
'xpack.elasticAssistant.assistant.settings.settingsContextMenu.aiAssistantMenuAriaLabel',
{
defaultMessage: 'AI Assistant menu',
}
);

0 comments on commit aac95fd

Please sign in to comment.