Skip to content

Commit

Permalink
[EuiComboBox] Only delete the last selected pill when pressing the ba…
Browse files Browse the repository at this point in the history
…ckspace key when the input caret is present (#6699)

* Remove `backspace` keydown on entire combobox

- it should be firing only on the input, and not (e.g.) on the clear button or on individual pills

* Write Cypress E2E tests for backspace behavior

+ fix `data-test-subj` that was causing an extra space and a failed selector get

* changelog
  • Loading branch information
cee-chen authored Apr 7, 2023
1 parent 35b6a27 commit a57fbda
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 31 deletions.
84 changes: 83 additions & 1 deletion src/components/combo_box/combo_box.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

/// <reference types="../../../cypress/support"/>

import React from 'react';
import React, { useState } from 'react';
import { EuiComboBox } from './index';

describe('EuiComboBox', () => {
Expand All @@ -34,4 +34,86 @@ describe('EuiComboBox', () => {
cy.focused().should('have.attr', 'data-test-subj', 'comboBoxSearchInput');
});
});

describe('Backspace to delete last pill', () => {
const options = [
{ label: 'Item 1' },
{ label: 'Item 2' },
{ label: 'Item 3' },
];

const TestComboBox = (...rest) => {
const [selectedOptions, setSelected] = useState([]);
const onChange = (selectedOptions) => {
setSelected(selectedOptions);
};
return (
<EuiComboBox
options={options}
selectedOptions={selectedOptions}
onChange={onChange}
{...rest}
/>
);
};

it('does not delete the last pill if there is search text', () => {
cy.realMount(<TestComboBox />);
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(<TestComboBox />);
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(<TestComboBox />);
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('opens up the selection list again after deleting the active single selection ', () => {
cy.realMount(<TestComboBox 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);
});
});
});
25 changes: 0 additions & 25 deletions src/components/combo_box/combo_box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -418,26 +418,6 @@ export class EuiComboBox<T> extends Component<
this.onSearchChange('');
};

removeLastOption = () => {
if (!this.props.selectedOptions.length) {
return;
}

// Backspace will be used to delete the input, not a pill.
if (this.state.searchValue.length) {
return;
}

// Delete last pill.
this.onRemoveOption(
this.props.selectedOptions[this.props.selectedOptions.length - 1]
);

if (Boolean(this.props.singleSelection) && !this.state.isListOpen) {
this.openList();
}
};

addCustomOption = (isContainerBlur: boolean, searchValue: string) => {
const {
isCaseSensitive,
Expand Down Expand Up @@ -625,11 +605,6 @@ export class EuiComboBox<T> extends Component<
}
break;

case keys.BACKSPACE:
event.stopPropagation();
this.removeLastOption();
break;

case keys.ESCAPE:
if (this.state.isListOpen) {
event.preventDefault();
Expand Down
33 changes: 29 additions & 4 deletions src/components/combo_box/combo_box_input/combo_box_input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@
*/

import React, {
ChangeEventHandler,
Component,
ChangeEventHandler,
FocusEventHandler,
KeyboardEventHandler,
RefCallback,
} from 'react';
import classNames from 'classnames';
import AutosizeInput from 'react-input-autosize';

import { CommonProps } from '../../common';
import { htmlIdGenerator } from '../../../services';
import { htmlIdGenerator, keys } from '../../../services';
import { EuiScreenReaderOnly } from '../../accessibility';
import {
EuiFormControlLayout,
Expand Down Expand Up @@ -51,11 +52,11 @@ export interface EuiComboBoxInputProps<T> extends CommonProps {
onCloseListClick: () => void;
onFocus: FocusEventHandler<HTMLInputElement>;
onOpenListClick: () => void;
onRemoveOption?: OptionHandler<T>;
onRemoveOption: OptionHandler<T>;
placeholder?: string;
rootId: ReturnType<typeof htmlIdGenerator>;
searchValue: string;
selectedOptions?: Array<EuiComboBoxOptionOption<T>>;
selectedOptions: Array<EuiComboBoxOptionOption<T>>;
singleSelection?: boolean | EuiComboBoxSingleSelectionShape;
toggleButtonRef?: RefCallback<HTMLButtonElement | HTMLSpanElement>;
updatePosition: UpdatePositionHandler;
Expand Down Expand Up @@ -104,6 +105,29 @@ export class EuiComboBoxInput<T> extends Component<
});
};

onKeyDown: KeyboardEventHandler<HTMLInputElement> = (event) => {
const {
searchValue,
selectedOptions,
onRemoveOption,
singleSelection,
isListOpen,
onOpenListClick,
} = this.props;

// When backspacing from an empty input, delete the last pill option in the list
const searchIsEmpty = !searchValue.length;
const hasPills = selectedOptions.length;

if (event.key === keys.BACKSPACE && searchIsEmpty && hasPills) {
onRemoveOption(selectedOptions[selectedOptions.length - 1]);

if (!!singleSelection && !isListOpen) {
onOpenListClick();
}
}
};

componentDidUpdate(prevProps: EuiComboBoxInputProps<T>) {
const { searchValue } = prevProps;

Expand Down Expand Up @@ -310,6 +334,7 @@ export class EuiComboBoxInput<T> extends Component<
onBlur={this.onBlur}
onChange={this.inputOnChange}
onFocus={this.onFocus}
onKeyDown={this.onKeyDown}
ref={this.inputRefCallback}
role="combobox"
style={{ fontSize: 14 }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ export class EuiComboBoxOptionsList<T> extends Component<
hasShadow={false}
className={classes}
panelRef={this.listRefCallback}
data-test-subj={`comboBoxOptionsList ${dataTestSubj}`}
data-test-subj={classNames('comboBoxOptionsList', dataTestSubj)}
style={{ ...style, zIndex: zIndex }}
isOpen
isAttached
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/6699.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed `EuiComboBox` to only delete the last selected item on backspace if the input caret is present

0 comments on commit a57fbda

Please sign in to comment.