Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

Merged
merged 32 commits into from
Aug 2, 2022
Merged

Fixes Spaces Menu Keyboard and Screen Reader Navigation #134454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7c1c797
Refactor of spaces pop-over using EuiSelectable.
jeramysoucy Jun 15, 2022
7a9df0d
Updated to use Eui type instead of interface.
jeramysoucy Jun 15, 2022
d7b2ac2
Reorganization of code, began to add EuiSelectableOnChangeEvent usage.
jeramysoucy Jun 16, 2022
4c139fc
Repathed import after rebase
jeramysoucy Jun 16, 2022
733fbc1
Integrates the EuiSelectableOnChangeEvent argument to intercept keybo…
jeramysoucy Jul 5, 2022
de8a7d2
Resolves missing property error in nav popover test.
jeramysoucy Jul 5, 2022
6c17696
Handles reintroduction of translated strings into empty and no match …
jeramysoucy Jul 6, 2022
15d45f9
Refactor of spaces_popover_list to use EuiSelectable, resolves keyboa…
jeramysoucy Jul 7, 2022
5f63340
Updated tests for spaces popover list and created more comprehensive …
jeramysoucy Jul 11, 2022
66a5d16
Adjuted focus behavior in popover. Rebased to use EUI 60.1.2
jeramysoucy Jul 12, 2022
81745b8
Fixed typo in roles spaces popover list. Keyboard navigation with opt…
jeramysoucy Jul 12, 2022
abc8ae4
Updated comments.
jeramysoucy Jul 13, 2022
762eeff
Fixes issue with keyboard operation of manage spaces button of spaces…
jeramysoucy Jul 14, 2022
d44cfc1
Fixes name of existing string reference in spaces_menu.
jeramysoucy Jul 14, 2022
daff0f5
Fixes CSS selector calls in spaces functional test.
jeramysoucy Jul 14, 2022
593865b
Fixes space selection in functional tests.
jeramysoucy Jul 18, 2022
ee47622
Fixes popover close issue on Manage spaces button click. Implements c…
jeramysoucy Jul 18, 2022
ef9f68a
Updated jest snapshot for spaces nav test.
jeramysoucy Jul 18, 2022
53858e4
Fixes ML functional tests to accommodate new behavior when selecting …
jeramysoucy Jul 19, 2022
db3778e
Added active space highlight feedback. Removed state from spaces menu…
jeramysoucy Jul 19, 2022
6c17261
Rebased, added check mark for active space, resolved activeOption key…
jeramysoucy Jul 21, 2022
d7a5b0c
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 21, 2022
c74286a
Fixes initial active space highlight which was caused by initial empt…
jeramysoucy Jul 22, 2022
f52def9
Added debug output of actual URL in route expects.
jeramysoucy Jul 22, 2022
9c5dad1
Added loading message to spaces navigation.
jeramysoucy Jul 25, 2022
223813e
Updated jest snapshot.
jeramysoucy Jul 26, 2022
b2dfb91
Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav
kibanamachine Jul 26, 2022
0ef6330
Addressed flaky test behavior related to space menu nav. Implemented …
jeramysoucy Jul 29, 2022
1949640
Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav
kibanamachine Jul 29, 2022
77fab8b
Adds sleep to functional test UI interactions.
jeramysoucy Aug 1, 2022
d623e6f
Merge branch 'main' into fix-spaces-menu-kb-and-screen-reader-nav
kibanamachine Aug 1, 2022
712798e
Replaced use of any with ExclusiveUnion for search props of EuiSelect…
jeramysoucy Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 77 additions & 14 deletions ...curity/public/management/roles/edit_role/spaces_popover_list/spaces_popover_list.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@

import {
EuiButtonEmpty,
EuiContextMenuItem,
EuiContextMenuPanel,
EuiFieldSearch,
EuiPopover,
EuiSelectable,
EuiSelectableListItem,
} from '@elastic/eui';
import { act } from '@testing-library/react';
import React from 'react';
Expand Down Expand Up @@ -66,27 +67,89 @@ describe('SpacesPopoverList', () => {
expect(wrapper.find(EuiContextMenuPanel)).toHaveLength(0);
});

it('clicking the button renders a context menu with the provided spaces', async () => {
it('clicking the button renders an EuiSelectable menu with the provided spaces', async () => {
const wrapper = await setup(mockSpaces);
await act(async () => {
wrapper.find(EuiButtonEmpty).simulate('click');
});
wrapper.update();

const menu = wrapper.find(EuiContextMenuPanel);
expect(menu).toHaveLength(1);
await act(async () => {
const menu = wrapper.find(EuiSelectable);
expect(menu).toHaveLength(1);

const items = menu.find(EuiContextMenuItem);
expect(items).toHaveLength(mockSpaces.length);
const items = menu.find(EuiSelectableListItem);
expect(items).toHaveLength(mockSpaces.length);

mockSpaces.forEach((space, index) => {
const spaceAvatar = items.at(index).find(SpaceAvatarInternal);
expect(spaceAvatar.props().space).toEqual(space);
mockSpaces.forEach((space, index) => {
const spaceAvatar = items.at(index).find(SpaceAvatarInternal);
expect(spaceAvatar.props().space).toEqual(space);
});
});
});

it('Should NOT render a search box when there is less than 8 spaces', async () => {
const wrapper = await setup(mockSpaces);
it('should render a search box when there are 8 or more spaces', async () => {
const eightSpaces = mockSpaces.concat([
{
id: 'space-3',
name: 'Space-3',
disabledFeatures: [],
},
{
id: 'space-4',
name: 'Space 4',
disabledFeatures: [],
},
{
id: 'space-5',
name: 'Space 5',
disabledFeatures: [],
},
{
id: 'space-6',
name: 'Space 6',
disabledFeatures: [],
},
{
id: 'space-7',
name: 'Space 7',
disabledFeatures: [],
},
]);
const wrapper = await setup(eightSpaces);
await act(async () => {
wrapper.find(EuiButtonEmpty).simulate('click');
});
wrapper.update();

expect(wrapper.find(EuiFieldSearch)).toHaveLength(1);
});

it('should NOT render a search box when there are less than 8 spaces', async () => {
const sevenSpaces = mockSpaces.concat([
{
id: 'space-3',
name: 'Space-3',
disabledFeatures: [],
},
{
id: 'space-4',
name: 'Space 4',
disabledFeatures: [],
},
{
id: 'space-5',
name: 'Space 5',
disabledFeatures: [],
},
{
id: 'space-6',
name: 'Space 6',
disabledFeatures: [],
},
]);

const wrapper = await setup(sevenSpaces);
await act(async () => {
wrapper.find(EuiButtonEmpty).simulate('click');
});
Expand All @@ -101,11 +164,11 @@ describe('SpacesPopoverList', () => {
wrapper.find(EuiButtonEmpty).simulate('click');
});
wrapper.update();

expect(wrapper.find(EuiPopover).props().isOpen).toEqual(true);

wrapper.find(EuiPopover).props().closePopover();

await act(async () => {
wrapper.find(EuiPopover).props().closePopover();
});
wrapper.update();

expect(wrapper.find(EuiPopover).props().isOpen).toEqual(false);
Expand Down
212 changes: 83 additions & 129 deletions ...ns/security/public/management/roles/edit_role/spaces_popover_list/spaces_popover_list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

import './spaces_popover_list.scss';

import type { EuiSelectableOption } from '@elastic/eui';
import {
EuiButtonEmpty,
EuiContextMenuItem,
EuiContextMenuPanel,
EuiFieldSearch,
EuiFocusTrap,
EuiLoadingSpinner,
EuiPopover,
EuiPopoverTitle,
EuiSelectable,
EuiText,
} from '@elastic/eui';
import React, { Component, memo } from 'react';
import React, { Component, memo, Suspense } from 'react';

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
Expand All @@ -29,14 +31,12 @@ interface Props {
}

interface State {
searchTerm: string;
allowSpacesListFocus: boolean;
isPopoverOpen: boolean;
}

export class SpacesPopoverList extends Component<Props, State> {
public state = {
searchTerm: '',
allowSpacesListFocus: false,
isPopoverOpen: false,
};
Expand All @@ -56,152 +56,106 @@ export class SpacesPopoverList extends Component<Props, State> {
closePopover={this.closePopover}
panelPaddingSize="none"
anchorPosition="downLeft"
ownFocus
ownFocus={false}
>
{this.getMenuPanel()}
<EuiFocusTrap>{this.getMenuPanel()}</EuiFocusTrap>
</EuiPopover>
);
}

private getMenuPanel = () => {
const { searchTerm } = this.state;

const items = this.getVisibleSpaces(searchTerm).map(this.renderSpaceMenuItem);

const panelProps = {
className: 'spcMenu',
title: i18n.translate('xpack.security.management.editRole.spacesPopoverList.popoverTitle', {
defaultMessage: 'Spaces',
}),
};

if (this.props.spaces.length >= SPACE_SEARCH_COUNT_THRESHOLD) {
return (
<EuiContextMenuPanel {...panelProps}>
{this.renderSearchField()}
{this.renderSpacesListPanel(items, searchTerm)}
</EuiContextMenuPanel>
);
}
const options = this.getSpaceOptions();

const noSpacesMessage = (
<EuiText color="subdued" className="eui-textCenter">
<FormattedMessage
id="xpack.security.management.editRole.spacesPopoverList.noSpacesFoundTitle"
defaultMessage=" no spaces found "
/>
</EuiText>
);

return <EuiContextMenuPanel {...panelProps} items={items} />;
return (
<EuiSelectable
className={'spcMenu'}
title={i18n.translate('xpack.security.management.editRole.spacesPopoverList.popoverTitle', {
defaultMessage: 'Spaces',
})}
searchable={this.props.spaces.length >= SPACE_SEARCH_COUNT_THRESHOLD}
searchProps={
this.props.spaces.length >= SPACE_SEARCH_COUNT_THRESHOLD
? ({
placeholder: i18n.translate(
'xpack.security.management.editRole.spacesPopoverList.findSpacePlaceholder',
{
defaultMessage: 'Find a space',
}
),
compressed: true,
isClearable: true,
id: 'spacesPopoverListSearch',
} as any)
jeramysoucy marked this conversation as resolved.
Show resolved Hide resolved
: undefined
}
noMatchesMessage={noSpacesMessage}
options={options}
singleSelection={true}
style={{ width: 300 }}
listProps={{
rowHeight: 40,
showIcons: false,
onFocusBadge: false,
}}
>
{(list, search) => (
<>
<EuiPopoverTitle paddingSize="s">
{i18n.translate(
'xpack.security.management.editRole.spacesPopoverList.selectSpacesTitle',
{
defaultMessage: 'Spaces',
}
)}
</EuiPopoverTitle>
{search}
{list}
</>
)}
</EuiSelectable>
);
};

private onButtonClick = () => {
this.setState({
isPopoverOpen: !this.state.isPopoverOpen,
searchTerm: '',
});
};

private closePopover = () => {
this.setState({
isPopoverOpen: false,
searchTerm: '',
});
};

private getVisibleSpaces = (searchTerm: string): Space[] => {
const { spaces } = this.props;

let filteredSpaces = spaces;
if (searchTerm) {
filteredSpaces = spaces.filter((space) => {
const { name, description = '' } = space;
return (
name.toLowerCase().indexOf(searchTerm) >= 0 ||
description.toLowerCase().indexOf(searchTerm) >= 0
);
});
}

return filteredSpaces;
};
private getSpaceOptions = (): EuiSelectableOption[] => {
const LazySpaceAvatar = memo(this.props.spacesApiUi.components.getSpaceAvatar);

private renderSpacesListPanel = (items: JSX.Element[], searchTerm: string) => {
if (items.length === 0) {
return (
<EuiText color="subdued" className="eui-textCenter">
<FormattedMessage
id="xpack.security.management.editRole.spacesPopoverList.noSpacesFoundTitle"
defaultMessage=" no spaces found "
/>
</EuiText>
return this.props.spaces.map((space) => {
const icon = (
<Suspense fallback={<EuiLoadingSpinner size="m" />}>
<LazySpaceAvatar space={space} size={'s'} announceSpaceName={false} />
</Suspense>
);
}

return (
<EuiContextMenuPanel
key={`spcMenuList`}
data-search-term={searchTerm}
className="spcMenu__spacesList"
initialFocusedItemIndex={this.state.allowSpacesListFocus ? 0 : undefined}
items={items}
/>
);
};

private renderSearchField = () => {
return (
<div key="manageSpacesSearchField" className="spcMenu__searchFieldWrapper">
{
<EuiFieldSearch
placeholder={i18n.translate(
'xpack.security.management.editRole.spacesPopoverList.findSpacePlaceholder',
{
defaultMessage: 'Find a space',
}
)}
incremental={true}
onSearch={this.onSearch}
onKeyDown={this.onSearchKeyDown}
onFocus={this.onSearchFocus}
compressed
/>
}
</div>
);
};

private onSearchKeyDown = (e: any) => {
// 9: tab
// 13: enter
// 40: arrow-down
const focusableKeyCodes = [9, 13, 40];

const keyCode = e.keyCode;
if (focusableKeyCodes.includes(keyCode)) {
// Allows the spaces list panel to recieve focus. This enables keyboard and screen reader navigation
this.setState({
allowSpacesListFocus: true,
});
}
};

private onSearchFocus = () => {
this.setState({
allowSpacesListFocus: false,
});
};

private onSearch = (searchTerm: string) => {
this.setState({
searchTerm: searchTerm.trim().toLowerCase(),
return {
'aria-label': space.name,
'aria-roledescription': 'space',
label: space.name,
key: space.id,
prepend: icon,
checked: undefined,
'data-test-subj': `${space.id}-selectableSpaceItem`,
className: 'selectableSpaceItem',
};
});
};

private renderSpaceMenuItem = (space: Space): JSX.Element => {
const LazySpaceAvatar = memo(this.props.spacesApiUi.components.getSpaceAvatar);
const icon = <LazySpaceAvatar space={space} size={'s'} />; // wrapped in a Suspense above
return (
<EuiContextMenuItem
key={space.id}
icon={icon}
toolTipTitle={space.description && space.name}
toolTipContent={space.description}
>
{space.name}
</EuiContextMenuItem>
);
};
}
Loading