Skip to content

Commit

Permalink
[select]swapped onClick with onMouseDown
Browse files Browse the repository at this point in the history
Add tests for the new behavior

changed Select.test.js
  • Loading branch information
SarthakC committed Oct 29, 2019
1 parent e693f95 commit 452bc99
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 10 deletions.
36 changes: 29 additions & 7 deletions packages/material-ui/src/Select/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('<Select />', () => {
const { getByRole } = render(<Select value="" autoFocus />);

act(() => {
getByRole('button').click();
fireEvent.mouseDown(getByRole('button'));
});

// TODO not matching WAI-ARIA authoring practices. It should focus the first (or selected) item.
Expand All @@ -224,7 +224,7 @@ describe('<Select />', () => {
<MenuItem value="2" />
</Select>,
);
getByRole('button').click();
fireEvent.mouseDown(getByRole('button'));
getAllByRole('option')[1].click();

expect(onChangeHandler.calledOnce).to.be.true;
Expand Down Expand Up @@ -514,7 +514,7 @@ describe('<Select />', () => {
</Select>,
);

fireEvent.click(getByRole('button'));
fireEvent.mouseDown(getByRole('button'));
act(() => {
clock.tick(99);
});
Expand Down Expand Up @@ -613,7 +613,7 @@ describe('<Select />', () => {
const { getByRole, queryByRole } = render(<ControlledWrapper />);

act(() => {
getByRole('button').click();
fireEvent.mouseDown(getByRole('button'));
});

expect(getByRole('listbox')).to.be.ok;
Expand Down Expand Up @@ -656,7 +656,7 @@ describe('<Select />', () => {
stub(button, 'clientWidth').get(() => 14);

act(() => {
button.click();
fireEvent.mouseDown(button);
});

expect(getByTestId('paper').style).to.have.property('minWidth', '14px');
Expand All @@ -672,7 +672,7 @@ describe('<Select />', () => {
stub(button, 'clientWidth').get(() => 14);

act(() => {
button.click();
fireEvent.mouseDown(button);
});

expect(getByTestId('paper').style).to.have.property('minWidth', '');
Expand Down Expand Up @@ -795,7 +795,7 @@ describe('<Select />', () => {
});
const { getByRole, getAllByRole } = render(<ControlledSelectInput onChange={onChange} />);

fireEvent.click(getByRole('button'));
fireEvent.mouseDown(getByRole('button'));
const options = getAllByRole('option');
fireEvent.click(options[2]);

Expand Down Expand Up @@ -867,4 +867,26 @@ describe('<Select />', () => {
expect(getByRole('button')).to.have.attribute('id', 'mui-component-select-foo');
});
});

describe('prop: onMouseDown', () => {
it('is forwarded to the trigger button', () => {
const handleMouseDown = spy();
const { getByRole } = render(<Select onMouseDown={handleMouseDown} value="" />);

fireEvent.mouseDown(getByRole('button'));

expect(handleMouseDown.callCount).to.equal(1);
});
});

describe('prop: onClick', () => {
it('is forwarded to the trigger button', () => {
const handleClick = spy();
const { getByRole } = render(<Select onClick={handleClick} value="" />);

fireEvent.click(getByRole('button'));

expect(handleClick.callCount).to.equal(1);
});
});
});
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/SelectInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
aria-labelledby={`${labelId || ''} ${buttonId || ''}`}
aria-haspopup="listbox"
onKeyDown={handleKeyDown}
onClick={disabled || readOnly ? null : handleClick}
onMouseDown={disabled || readOnly ? null : handleClick}
onBlur={handleBlur}
onFocus={onFocus}
{...SelectDisplayProps}
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/test/integration/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('<Select> integration', () => {
// Let's open the select component
// in the browser user click also focuses
trigger.focus();
trigger.click();
fireEvent.mouseDown(trigger);

const options = getAllByRole('option');
expect(options[1]).to.be.focused;
Expand All @@ -82,7 +82,7 @@ describe('<Select> integration', () => {
// Let's open the select component
// in the browser user click also focuses
trigger.focus();
trigger.click();
fireEvent.mouseDown(trigger);

const options = getAllByRole('option');
expect(options[1]).to.be.focused;
Expand Down

0 comments on commit 452bc99

Please sign in to comment.