From e10ab9b4610f159fbf4e83086c4c96e1f4ca4cb3 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Mon, 16 Jul 2018 23:51:44 +0200 Subject: [PATCH] [Tooltip] Fix some new issues --- packages/material-ui/src/Popper/Popper.js | 2 +- packages/material-ui/src/Tooltip/Tooltip.js | 50 +++++++++++++++---- .../material-ui/src/Tooltip/Tooltip.test.js | 32 ++++++++++-- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/packages/material-ui/src/Popper/Popper.js b/packages/material-ui/src/Popper/Popper.js index 8d9d70cb692e34..fcefbea7f6ee57 100644 --- a/packages/material-ui/src/Popper/Popper.js +++ b/packages/material-ui/src/Popper/Popper.js @@ -111,7 +111,7 @@ class Popper extends React.Component { : { // It's using scrollParent by default, we can use the viewport when using a portal. preventOverflow: { - boundariesElement: 'viewport', + boundariesElement: 'window', }, }), ...modifiers, diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js index 8bbb2992db8bb1..fc50f336ea7203 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.js +++ b/packages/material-ui/src/Tooltip/Tooltip.js @@ -75,6 +75,11 @@ class Tooltip extends React.Component { defaultId = null; + internalState = { + hover: false, + focus: false, + }; + constructor(props) { super(props); @@ -91,9 +96,7 @@ class Tooltip extends React.Component { componentDidMount() { warning( - !this.childrenRef || - !this.childrenRef.disabled || - !this.childrenRef.tagName.toLowerCase() === 'button', + !this.childrenRef.disabled || !this.childrenRef.tagName.toLowerCase() === 'button', [ 'Material-UI: you are providing a disabled `button` child to the Tooltip component.', 'A disabled element does not fire events.', @@ -125,18 +128,31 @@ class Tooltip extends React.Component { const { children, enterDelay } = this.props; const childrenProps = children.props; - if (event.type === 'focus' && childrenProps.onFocus) { - childrenProps.onFocus(event); + if (event.type === 'focus') { + this.internalState.focus = true; + + if (childrenProps.onFocus) { + childrenProps.onFocus(event); + } } - if (event.type === 'mouseenter' && childrenProps.onMouseEnter) { - childrenProps.onMouseEnter(event); + if (event.type === 'mouseenter') { + this.internalState.hover = true; + + if (childrenProps.onMouseEnter) { + childrenProps.onMouseEnter(event); + } } if (this.ignoreNonTouchEvents && event.type !== 'touchstart') { return; } + // Remove the title ahead of time. + // We don't want to wait for the next render commit. + // We would risk displaying two tooltips at the same time (native + this one). + this.childrenRef.setAttribute('title', ''); + clearTimeout(this.enterTimer); clearTimeout(this.leaveTimer); if (enterDelay) { @@ -163,12 +179,20 @@ class Tooltip extends React.Component { const { children, leaveDelay } = this.props; const childrenProps = children.props; - if (event.type === 'blur' && childrenProps.onBlur) { - childrenProps.onBlur(event); + if (event.type === 'blur') { + this.internalState.focus = false; + + if (childrenProps.onBlur) { + childrenProps.onBlur(event); + } } - if (event.type === 'mouseleave' && childrenProps.onMouseLeave) { - childrenProps.onMouseLeave(event); + if (event.type === 'mouseleave') { + this.internalState.hover = false; + + if (childrenProps.onMouseLeave) { + childrenProps.onMouseLeave(event); + } } clearTimeout(this.enterTimer); @@ -184,6 +208,10 @@ class Tooltip extends React.Component { }; handleClose = event => { + if (this.internalState.focus || this.internalState.hover) { + return; + } + if (!this.isControlled) { this.setState({ open: false }); } diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js index 46d481914a22a9..5ef5e85d4cbe40 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.test.js +++ b/packages/material-ui/src/Tooltip/Tooltip.test.js @@ -20,7 +20,7 @@ describe('', () => { }; before(() => { - shallow = createShallow({ dive: true }); + shallow = createShallow({ dive: true, disableLifecycleMethods: true }); mount = createMount(); classes = getClasses(); }); @@ -58,11 +58,12 @@ describe('', () => { it('should respond to external events', () => { const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); assert.strictEqual(wrapper.state().open, false); - children.simulate('mouseEnter', {}); + children.simulate('mouseEnter', { type: 'mouseenter' }); assert.strictEqual(wrapper.state().open, true); - children.simulate('blur', {}); + children.simulate('mouseLeave', { type: 'mouseleave' }); assert.strictEqual(wrapper.state().open, false); }); @@ -73,17 +74,32 @@ describe('', () => { const wrapper = shallow( , ); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); assert.strictEqual(handleRequestOpen.callCount, 0); assert.strictEqual(handleClose.callCount, 0); - children.simulate('mouseEnter', { type: 'mouseover' }); + children.simulate('mouseEnter', { type: 'mouseenter' }); assert.strictEqual(handleRequestOpen.callCount, 1); assert.strictEqual(handleClose.callCount, 0); - children.simulate('blur', { type: 'blur' }); + children.simulate('mouseLeave', { type: 'mouseleave' }); assert.strictEqual(handleRequestOpen.callCount, 1); assert.strictEqual(handleClose.callCount, 1); }); + it('should close when the interaction is over', () => { + const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); + const children = wrapper.childAt(0).childAt(0); + assert.strictEqual(wrapper.state().open, false); + children.simulate('mouseEnter', { type: 'mouseenter' }); + children.simulate('focus', { type: 'focus' }); + assert.strictEqual(wrapper.state().open, true); + children.simulate('mouseLeave', { type: 'mouseleave' }); + assert.strictEqual(wrapper.state().open, true); + children.simulate('blur', { type: 'blur' }); + assert.strictEqual(wrapper.state().open, false); + }); + describe('touch screen', () => { let clock; @@ -97,6 +113,7 @@ describe('', () => { it('should not respond to quick events', () => { const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); children.simulate('touchStart', { type: 'touchstart', persist }); children.simulate('touchEnd', { type: 'touchend', persist }); @@ -107,6 +124,7 @@ describe('', () => { it('should open on long press', () => { const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); children.simulate('touchStart', { type: 'touchstart', persist }); children.simulate('focus', { type: 'focus' }); @@ -114,6 +132,7 @@ describe('', () => { clock.tick(1e3); assert.strictEqual(wrapper.state().open, true); children.simulate('touchEnd', { type: 'touchend', persist }); + children.simulate('blur', { type: 'blur' }); clock.tick(1500); assert.strictEqual(wrapper.state().open, false); }); @@ -138,6 +157,7 @@ describe('', () => { it('should take the enterDelay into account', () => { const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); children.simulate('focus', { type: 'focus', persist }); assert.strictEqual(wrapper.state().open, false); @@ -147,6 +167,7 @@ describe('', () => { it('should take the leaveDelay into account', () => { const wrapper = shallow(); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); children.simulate('focus', { type: 'focus' }); assert.strictEqual(wrapper.state().open, true); @@ -169,6 +190,7 @@ describe('', () => { , ); + wrapper.instance().childrenRef = document.createElement('div'); const children = wrapper.childAt(0).childAt(0); const type = name.slice(2).toLowerCase(); children.simulate(type, { type, persist });