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

Removed highlight from Dropdown components #16789

Merged
merged 18 commits into from
Aug 6, 2024
Merged

Removed highlight from Dropdown components #16789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
06ad542
fix: removed highlight from first item
guidari Jun 14, 2024
58de69d
test: fixed tests to match new behaviour
guidari Jun 17, 2024
03fc441
test: fixed fluid components tests
guidari Jun 17, 2024
c3ff22f
fix: fixed click on arrow
guidari Jun 19, 2024
9d39ff4
Merge branch 'main' into 16512-dropdown
guidari Jul 1, 2024
04d34ed
fix: fixed focus state
guidari Jul 1, 2024
ddd8a2c
Merge branch '16512-dropdown' of github.com:guidari/carbon into 16512…
guidari Jul 1, 2024
3f4e50d
fix: fixed filterable multiselect focus
guidari Jul 12, 2024
44f49e5
fix: fixed focus state for combobox
guidari Jul 15, 2024
70e1289
fix: fixed scss
guidari Jul 16, 2024
72931af
fix: fixed multiselect focus state
guidari Jul 22, 2024
e0c9822
fix: removed console.log
guidari Jul 22, 2024
9ee8be1
fix: fixex arrowDown to open the menu
guidari Jul 24, 2024
b575919
fix: fixed tests
guidari Jul 24, 2024
3c035c4
fix: fixed issues
guidari Jul 24, 2024
264c8b0
Merge branch 'main' into 16512-dropdown
guidari Jul 24, 2024
d9c1de6
Merge branch 'main' into 16512-dropdown
guidari Aug 1, 2024
22e5a44
Merge branch 'main' into 16512-dropdown
Kritvi-bhatia17 Aug 2, 2024
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
7 changes: 5 additions & 2 deletions e2e/components/ComboBox/ComboBox-test.avt.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ test.describe('@avt ComboBox', () => {
await expect(combobox).toBeFocused();
await page.keyboard.press('Enter');
await expect(menu).toBeVisible();
await page.keyboard.press('ArrowDown');
// Navigation inside the menu
// move to first option
await expect(optionOne).toHaveClass(
Expand Down Expand Up @@ -122,15 +123,17 @@ test.describe('@avt ComboBox', () => {
await page.keyboard.press('Enter');
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await expect(combobox).toHaveValue('Option 1');
await expect(combobox).toHaveValue(
'An example option that is really long to show what should be done to handle long text'
);
await page.keyboard.press('Escape');

// should open and select option 2
await page.keyboard.press('Enter');
await page.keyboard.press('ArrowDown');
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await expect(combobox).toHaveValue('Option 2');
await expect(combobox).toHaveValue('Option 1');
await page.keyboard.press('Escape');
});
});
1 change: 1 addition & 0 deletions e2e/components/FluidComboBox/FluidComboBox-test.avt.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ test.describe('@avt FluidComboBox', () => {
await expect(combobox).toBeFocused();
await page.keyboard.press('Enter');
await expect(menu).toBeVisible();
await page.keyboard.press('ArrowDown');
// Navigation inside the menu
// move to first option
await expect(optionOne).toHaveClass(
Expand Down
2 changes: 2 additions & 0 deletions e2e/components/FluidMultiSelect/FluidMultiSelect-test.avt.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ test.describe('@avt FluidMultiSelect', () => {
await expect(toggleButton).toBeFocused();
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
await page.keyboard.press('ArrowDown');
// Navigation inside the menu
// Focus on first element by default
await expect(
Expand Down Expand Up @@ -175,6 +176,7 @@ test.describe('@avt FluidMultiSelect', () => {
await expect(toggleButton).toBeFocused();
await page.keyboard.press('Space');
await expect(menu).toBeVisible();
await page.keyboard.press('ArrowDown');
// Navigation inside the menu
// Focus on first element by default
await expect(
Expand Down
3 changes: 2 additions & 1 deletion e2e/components/MultiSelect/MultiSelect-test.avt.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,11 @@ test.describe('@avt MultiSelect', () => {
page.getByRole('option', {
name: 'An example option that is really long to show what should be done to handle long text',
})
).toHaveClass(
).not.toHaveClass(
'cds--list-box__menu-item cds--list-box__menu-item--highlighted'
);
// select first option (should select with enter and space)
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await expect(
page.getByRole('option', {
Expand Down
10 changes: 5 additions & 5 deletions e2e/components/Toggle/Toggle-test.avt.e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ test.describe('@avt Toggle', () => {
theme: 'white',
},
});
const toggleSwitch = page.getByRole('switch');
await page.keyboard.press('Tab');
await expect(page.getByRole('switch')).toBeVisible();
await page.keyboard.press('Space');
page.getByText('Off');
await page.keyboard.press('Space');
page.getByText('On');
await expect(toggleSwitch).toBeVisible();
await expect(toggleSwitch).toHaveAttribute('aria-checked', 'true');
await page.keyboard.press('Enter');
await expect(toggleSwitch).toHaveAttribute('aria-checked', 'false');
});
});
4 changes: 2 additions & 2 deletions packages/react/src/components/ComboBox/ComboBox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ const ComboBox = forwardRef(
case FunctionToggleMenu:
case ToggleButtonClick:
if (changes.isOpen && !changes.selectedItem) {
return { ...changes, highlightedIndex: 0 };
return { ...changes };
}
return changes;

Expand Down Expand Up @@ -575,7 +575,7 @@ const ComboBox = forwardRef(

const inputClasses = cx(`${prefix}--text-input`, {
[`${prefix}--text-input--empty`]: !inputValue,
[`${prefix}--combo-box--input--focus`]: isFocused && !isFluid,
[`${prefix}--combo-box--input--focus`]: isFocused,
});

// needs to be Capitalized for react to render it correctly
Expand Down
28 changes: 22 additions & 6 deletions packages/react/src/components/MultiSelect/FilterableMultiSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,16 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
}
}, [controlledSelectedItems, isOpen, setTopItems]);

const validateHighlightFocus = () => {
if (controlledSelectedItems.length > 0) {
setHighlightedIndex(0);
}
};

function handleMenuChange(forceIsOpen: boolean): void {
const nextIsOpen = forceIsOpen ?? !isOpen;
setIsOpen(nextIsOpen);
validateHighlightFocus();
if (onMenuChange) {
onMenuChange(nextIsOpen);
}
Expand All @@ -508,8 +515,8 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
} = useCombobox<ItemType>({
isOpen,
items: sortedItems,
// defaultHighlightedIndex: 0, // after selection, highlight the first item.
itemToString,
defaultHighlightedIndex: 0, // after selection, highlight the first item.
id,
labelId,
menuId,
Expand All @@ -520,7 +527,6 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
return (item as any).disabled;
},
});

function stateReducer(state, actionAndChanges) {
const { type, props, changes } = actionAndChanges;
const { highlightedIndex } = changes;
Expand Down Expand Up @@ -548,20 +554,30 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
return changes;
case FunctionToggleMenu:
case ToggleButtonClick:
validateHighlightFocus();
if (changes.isOpen && !changes.selectedItem) {
return { ...changes, highlightedIndex: 0 };
return { ...changes };
}
return changes;

return { ...changes, highlightedIndex: null };
case InputChange:
if (onInputValueChange) {
onInputValueChange(changes.inputValue);
}
setInputValue(changes.inputValue ?? '');
setIsOpen(true);
return changes;
return { ...changes, highlightedIndex: 0 };

case InputClick:
return { ...changes, isOpen: false };
validateHighlightFocus();
if (changes.isOpen && !changes.selectedItem) {
return { ...changes };
}
return {
...changes,
isOpen: false,
highlightedIndex: null,
};
case MenuMouseLeave:
return { ...changes, highlightedIndex: state.highlightedIndex };
case InputKeyDownArrowUp:
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/components/MultiSelect/MultiSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,11 @@ const MultiSelect = React.forwardRef(
break;
case ToggleButtonClick:
setIsOpenWrapper(changes.isOpen || false);
return { ...changes, highlightedIndex: 0 };
return {
...changes,
highlightedIndex:
controlledSelectedItems.length > 0 ? 0 : undefined,
};
case ItemClick:
setHighlightedIndex(changes.selectedItem);
onItemChange(changes.selectedItem);
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ describe('MultiSelect', () => {

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

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

expect(itemNode).toHaveAttribute('data-contained-checkbox-state', 'true');
Expand Down
4 changes: 3 additions & 1 deletion packages/styles/scss/components/combo-box/_combo-box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
@include focus-outline('outline');
}

.#{$prefix}--list-box--expanded
.#{$prefix}--combo-box.#{$prefix}--list-box--expanded:has(
input[aria-activedescendant]:not([aria-activedescendant=''])
)
.#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
outline-offset: convert.to-rem(-1px);
outline-width: convert.to-rem(1px);
Expand Down
11 changes: 7 additions & 4 deletions packages/styles/scss/components/fluid-combo-box/_fluid-combo-box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,13 @@
white-space: nowrap;
}

.#{$prefix}--list-box__wrapper--fluid
.#{$prefix}--combo-box
.#{$prefix}--text-input:focus {
outline: none;
.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus
.#{$prefix}--combo-box.#{$prefix}--list-box--expanded:has(
input[aria-activedescendant]:not([aria-activedescendant=''])
)
.#{$prefix}--combo-box--input--focus.#{$prefix}--text-input {
outline-offset: convert.to-rem(-1px);
outline-width: convert.to-rem(1px);
}

.#{$prefix}--list-box__wrapper--fluid
Expand Down
6 changes: 6 additions & 0 deletions packages/styles/scss/components/fluid-list-box/_fluid-list-box.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@
outline-offset: 0;
}

.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
.#{$prefix}--combo-box
) {
outline: none;
}

.#{$prefix}--list-box__wrapper--fluid.#{$prefix}--list-box__wrapper--fluid--focus:has(
.#{$prefix}--list-box--expanded
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/styles/scss/components/multiselect/_multiselect.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
.#{$prefix}--list-box__field--wrapper--input-focused:has(
button[aria-expanded='false']
),
.#{$prefix}--multi-select.#{$prefix}--multi-select--selected
.#{$prefix}--multi-select
.#{$prefix}--list-box__field--wrapper--input-focused:has(
button[aria-expanded='true']
) {
Expand Down
Loading