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

[EuiComboBox] Fix enter key not working correctly on selection clear/deletion buttons #8105

Merged
merged 5 commits into from
Oct 30, 2024
Merged

[EuiComboBox] Fix enter key not working correctly on selection clear/deletion buttons #8105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1481718
Fix Enter key to work for deleting/clearing selected items
cee-chen Oct 29, 2024
1c7c30b
[tests] setup
cee-chen Oct 29, 2024
630d1a2
Add regression E2E test for enter key bug
cee-chen Oct 29, 2024
39c5d64
changelog
cee-chen Oct 29, 2024
e62f0db
Merge branch 'main' into combobox/enter-key-fix
cee-chen Oct 29, 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
3 changes: 3 additions & 0 deletions packages/eui/changelogs/upcoming/8105.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed an `EuiComboBox` bug where Enter keypresses were not working correctly on selection clear buttons
240 changes: 135 additions & 105 deletions packages/eui/src/components/combo_box/combo_box.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,31 @@ import {
} from './index';

describe('EuiComboBox', () => {
const defaultOptions: Array<EuiComboBoxOptionOption<{}>> = [
{ label: 'Item 1' },
{ label: 'Item 2' },
{ label: 'Item 3' },
];
const StatefulComboBox: FunctionComponent<Partial<EuiComboBoxProps<{}>>> = ({
options = defaultOptions,
selectedOptions: _selectedOptions = [],
...rest
}) => {
const [selectedOptions, setSelected] =
useState<typeof options>(_selectedOptions);
const onChange = (selectedOptions: typeof options) => {
setSelected(selectedOptions);
};
return (
<EuiComboBox
options={options}
selectedOptions={selectedOptions}
{...rest}
onChange={onChange}
/>
);
};

describe('focus management', () => {
it('keeps focus on the input box when clicking a disabled item', () => {
cy.realMount(
Expand All @@ -44,6 +69,116 @@ describe('EuiComboBox', () => {
});
});

describe('keyboard UX', () => {
it('allows the enter key to delete items and clear selections', () => {
cy.realMount(
<StatefulComboBox
data-test-subj="combobox"
selectedOptions={[{ label: 'Item 1' }, { label: 'Item 2' }]}
options={defaultOptions}
/>
);
cy.get('.euiComboBoxPill').should('have.length', 2);

cy.realPress('Tab');
cy.focused().should(
'have.attr',
'aria-label',
'Remove Item 1 from selection in this group'
);
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 1);

cy.realPress('Tab');
cy.focused().should('have.attr', 'data-test-subj', 'comboBoxClearButton');
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 0);
});

describe('backspace to delete last pill', () => {
it('does not delete the last pill if there is search text', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 2);

cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');

cy.get('[data-test-subj=comboBoxSearchInput]')
.invoke('val')
.should('equal', 'tes');
cy.get('.euiComboBoxPill').should('have.length', 2);
});

it('does not delete the last pill if the input is not active when backspace is pressed', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
cy.realPress('Escape');
cy.get('.euiComboBoxPill').should('have.length', 1);

cy.realPress(['Shift', 'Tab']); // Should be focused on the first pill's X button
cy.realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);

cy.repeatRealPress('Tab', 2); // Should be focused on the clear button
cy.realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);
});

it('deletes the last pill added when backspace on the input is pressed ', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 2);

cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);
});

it('`asPlainText`: deletes the selection and only a single character', () => {
cy.realMount(
// @ts-ignore - not totally sure why TS is kicking up a fuss here
<StatefulComboBox singleSelection={{ asPlainText: true }} />
);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('[data-test-subj=comboBoxSearchInput]').should(
'have.value',
'Item 1'
);
cy.get('[data-test-subj="comboBoxClearButton"]').should('exist'); // indicates selection

cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
cy.get('[data-test-subj="comboBoxClearButton"]').should('not.exist'); // selection removed
cy.get('[data-test-subj=comboBoxSearchInput]').should(
'have.value',
'Item '
);
});

it('opens up the selection list again after deleting the active single selection ', () => {
cy.realMount(<StatefulComboBox singleSelection />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');

cy.realPress('Backspace');
cy.get('[data-test-subj=comboBoxOptionsList]').should('have.length', 1);
});
});
});

describe('input auto sizing', () => {
it('resizes the width of the input based to fit the search text', () => {
cy.realMount(<EuiComboBox options={[]} />);
Expand Down Expand Up @@ -221,28 +356,6 @@ describe('EuiComboBox', () => {
});

describe('selection', () => {
const defaultOptions: Array<EuiComboBoxOptionOption<{}>> = [
{ label: 'Item 1' },
{ label: 'Item 2' },
{ label: 'Item 3' },
];
const StatefulComboBox: FunctionComponent<
Partial<EuiComboBoxProps<{}>>
> = ({ options = defaultOptions, ...rest }) => {
const [selectedOptions, setSelected] = useState<typeof options>([]);
const onChange = (selectedOptions: typeof options) => {
setSelected(selectedOptions);
};
return (
<EuiComboBox
options={options}
selectedOptions={selectedOptions}
{...rest}
onChange={onChange}
/>
);
};

describe('delimiter', () => {
it('selects the option when the delimiter option is typed into the search', () => {
cy.mount(<StatefulComboBox delimiter="," />);
Expand Down Expand Up @@ -343,89 +456,6 @@ describe('EuiComboBox', () => {
});
});
});

describe('backspace to delete last pill', () => {
it('does not delete the last pill if there is search text', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 2);

cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');

cy.get('[data-test-subj=comboBoxSearchInput]')
.invoke('val')
.should('equal', 'tes');
cy.get('.euiComboBoxPill').should('have.length', 2);
});

it('does not delete the last pill if the input is not active when backspace is pressed', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('[data-test-subj=comboBoxSearchInput]').type('test');
cy.realPress('Escape');
cy.get('.euiComboBoxPill').should('have.length', 1);

cy.realPress(['Shift', 'Tab']); // Should be focused on the first pill's X button
cy.realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);

cy.repeatRealPress('Tab', 2); // Should be focused on the clear button
cy.realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);
});

it('deletes the last pill added when backspace on the input is pressed ', () => {
cy.realMount(<StatefulComboBox />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('.euiComboBoxPill').should('have.length', 2);

cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
cy.get('.euiComboBoxPill').should('have.length', 1);
});

it('`asPlainText`: deletes the selection and only a single character', () => {
cy.realMount(
// @ts-ignore - not totally sure why TS is kicking up a fuss here
<StatefulComboBox singleSelection={{ asPlainText: true }} />
);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');
cy.get('[data-test-subj=comboBoxSearchInput]').should(
'have.value',
'Item 1'
);
cy.get('[data-test-subj="comboBoxClearButton"]').should('exist'); // indicates selection

cy.get('[data-test-subj=comboBoxSearchInput]').realPress('Backspace');
cy.get('[data-test-subj="comboBoxClearButton"]').should('not.exist'); // selection removed
cy.get('[data-test-subj=comboBoxSearchInput]').should(
'have.value',
'Item '
);
});

it('opens up the selection list again after deleting the active single selection ', () => {
cy.realMount(<StatefulComboBox singleSelection />);
cy.get('[data-test-subj=comboBoxSearchInput]').realClick();
cy.realPress('{downarrow}');
cy.realPress('Enter');

cy.realPress('Backspace');
cy.get('[data-test-subj=comboBoxOptionsList]').should('have.length', 1);
});
});
});

describe('z-index regression testing', () => {
Expand Down
19 changes: 11 additions & 8 deletions packages/eui/src/components/combo_box/combo_box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,14 +536,17 @@ export class EuiComboBox<T> extends Component<
break;

case keys.ENTER:
event.preventDefault();
event.stopPropagation();
if (this.hasActiveOption()) {
this.onAddOption(
this.state.matchingOptions[this.state.activeOptionIndex]
);
} else {
this.setCustomOptions(false);
// Do not block enter keypresses for the clear button or delete selection buttons
if (event.target === this.searchInputRefInstance) {
event.preventDefault();
event.stopPropagation();
if (this.hasActiveOption()) {
this.onAddOption(
this.state.matchingOptions[this.state.activeOptionIndex]
);
} else {
this.setCustomOptions(false);
}
}
break;

Expand Down
Loading