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(Combobox): add event propagation to mouse down #9402

Merged
Merged
27 changes: 27 additions & 0 deletions packages/react/src/components/ComboBox/ComboBox-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,30 @@ export const light = () => (
/>
</div>
);

export const multipleInstances = () => (
<>
<div style={{ width: 300 }}>
<ComboBox
onChange={() => {}}
id="carbon-combobox-1"
items={items}
itemToString={(item) => (item ? item.text : '')}
placeholder="Filter..."
titleText="ComboBox title"
helperText="Combobox helper text"
/>
</div>
<div style={{ width: 300, position: 'absolute', right: 0 }}>
<ComboBox
onChange={() => {}}
id="carbon-combobox-2"
items={items}
itemToString={(item) => (item ? item.text : '')}
placeholder="Filter..."
titleText="ComboBox title"
helperText="Combobox helper text"
/>
</div>
</>
);
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', () => {
sstrubberg marked this conversation as resolved.
Show resolved Hide resolved
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