Skip to content

Commit 6c9df3c

Browse files
Kartik-MurthyZeeshanTambolisiriwatknp
authored
[Select] Implement pointer cancellation (#45789)
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com> Co-authored-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com> Co-authored-by: Siriwat K <siriwatkunaporn@gmail.com>
1 parent 3d9d290 commit 6c9df3c

File tree

2 files changed

+254
-11
lines changed

2 files changed

+254
-11
lines changed

packages/mui-material/src/Select/Select.test.js

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,74 @@ describe('<Select />', () => {
3434
skip: ['componentProp', 'componentsProp', 'themeVariants', 'themeStyleOverrides'],
3535
}));
3636

37+
describe('Pointer Cancellation', () => {
38+
beforeEach(function beforeEachCallback() {
39+
// Run these tests only in browser because JSDOM doesn't have getBoundingClientRect() API
40+
if (/jsdom/.test(window.navigator.userAgent)) {
41+
this.skip();
42+
}
43+
});
44+
45+
it('should close the menu when mouse is outside the select', () => {
46+
render(
47+
<Select value="" MenuProps={{ slotProps: { backdrop: { 'data-testid': 'backdrop' } } }}>
48+
<MenuItem value="">none</MenuItem>
49+
<MenuItem value={10}>Ten</MenuItem>
50+
</Select>,
51+
);
52+
const trigger = screen.getByRole('combobox');
53+
54+
// Open the menu
55+
fireEvent.mouseDown(trigger);
56+
expect(screen.getByRole('listbox')).not.to.equal(null);
57+
58+
// Simulate mouse up outside the menu. The mouseup target is the backdrop when the menu is opened.
59+
fireEvent.mouseUp(screen.getByTestId('backdrop'), { clientX: 60, clientY: 10 });
60+
61+
// Menu should be closed now
62+
expect(screen.queryByRole('listbox', { hidden: false })).to.equal(null);
63+
});
64+
65+
it('should not close the menu when mouse is inside the trigger', () => {
66+
render(
67+
<Select value="">
68+
<MenuItem value="">none</MenuItem>
69+
<MenuItem value={10}>Ten</MenuItem>
70+
</Select>,
71+
);
72+
const trigger = screen.getByRole('combobox');
73+
74+
// Open the menu
75+
fireEvent.mouseDown(trigger);
76+
expect(screen.getByRole('listbox')).not.to.equal(null);
77+
78+
// Simulate mouse up inside the trigger
79+
fireEvent.mouseUp(trigger, { clientX: 20, clientY: 20 });
80+
81+
// Menu should still be open
82+
expect(screen.queryByRole('listbox', { hidden: false })).not.to.equal(null);
83+
});
84+
85+
it('should not close the menu when releasing on menu paper', () => {
86+
render(
87+
<Select value="" MenuProps={{ slotProps: { paper: { 'data-testid': 'paper' } } }}>
88+
<MenuItem value="">none</MenuItem>
89+
<MenuItem value={10}>Ten</MenuItem>
90+
</Select>,
91+
);
92+
const trigger = screen.getByRole('combobox');
93+
94+
// Open the menu
95+
fireEvent.mouseDown(trigger);
96+
97+
// Simulate mouse up on menu paper
98+
fireEvent.mouseUp(screen.getByTestId('paper'));
99+
100+
// Menu should still be open
101+
expect(screen.getByRole('listbox')).not.to.equal(null);
102+
});
103+
});
104+
37105
describe('prop: inputProps', () => {
38106
it('should be able to provide a custom classes property', () => {
39107
render(
@@ -931,6 +999,23 @@ describe('<Select />', () => {
931999

9321000
expect(backdrop.style).to.have.property('backgroundColor', 'red');
9331001
});
1002+
1003+
it('should merge ref coming from paper props', () => {
1004+
const paperRef = React.createRef();
1005+
1006+
render(
1007+
<Select
1008+
defaultValue="10"
1009+
open
1010+
MenuProps={{ slotProps: { paper: { 'data-testid': 'paper', ref: paperRef } } }}
1011+
>
1012+
<MenuItem value="10">Ten</MenuItem>
1013+
<MenuItem value="20">Twenty</MenuItem>
1014+
</Select>,
1015+
);
1016+
1017+
expect(paperRef.current).to.equal(screen.getByTestId('paper'));
1018+
});
9341019
});
9351020

9361021
describe('prop: SelectDisplayProps', () => {
@@ -1352,6 +1437,72 @@ describe('<Select />', () => {
13521437
combinedStyle,
13531438
);
13541439
});
1440+
1441+
it('should be able to select the items on click of options', async () => {
1442+
// Restore real timers — needed for `userEvent` to work correctly with async events.
1443+
clock.restore();
1444+
1445+
const { user } = render(
1446+
<Select defaultValue={[10]} multiple>
1447+
<MenuItem value={10}>Ten</MenuItem>
1448+
<MenuItem value={20}>Twenty</MenuItem>
1449+
<MenuItem value={30}>Thirty</MenuItem>
1450+
</Select>,
1451+
);
1452+
1453+
const trigger = screen.getByRole('combobox');
1454+
1455+
expect(trigger).to.have.text('Ten');
1456+
1457+
// open the menu
1458+
fireEvent.mouseDown(trigger);
1459+
1460+
const listbox = screen.queryByRole('listbox');
1461+
expect(listbox).not.to.equal(null);
1462+
1463+
const options = screen.getAllByRole('option');
1464+
// Click second option
1465+
await user.click(options[1]);
1466+
1467+
expect(trigger).to.have.text('Ten, Twenty');
1468+
1469+
// Menu is still open in case of multiple
1470+
expect(listbox).not.to.equal(null);
1471+
});
1472+
1473+
it('should be able to select the items on mouseup', async () => {
1474+
// Restore real timers — needed for `userEvent` to work correctly with async events.
1475+
clock.restore();
1476+
1477+
const { user } = render(
1478+
<Select defaultValue={[10]} multiple>
1479+
<MenuItem value={10}>Ten</MenuItem>
1480+
<MenuItem value={20}>Twenty</MenuItem>
1481+
<MenuItem value={30}>Thirty</MenuItem>
1482+
</Select>,
1483+
);
1484+
1485+
const trigger = screen.getByRole('combobox');
1486+
1487+
expect(trigger).to.have.text('Ten');
1488+
1489+
// Open the menu without releasing the mouse
1490+
await user.pointer({ keys: '[MouseLeft>]', target: trigger });
1491+
1492+
const listbox = screen.queryByRole('listbox');
1493+
expect(listbox).not.to.equal(null);
1494+
1495+
const options = screen.getAllByRole('option');
1496+
// Mouse up on second option, release the mouse
1497+
await user.pointer(
1498+
{ keys: '[/MouseLeft]', target: options[1] }, // mouseup
1499+
);
1500+
1501+
expect(trigger).to.have.text('Ten, Twenty');
1502+
1503+
// Menu is still open in case of multiple
1504+
expect(listbox).not.to.equal(null);
1505+
});
13551506
});
13561507

13571508
describe('prop: autoFocus', () => {
@@ -1461,6 +1612,22 @@ describe('<Select />', () => {
14611612
expect(onClick.callCount).to.equal(1);
14621613
});
14631614

1615+
it('should pass onMouseUp prop to MenuItem', () => {
1616+
const onMouseUp = spy();
1617+
const { getAllByRole } = render(
1618+
<Select open value="30">
1619+
<MenuItem onMouseUp={onMouseUp} value={30}>
1620+
Thirty
1621+
</MenuItem>
1622+
</Select>,
1623+
);
1624+
1625+
const options = getAllByRole('option');
1626+
fireEvent.mouseUp(options[0]);
1627+
1628+
expect(onMouseUp.callCount).to.equal(1);
1629+
});
1630+
14641631
// https://github.com/testing-library/react-testing-library/issues/322
14651632
// https://x.com/devongovett/status/1248306411508916224
14661633
it('should handle the browser autofill event and simple testing-library API', () => {
@@ -1839,4 +2006,28 @@ describe('<Select />', () => {
18392006
expect(container.querySelector('.MuiSelect-iconFilled')).not.to.equal(null);
18402007
expect(container.querySelector('.MuiSelect-filled ~ .MuiSelect-icon')).not.to.equal(null);
18412008
});
2009+
2010+
it('should select the item on mouse up', () => {
2011+
render(
2012+
<Select defaultValue={10}>
2013+
<MenuItem value={10}>Ten</MenuItem>
2014+
<MenuItem value={20}>Twenty</MenuItem>
2015+
<MenuItem value={30}>Thirty</MenuItem>
2016+
</Select>,
2017+
);
2018+
2019+
const trigger = screen.getByRole('combobox');
2020+
2021+
// open the menu
2022+
fireEvent.mouseDown(trigger);
2023+
expect(screen.queryByRole('listbox')).not.to.equal(null);
2024+
2025+
const options = screen.getAllByRole('option');
2026+
fireEvent.mouseUp(options[1]);
2027+
2028+
expect(trigger).to.have.text('Twenty');
2029+
2030+
// Menu should be closed now
2031+
expect(screen.queryByRole('listbox', { hidden: false })).to.equal(null);
2032+
});
18422033
});

packages/mui-material/src/Select/SelectInput.js

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
133133
...other
134134
} = props;
135135

136+
const paperProps = {
137+
...MenuProps.PaperProps,
138+
...MenuProps.slotProps?.paper,
139+
};
140+
136141
const [value, setValueState] = useControlled({
137142
controlled: valueProp,
138143
default: defaultValue,
@@ -146,10 +151,15 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
146151

147152
const inputRef = React.useRef(null);
148153
const displayRef = React.useRef(null);
154+
const paperRef = React.useRef(null);
155+
const didPointerDownRef = React.useRef(false);
156+
149157
const [displayNode, setDisplayNode] = React.useState(null);
150158
const { current: isOpenControlled } = React.useRef(openProp != null);
151159
const [menuMinWidthState, setMenuMinWidthState] = React.useState();
160+
152161
const handleRef = useForkRef(ref, inputRefProp);
162+
const handlePaperRef = useForkRef(paperProps.ref, paperRef);
153163

154164
const handleDisplayRef = React.useCallback((node) => {
155165
displayRef.current = node;
@@ -233,6 +243,37 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
233243
event.preventDefault();
234244
displayRef.current.focus();
235245

246+
const doc = ownerDocument(event.currentTarget);
247+
248+
function handleMouseUp(mouseEvent) {
249+
if (!displayRef.current) {
250+
return;
251+
}
252+
253+
// mouse is over the options/menuitem, don't close the menu
254+
if (paperRef.current.contains(mouseEvent.target)) {
255+
return;
256+
}
257+
258+
const triggerElement = displayRef.current.getBoundingClientRect();
259+
260+
// mouse is inside the trigger, don't close the menu
261+
if (
262+
mouseEvent.clientX >= triggerElement.left &&
263+
mouseEvent.clientX <= triggerElement.right &&
264+
mouseEvent.clientY >= triggerElement.top &&
265+
mouseEvent.clientY <= triggerElement.bottom
266+
) {
267+
return;
268+
}
269+
270+
// close the menu
271+
update(false, mouseEvent);
272+
}
273+
274+
// `{ once: true }` to automatically remove the listener, see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#once
275+
doc.addEventListener('mouseup', handleMouseUp, { once: true });
276+
236277
update(true, event);
237278
};
238279

@@ -257,7 +298,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
257298
}
258299
};
259300

260-
const handleItemClick = (child) => (event) => {
301+
const handleItemSelect = (child) => (event) => {
261302
let newValue;
262303

263304
// We use the tabindex attribute to signal the available options.
@@ -277,10 +318,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
277318
newValue = child.props.value;
278319
}
279320

280-
if (child.props.onClick) {
281-
child.props.onClick(event);
282-
}
283-
284321
if (value !== newValue) {
285322
setValueState(newValue);
286323

@@ -394,7 +431,26 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
394431

395432
return React.cloneElement(child, {
396433
'aria-selected': selected ? 'true' : 'false',
397-
onClick: handleItemClick(child),
434+
onPointerDown: () => {
435+
didPointerDownRef.current = true;
436+
},
437+
onClick: (event) => {
438+
didPointerDownRef.current = false;
439+
if (child.props.onClick) {
440+
child.props.onClick(event);
441+
}
442+
handleItemSelect(child)(event);
443+
},
444+
onMouseUp: (event) => {
445+
if (didPointerDownRef.current) {
446+
didPointerDownRef.current = false;
447+
return;
448+
}
449+
if (child.props.onMouseUp) {
450+
child.props.onMouseUp(event);
451+
}
452+
handleItemSelect(child)(event);
453+
},
398454
onKeyUp: (event) => {
399455
if (event.key === ' ') {
400456
// otherwise our MenuItems dispatches a click event
@@ -482,11 +538,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
482538

483539
const classes = useUtilityClasses(ownerState);
484540

485-
const paperProps = {
486-
...MenuProps.PaperProps,
487-
...MenuProps.slotProps?.paper,
488-
};
489-
490541
const listProps = {
491542
...MenuProps.MenuListProps,
492543
...MenuProps.slotProps?.list,
@@ -572,6 +623,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
572623
},
573624
paper: {
574625
...paperProps,
626+
ref: handlePaperRef,
575627
style: {
576628
minWidth: menuMinWidth,
577629
...(paperProps != null ? paperProps.style : null),

0 commit comments

Comments
 (0)