Skip to content

Commit

Permalink
Fixes Spaces Menu Keyboard and Screen Reader Navigation (elastic#134454)
Browse files Browse the repository at this point in the history 
* Refactor of spaces pop-over using EuiSelectable.

* Updated to use Eui type instead of interface.

* Reorganization of code, began to add EuiSelectableOnChangeEvent usage.

* Repathed import after rebase

* Integrates the EuiSelectableOnChangeEvent argument to intercept keyboard and mouse event details (shift, ctrl, middle click).

* Resolves missing property error in nav popover test.

* Handles reintroduction of translated strings into empty and no match messages of the Spaces selectable.

* Refactor of spaces_popover_list to use EuiSelectable, resolves keyboard navigation.

* Updated tests for spaces popover list and created more comprehensive tests for nav control popover.
Eliminated tab selection of popover panel itself. Tested screen reader extensively with Chrome on Mac.
Note: Safari screen reader and tabbing behavior is not consistent and differs significantly from Chrome.

* Adjuted focus behavior in popover. Rebased to use EUI 60.1.2

* Fixed typo in roles spaces popover list. Keyboard navigation with options (shift, ctrl, etc.) is not yet working - event argument in selectable onChange is null for keyboard events.

* Updated comments.

* Fixes issue with keyboard operation of manage spaces button of spaces_menu. Fixes naming of translated string resources in spaces_popover_list.

* Fixes name of existing string reference in spaces_menu.

* Fixes CSS selector calls in spaces functional test.

* Fixes space selection in functional tests.

* Fixes popover close issue on Manage spaces button click. Implements check for re-selecting active space. Implements popover close when opening a space in a new window.

* Updated jest snapshot for spaces nav test.

* Fixes ML functional tests to accommodate new behavior when selecting already active space.

* Added active space highlight feedback. Removed state from spaces menu class.

* Rebased, added check mark for active space, resolved activeOption keyboard nav break from EUI update

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Fixes initial active space highlight which was caused by initial empty spaces state in nav_control_popover and loading render logic.
Removes isLoading from spaces_menu - no longer necessary, SpacesDescription is now used during loading.

* Added debug output of actual URL in route expects.

* Added loading message to spaces navigation.

* Updated jest snapshot.

* Addressed flaky test behavior related to space menu nav. Implemented review feedback suggestions.

* Adds sleep to functional test UI interactions.

* Replaced use of any with ExclusiveUnion for search props of EuiSelectable.
Resolved flaky test for spaces nav.

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 31c2c0f)
  • Loading branch information
@jeramysoucy
jeramysoucy committed Aug 2, 2022
1 parent 00145be commit 39cc4c4
Show file tree
Hide file tree
Showing 12 changed files with 562 additions and 344 deletions.
91 changes: 77 additions & 14 deletions ...curity/public/management/roles/edit_role/spaces_popover_list/spaces_popover_list.test.tsx
View file
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
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)
: 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

0 comments on commit 39cc4c4

Please sign in to comment.