Skip to content

Commit

Permalink
fix(Combobox): add event propagation to mouse down (#9402)
Browse files Browse the repository at this point in the history
* fix(Combobox): add event propagation to mouse down

* fix(combobox): ignore downshift behavior for chevron click when listbox is open

* docs(combobox): add multiple instances story

* test(combobox): multiple instances should only render one listbox at a time

* chore: remove unecessary id value

* test(combobox): add query abstraction for readability

* fix(Combobox): remove test story

Co-authored-by: Taylor Jones <taylor.jones826@gmail.com>
Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
Co-authored-by: TJ Egan <tw15egan@gmail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
5 people authored Sep 2, 2021
1 parent 4907951 commit 2dd2b31
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
46 changes: 46 additions & 0 deletions packages/react/src/components/ComboBox/ComboBox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import React from 'react';
import { mount } from 'enzyme';
import { render, screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {
findListBoxNode,
findMenuNode,
Expand Down Expand Up @@ -201,5 +203,49 @@ describe('ComboBox', () => {
const wrapper = mount(<ComboBox {...mockProps} />);
expect(wrapper.find('input').instance().value).toBe('');
});

it('should only render one listbox at a time when multiple comboboxes are present', () => {
render(
<>
<div data-testid="combobox-1">
<ComboBox {...mockProps} id="combobox-1" />
</div>
<div data-testid="combobox-2">
<ComboBox {...mockProps} id="combobox-2" />
</div>
</>
);
const firstCombobox = screen.getByTestId('combobox-1');
const secondCombobox = screen.getByTestId('combobox-2');

const firstComboboxChevron = within(firstCombobox).getByRole('button');
const secondComboboxChevron = within(secondCombobox).getByRole('button');

function firstListBox() {
return within(firstCombobox).getByRole('listbox');
}

function secondListBox() {
return within(secondCombobox).getByRole('listbox');
}

expect(firstListBox()).toBeEmptyDOMElement();
expect(secondListBox()).toBeEmptyDOMElement();

userEvent.click(firstComboboxChevron);

expect(firstListBox()).not.toBeEmptyDOMElement();
expect(secondListBox()).toBeEmptyDOMElement();

userEvent.click(secondComboboxChevron);

expect(firstListBox()).toBeEmptyDOMElement();
expect(secondListBox()).not.toBeEmptyDOMElement();

userEvent.click(secondComboboxChevron);

expect(firstListBox()).toBeEmptyDOMElement();
expect(secondListBox()).toBeEmptyDOMElement();
});
});
});
8 changes: 5 additions & 3 deletions packages/react/src/components/ComboBox/ComboBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,13 @@ const ComboBox = React.forwardRef((props, ref) => {
// https://github.com/downshift-js/downshift/blob/v5.2.1/src/downshift.js#L1051-L1065
//
// As a result, it will reset the state of the component and so we
// stop the event from propagating to prevent this. This allows the
// toggleMenu behavior for the toggleButton to correctly open and
// stop the event from propagating to prevent this if the menu is already open.
// This allows the toggleMenu behavior for the toggleButton to correctly open and
// close the menu.
onMouseUp(event) {
event.stopPropagation();
if (isOpen) {
event.stopPropagation();
}
},
});
const inputProps = getInputProps({
Expand Down

0 comments on commit 2dd2b31

Please sign in to comment.