Material-ui: [Tooltip] disableTouchListener not working

[devhyunjae]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

demo:

Expected Behavior 🤔

given: Tooltip comp with disableTouchListener prop true
when: User touches button
then: Tooltip should not show

Steps to Reproduce 🕹

https://codesandbox.io/s/material-demo-seecb

[oliviertassinari]

@devhyunjae Thanks for the bug report. I can reproduce the problem. What do you think of this patch?

diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js
index 1d8154194..c16c11122 100644
--- a/packages/material-ui/src/Tooltip/Tooltip.js
+++ b/packages/material-ui/src/Tooltip/Tooltip.js
@@ -388,13 +388,16 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
     }, leaveDelay);
   };

-  const handleTouchStart = (event) => {
+  const detectTouchStart = (event) => {
     ignoreNonTouchEvents.current = true;
     const childrenProps = children.props;
-
     if (childrenProps.onTouchStart) {
       childrenProps.onTouchStart(event);
     }
+  }
+
+  const handleTouchStart = (event) => {
+    detectTouchStart(event);

     clearTimeout(leaveTimer.current);
     clearTimeout(closeTimer.current);
@@ -447,6 +450,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
     ...other,
     ...children.props,
     className: clsx(other.className, children.props.className),
+    onTouchStart: detectTouchStart,
   };

   if (!disableTouchListener) {

Do you want to submit a pull request? We would also need a new test case :).

[devhyunjae]

@oliviertassinari Nah i don't want to make more mess in here 😅Up to you mate.

[embeddedt]

@oliviertassinari I can make a pull request for this. I just need to figure out how to make a test.

[weslenng]

@oliviertassinari I was working on this issue but I can't break my test
Any suggestions?

diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js
index f55e60ace..829dbd80c 100644
--- a/packages/material-ui/src/Tooltip/Tooltip.test.js
+++ b/packages/material-ui/src/Tooltip/Tooltip.test.js
@@ -175,6 +175,15 @@ describe('<Tooltip />', () => {
       wrapper.update();
       assert.strictEqual(wrapper.find(Popper).props().open, false);
     });
+
+    it('should not respond with disableTouchListener', () => {
+      const wrapper = mount(<Tooltip {...defaultProps} disableTouchListener />);
+      const children = wrapper.find('#testChild');
+      children.simulate('touchStart');
+      clock.tick(1000);
+      wrapper.update();
+      assert.strictEqual(wrapper.find('[role="tooltip"]').exists(), false);
+    });
   });

   describe('mount', () => {

[oliviertassinari]

@weslenng Start by replacing enzyme for testing-library.

[weslenng]

I made the test below, but the Tooltip keeps not rendering
So, I think which the disableTouchListener prop is working correctly 🤔

diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js
index f55e60ace..94cd05c45 100644
--- a/packages/material-ui/src/Tooltip/Tooltip.test.js
+++ b/packages/material-ui/src/Tooltip/Tooltip.test.js
@@ -175,6 +175,21 @@ describe('<Tooltip />', () => {
       wrapper.update();
       assert.strictEqual(wrapper.find(Popper).props().open, false);
     });
+
+    it('should not respond with disableTouchListener', () => {
+      const { baseElement } = render(
+        <Tooltip
+          {...defaultProps}
+          disableTouchListener
+        />,
+      );
+      const children = baseElement.querySelector("#testChild");
+      const firstTouch = new Touch({ target: children });
+      fireEvent.touchStart(children, { touches: [firstTouch] });
+      clock.tick(1500);
+      fireEvent.touchEnd(children, { touches: [firstTouch] });
+      expect(baseElement.querySelector('[role="tooltip"]')).to.equal(null);
+    });
   });

   describe('mount', () => {

I tried to dispatch the touch event manually in the given reproduction and the behavior as the same

For reference, the code who I used is here

Note if I remove the disableTouchListener, the Tooltip renders normally, in both test and reproduction

What do you think?

[oliviertassinari]

@weslenng I think that we should reproduce the sequence of events that causes the problem, I suspect it's the focus event that makes the tooltip visible, but I'm not sure, need to console the triggered events.

[oliviertassinari]

@devhyunjae What was your use case for disabling the tooltip on touch devices?