[Select] Improve response, react to mouse down (#17978)

mui · Nov 23, 2019 · 863dab7 · 863dab7
1 parent ce0c6e0
commit 863dab7
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 30 deletions.
diff --git a/packages/material-ui/src/Select/Select.test.js b/packages/material-ui/src/Select/Select.test.js
@@ -95,25 +95,21 @@ describe('<Select />', () => {
     const { getByRole, getAllByRole, queryByRole } = render(
       <Select
         onBlur={handleBlur}
+        value=""
         onMouseDown={event => {
           // simulating certain platforms that focus on mousedown
           if (event.defaultPrevented === false) {
             event.currentTarget.focus();
           }
         }}
-        value=""
       >
         <MenuItem value="">none</MenuItem>
         <MenuItem value={10}>Ten</MenuItem>
       </Select>,
     );
     const trigger = getByRole('button');
 
-    // simulating user click
-    act(() => {
-      fireEvent.mouseDown(trigger);
-      trigger.click();
-    });
+    fireEvent.mouseDown(trigger);
 
     expect(handleBlur.callCount).to.equal(0);
     expect(getByRole('listbox')).to.be.ok;
@@ -196,9 +192,7 @@ describe('<Select />', () => {
   it('should focus list if no selection', () => {
     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.
     expect(getByRole('listbox')).to.have.focus;
@@ -224,7 +218,7 @@ describe('<Select />', () => {
           <MenuItem value="2" />
         </Select>,
       );
-      getByRole('button').click();
+      fireEvent.mouseDown(getByRole('button'));
       getAllByRole('option')[1].click();
 
       expect(onChangeHandler.calledOnce).to.be.true;
@@ -514,7 +508,7 @@ describe('<Select />', () => {
         </Select>,
       );
 
-      fireEvent.click(getByRole('button'));
+      fireEvent.mouseDown(getByRole('button'));
       act(() => {
         clock.tick(99);
       });
@@ -612,10 +606,7 @@ describe('<Select />', () => {
       }
       const { getByRole, queryByRole } = render(<ControlledWrapper />);
 
-      act(() => {
-        getByRole('button').click();
-      });
-
+      fireEvent.mouseDown(getByRole('button'));
       expect(getByRole('listbox')).to.be.ok;
 
       act(() => {
@@ -654,10 +645,7 @@ describe('<Select />', () => {
       const button = getByRole('button');
       stub(button, 'clientWidth').get(() => 14);
 
-      act(() => {
-        button.click();
-      });
-
+      fireEvent.mouseDown(button);
       expect(getByTestId('paper').style).to.have.property('minWidth', '14px');
     });
 
@@ -670,10 +658,7 @@ describe('<Select />', () => {
       const button = getByRole('button');
       stub(button, 'clientWidth').get(() => 14);
 
-      act(() => {
-        button.click();
-      });
-
+      fireEvent.mouseDown(button);
       expect(getByTestId('paper').style).to.have.property('minWidth', '');
     });
   });
@@ -794,7 +779,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]);
 

diff --git a/packages/material-ui/src/Select/SelectInput.js b/packages/material-ui/src/Select/SelectInput.js
@@ -123,7 +123,11 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
     }
   };
 
-  const handleClick = event => {
+  const handleMouseDown = event => {
+    // Hijack the default focus behavior.
+    event.preventDefault();
+    displayNode.focus();
+
     update(true, event);
   };
 
@@ -322,7 +326,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 : handleMouseDown}
         onBlur={handleBlur}
         onFocus={onFocus}
         {...SelectDisplayProps}

diff --git a/packages/material-ui/test/integration/Select.test.js b/packages/material-ui/test/integration/Select.test.js
@@ -58,8 +58,7 @@ describe('<Select> integration', () => {
       const trigger = getByRole('button');
       // 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.have.focus;
@@ -81,8 +80,7 @@ describe('<Select> integration', () => {
       expect(trigger).to.have.text('Ten');
       // 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.have.focus;