Skip to content

Commit

Permalink
chore: Fixes embedded multiselect focus management (#2896)
Browse files Browse the repository at this point in the history
  • Loading branch information
pan-kot authored Oct 18, 2024
1 parent b88a10d commit cc75463
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 11 deletions.
49 changes: 45 additions & 4 deletions src/multiselect/__tests__/multiselect-embedded.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import * as React from 'react';
import React, { useState } from 'react';
import { render } from '@testing-library/react';

import { KeyCode } from '@cloudscape-design/component-toolkit/internal';
import { createWrapper } from '@cloudscape-design/test-utils-core/dom';

import '../../__a11y__/to-validate-a11y';
Expand Down Expand Up @@ -32,12 +33,26 @@ const defaultProps: EmbeddedMultiselectProps = {
errorText: 'Error',
};

function StatefulEmbeddedMultiselect(props: EmbeddedMultiselectProps) {
const [selectedOptions, setSelectedOptions] = useState(props.selectedOptions);
return (
<EmbeddedMultiselect
{...props}
selectedOptions={selectedOptions}
onChange={event => {
props.onChange?.(event);
setSelectedOptions(event.detail.selectedOptions);
}}
/>
);
}

function renderComponent(props: Partial<EmbeddedMultiselectProps>) {
const { container } = render(
<div>
<label htmlFor="list-control">Input name</label>
<input id="list-control" />
<EmbeddedMultiselect {...defaultProps} {...props} />
<StatefulEmbeddedMultiselect {...defaultProps} {...props} />
</div>
);
return { container };
Expand Down Expand Up @@ -83,13 +98,39 @@ test('ARIA labels', () => {
expect(list).toHaveAccessibleDescription('Loading...');
});

test('highlights first option when list is focused', () => {
test('highlights first option when list is focused and removes highlight when the focus is lost', () => {
renderComponent({});

const list = createWrapper().find('ul')!.getElement();
list.focus();

const highlightedItemsAfterFocus = createWrapper().findAllByClassName(selectableItemsStyles.highlighted);
expect(highlightedItemsAfterFocus).toHaveLength(1);
expect(highlightedItemsAfterFocus[0].getElement()).toHaveTextContent('First');

list.blur();

const highlightedItemsAfterBlur = createWrapper().findAllByClassName(selectableItemsStyles.highlighted);
expect(highlightedItemsAfterBlur).toHaveLength(0);
});

test('selects options with Enter and Space and keeps highlight after Esc is pressed', () => {
renderComponent({});

createWrapper().find('ul')!.focus();
createWrapper().find('ul')!.keydown(KeyCode.enter);
createWrapper().find('ul')!.keydown(KeyCode.down);
createWrapper().find('ul')!.keydown(KeyCode.space);

const highlightedItems = createWrapper().findAllByClassName(selectableItemsStyles.highlighted);
const selectedItems = createWrapper().findAllByClassName(selectableItemsStyles.selected);
expect(highlightedItems).toHaveLength(1);
expect(highlightedItems[0].getElement()).toHaveTextContent('First');
expect(selectedItems).toHaveLength(2);

createWrapper().find('ul')!.keydown(KeyCode.escape);

const highlightedItemsAfterEscape = createWrapper().findAllByClassName(selectableItemsStyles.highlighted);
const selectedItemsAfterEscape = createWrapper().findAllByClassName(selectableItemsStyles.selected);
expect(highlightedItemsAfterEscape).toHaveLength(1);
expect(selectedItemsAfterEscape).toHaveLength(2);
});
21 changes: 14 additions & 7 deletions src/select/utils/use-select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function useSelect({
const filterRef = useRef<HTMLInputElement>(null);
const triggerRef = useRef<HTMLButtonElement>(null);
const menuRef = useRef<HTMLUListElement>(null);
const hasFilter = filteringType !== 'none';
const hasFilter = filteringType !== 'none' && !embedded;
const activeRef = hasFilter ? filterRef : menuRef;
const __selectedOptions = connectOptionsByValue(options, selectedOptions);
const __selectedValuesSet = selectedOptions.reduce((selectedValuesSet: Set<string>, item: OptionDefinition) => {
Expand Down Expand Up @@ -146,8 +146,10 @@ export function useSelect({
goHome: goHomeWithKeyboard,
goEnd: goEndWithKeyboard,
closeDropdown: () => {
triggerRef.current?.focus();
closeDropdown();
if (!embedded) {
triggerRef.current?.focus();
closeDropdown();
}
},
preventNativeSpace: !hasFilter,
});
Expand Down Expand Up @@ -228,15 +230,20 @@ export function useSelect({
},
statusType,
};
if (!hasFilter || embedded) {
if (!hasFilter) {
menuProps.onKeyDown = activeKeyDownHandler;
menuProps.nativeAttributes = {
'aria-activedescendant': highlightedOptionId,
};
}
if (embedded) {
menuProps.onFocus = () => {
goHomeWithKeyboard();
if (!highlightedOption) {
goHomeWithKeyboard();
}
};
menuProps.onBlur = () => {
resetHighlightWithKeyboard();
};
}
return menuProps;
Expand Down Expand Up @@ -298,13 +305,13 @@ export function useSelect({
]);

useEffect(() => {
if (isOpen) {
if (isOpen && !embedded) {
// dropdown-fit calculations ensure that the dropdown will fit inside the current
// viewport, so prevent the browser from trying to scroll it into view (e.g. if
// scroll-padding-top is set on a parent)
activeRef.current?.focus({ preventScroll: true });
}
}, [isOpen, activeRef]);
}, [isOpen, activeRef, embedded]);

useForwardFocus(externalRef, triggerRef as React.RefObject<HTMLElement>);
const highlightedGroupSelected =
Expand Down

0 comments on commit cc75463

Please sign in to comment.