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.

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

fix(ListBox): update focus states for listbox components #15980

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ test.describe('@avt FluidMultiSelect', () => {
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
// Navigation inside the menu
// move to first option
await page.keyboard.press('ArrowDown');
// Focus on first element by default
await expect(
page.getByRole('option', {
name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.',
Expand Down Expand Up @@ -177,8 +176,7 @@ test.describe('@avt FluidMultiSelect', () => {
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
// Navigation inside the menu
// move to first option
await page.keyboard.press('ArrowDown');
// Focus on first element by default
await expect(
page.getByRole('option', {
name: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.',
Expand Down
6 changes: 2 additions & 4 deletions e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ test.describe('@avt MultiSelect', () => {
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
// Navigation inside the menu
// move to first option
await page.keyboard.press('ArrowDown');
// Focus on first element by default
await expect(
page.getByRole('option', {
name: 'An example option that is really long to show what should be done to handle long text',
Expand Down Expand Up @@ -206,8 +205,7 @@ test.describe('@avt MultiSelect', () => {
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
// Navigation inside the menu
// move to first option
await page.keyboard.press('ArrowDown');
// Focus on first element by default
await expect(
page.getByRole('option', {
name: 'An example option that is really long to show what should be done to handle long text',
Expand Down
16 changes: 12 additions & 4 deletions packages/react/src/components/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ const {
keyDownArrowUp,
keyDownEscape,
clickButton,
clickItem,
blurButton,
changeInput,
blurInput,
unknown,
} = Downshift.stateChangeTypes;

const defaultItemToString = <ItemType,>(item: ItemType | null) => {
Expand Down Expand Up @@ -450,15 +452,16 @@ const ComboBox = forwardRef(
switch (type) {
case keyDownArrowDown:
case keyDownArrowUp:
setHighlightedIndex(changes.highlightedIndex);
if (changes.isOpen) {
updateHighlightedIndex(getHighlightedIndex(changes));
} else {
setHighlightedIndex(changes.highlightedIndex);
}
break;
case blurButton:
case keyDownEscape:
setHighlightedIndex(changes.highlightedIndex);
break;
case clickButton:
setHighlightedIndex(changes.highlightedIndex);
break;
case changeInput:
updateHighlightedIndex(getHighlightedIndex(changes));
break;
Expand All @@ -470,6 +473,11 @@ const ComboBox = forwardRef(
}
}
break;
case clickButton:
case clickItem:
case unknown:
setHighlightedIndex(getHighlightedIndex(changes));
break;
}
};

Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const {
ToggleButtonKeyDownHome,
ToggleButtonKeyDownEnd,
ItemMouseMove,
MenuMouseLeave,
} = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & {
ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick;
};
Expand Down Expand Up @@ -303,6 +304,7 @@ const Dropdown = React.forwardRef(
}
return changes;
case ItemMouseMove:
case MenuMouseLeave:
return { ...changes, highlightedIndex: state.highlightedIndex };
}
return changes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
if (onMenuChange) {
onMenuChange(nextIsOpen);
}

if (!isOpen) {
setHighlightedIndex(0);
}
}

function handleOnOuterClick() {
Expand All @@ -173,6 +177,34 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
break;
case stateChangeTypes.keyDownEscape:
handleOnMenuChange(false);
setHighlightedIndex(0);
break;
case stateChangeTypes.changeInput:
setHighlightedIndex(0);
break;
case stateChangeTypes.keyDownEnter:
if (!isOpen) {
setHighlightedIndex(0);
}
break;
case stateChangeTypes.clickItem:
if (isOpen) {
const sortedItems = sortItems(
filterItems(items, { itemToString, inputValue }),
{
selectedItems: {
top: changes.selectedItems,
fixed: [],
'top-after-reopen': topItems,
}[selectionFeedback],
itemToString,
compareItems,
locale,
}
);
const sortedSelectedIndex = sortedItems.indexOf(changes.selectedItem);
setHighlightedIndex(sortedSelectedIndex);
}
break;
}
}
Expand Down
33 changes: 27 additions & 6 deletions packages/react/src/components/MultiSelect/MultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ const {
ToggleButtonKeyDownEscape,
ToggleButtonKeyDownSpaceButton,
ItemMouseMove,
MenuMouseLeave,
ToggleButtonClick,
ToggleButtonKeyDownHome,
ToggleButtonKeyDownEnd,
ToggleButtonKeyDownPageDown,
ToggleButtonKeyDownPageUp,
FunctionSetHighlightedIndex,
} = useSelect.stateChangeTypes as UseSelectInterface['stateChangeTypes'] & {
ToggleButtonClick: UseSelectStateChangeTypes.ToggleButtonClick;
};
Expand Down Expand Up @@ -400,6 +402,7 @@ const MultiSelect = React.forwardRef(
getItemProps,
selectedItem,
highlightedIndex,
setHighlightedIndex,
} = useSelect<ItemType>(selectProps);

const toggleButtonProps = getToggleButtonProps({
Expand All @@ -426,6 +429,7 @@ const MultiSelect = React.forwardRef(
match(e, keys.Enter)) &&
!isOpen
) {
setHighlightedIndex(0);
setItemsCleared(false);
setIsOpenWrapper(true);
}
Expand Down Expand Up @@ -522,7 +526,6 @@ const MultiSelect = React.forwardRef(
}

switch (type) {
case ItemClick:
case ToggleButtonKeyDownSpaceButton:
case ToggleButtonKeyDownEnter:
if (changes.selectedItem === undefined) {
Expand All @@ -539,11 +542,29 @@ const MultiSelect = React.forwardRef(
break;
case ToggleButtonClick:
setIsOpenWrapper(changes.isOpen || false);
break;
return { ...changes, highlightedIndex: 0 };
case ItemClick:
setHighlightedIndex(changes.selectedItem);
onItemChange(changes.selectedItem);
return { ...changes, highlightedIndex: state.highlightedIndex };
case MenuMouseLeave:
return { ...changes, highlightedIndex: state.highlightedIndex };
case FunctionSetHighlightedIndex:
if (!isOpen) {
return {
...changes,
highlightedIndex: 0,
};
} else {
return {
...changes,
highlightedIndex: props.items.indexOf(highlightedIndex),
};
}
case ToggleButtonKeyDownArrowDown:
case ToggleButtonKeyDownArrowUp:
case ToggleButtonKeyDownHome:
case ToggleButtonKeyDownEnd:
case ToggleButtonKeyDownPageDown:
case ToggleButtonKeyDownPageUp:
if (highlightedIndex > -1) {
const itemArray = document.querySelectorAll(
`li.${prefix}--list-box__menu-item[role="option"]`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ describe('MultiSelect', () => {

expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'false');

await userEvent.keyboard('[ArrowDown]');
await userEvent.keyboard('[Enter]');

expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'true');
Expand Down
7 changes: 7 additions & 0 deletions packages/styles/scss/components/combo-box/_combo-box.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
@use '../list-box';
@use '../../config' as *;
@use '../../theme' as *;
@use '../../utilities/convert';
@use '../../utilities/focus-outline' as *;

/// Combo box styles
Expand Down Expand Up @@ -36,6 +37,12 @@
@include focus-outline('outline');
}

.#{$prefix}--list-box--expanded
.#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
outline-offset: convert.to-rem(-1px);
outline-width: convert.to-rem(1px);
}

.#{$prefix}--combo-box .#{$prefix}--list-box__field,
.#{$prefix}--combo-box.#{$prefix}--list-box[data-invalid]
.#{$prefix}--list-box__field,
Expand Down
4 changes: 4 additions & 0 deletions packages/styles/scss/components/dropdown/_dropdown.scss
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@
border-block-end-color: $border-subtle;
}

.#{$prefix}--dropdown--open .cds--list-box__field {
outline: none;
}

.#{$prefix}--dropdown--invalid {
@include focus-outline('invalid');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,11 @@
+ .#{$prefix}--list-box__invalid-icon {
inset-inline-end: 1rem;
}

.#{$prefix}--list-box__wrapper--fluid
:not(.#{$prefix}--list-box--up)
.#{$prefix}--combo-box
.#{$prefix}--list-box__menu {
inset-block-start: calc(100% + convert.to-rem(1px));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,20 @@
outline-offset: 0;
}

.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
.#{$prefix}--list-box--expanded
) {
outline-width: convert.to-rem(1px);
}

.#{$prefix}--list-box__wrapper--fluid .#{$prefix}--list-box__field:focus {
outline: none;
outline-offset: 0;
}

.#{$prefix}--list-box__wrapper--fluid
:not(.#{$prefix}--list-box--up)
.#{$prefix}--list-box__wrapper--fluid:not(.#{$prefix}--list-box--up)
.#{$prefix}--list-box__menu {
inset-block-start: calc(100% + convert.to-rem(1px));
inset-block-start: calc(100%);
}

// Invalid / Warning styles
Expand Down Expand Up @@ -235,8 +240,7 @@
}

// direction - top
.#{$prefix}--list-box__wrapper--fluid
.#{$prefix}--list-box--up
.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box--up
.#{$prefix}--list-box__menu {
inset-block-end: $spacing-10;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
@use '../../motion' as *;
@use '../../spacing' as *;
@use '../../theme' as *;
@use '../../utilities/convert';
@use '../../utilities/focus-outline' as *;
@use '../multiselect';
@use '../fluid-list-box';
Expand Down Expand Up @@ -49,4 +50,11 @@
.#{$prefix}--list-box__field {
align-items: baseline;
}

.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--multi-select--filterable__wrapper:not(
.#{$prefix}--list-box--up
)
.#{$prefix}--list-box__menu {
inset-block-start: calc(100% + convert.to-rem(1px));
}
}
11 changes: 11 additions & 0 deletions packages/styles/scss/components/multiselect/_multiselect.scss
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@
@include focus-outline('outline');
}

.#{$prefix}--multi-select.#{$prefix}--list-box--expanded
.#{$prefix}--list-box__field--wrapper:has(
button[aria-activedescendant]:not([aria-activedescendant=''])
),
.#{$prefix}--multi-select--filterable.#{$prefix}--list-box--expanded:has(
input[aria-activedescendant]:not([aria-activedescendant=''])
) {
outline-offset: convert.to-rem(-1px);
outline-width: convert.to-rem(1px);
}

.#{$prefix}--multi-select--filterable.#{$prefix}--multi-select--selected
.#{$prefix}--text-input,
.#{$prefix}--multi-select.#{$prefix}--multi-select--selected
Expand Down
Loading