Skip to content

Commit

Permalink
Components: Avoid displaying tooltip on focus from mousedown
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Jul 29, 2019
1 parent d45c8ea commit 3cb05d1
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 0 deletions.
52 changes: 52 additions & 0 deletions packages/components/src/tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,29 @@ class Tooltip extends Component {
TOOLTIP_DELAY
);

/**
* Prebound `isInMouseDown` handler, created as a constant reference to
* assure ability to remove in component unmount.
*/
this.cancelIsMouseDown = this.createSetIsMouseDown( false );

/**
* Whether a the mouse is currently pressed, used in determining whether
* to handle a focus event as displaying the tooltip immediately.
*
* @type {boolean}
*/
this.isInMouseDown = false;

this.state = {
isOver: false,
};
}

componentWillUnmount() {
this.delayedSetIsOver.cancel();

document.removeEventListener( 'mouseup', this.cancelIsMouseDown );
}

emitToChild( eventName, event ) {
Expand Down Expand Up @@ -71,6 +87,13 @@ class Tooltip extends Component {
return;
}

// A focus event will occur as a result of a mouse click, but it
// should be disambiguated between interacting with the button and
// using an explicit focus shift as a cue to display the tooltip.
if ( 'focus' === event.type && this.isInMouseDown ) {
return;
}

// Needed in case unsetting is over while delayed set pending, i.e.
// quickly blur/mouseleave before delayedSetIsOver is called
this.delayedSetIsOver.cancel();
Expand All @@ -88,6 +111,34 @@ class Tooltip extends Component {
};
}

/**
* Creates an event callback to handle assignment of the `isInMouseDown`
* instance property in response to a `mousedown` or `mouseup` event.
*
* @param {booelan} isMouseDown Whether handler is to be created for the
* `mousedown` event, as opposed to `mouseup`.
*
* @return {Function} Event callback handler.
*/
createSetIsMouseDown( isMouseDown ) {
return ( event ) => {
// Preserve original child callback behavior
this.emitToChild( isMouseDown ? 'onMouseDown' : 'onMouseUp', event );

// On mouse down, the next `mouseup` should revert the value of the
// instance property and remove its own event handler. The bind is
// made on the document since the `mouseup` might not occur within
// the bounds of the element.
document[
isMouseDown ?
'addEventListener' :
'removeEventListener'
]( 'mouseup', this.cancelIsMouseDown );

this.isInMouseDown = isMouseDown;
};
}

render() {
const { children, position, text, shortcut } = this.props;
if ( Children.count( children ) !== 1 ) {
Expand All @@ -107,6 +158,7 @@ class Tooltip extends Component {
onClick: this.createToggleIsOver( 'onClick' ),
onFocus: this.createToggleIsOver( 'onFocus' ),
onBlur: this.createToggleIsOver( 'onBlur' ),
onMouseDown: this.createSetIsMouseDown( true ),
children: concatChildren(
child.props.children,
isOver && (
Expand Down
38 changes: 38 additions & 0 deletions packages/components/src/tooltip/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,44 @@ describe( 'Tooltip', () => {
expect( popover ).toHaveLength( 1 );
} );

it( 'should show not popover on focus as result of mousedown', () => {
const originalOnMouseDown = jest.fn();
const originalOnMouseUp = jest.fn();
const wrapper = mount(
<Tooltip text="Help text">
<button
onMouseDown={ originalOnMouseDown }
onMouseUp={ originalOnMouseUp }
>
Hover Me!
</button>
</Tooltip>
);

const button = wrapper.find( 'button' );

let event;

event = { type: 'mousedown' };
button.simulate( event.type, event );
expect( originalOnMouseDown ).toHaveBeenCalledWith( expect.objectContaining( {
type: event.type,
} ) );

event = { type: 'focus', currentTarget: {} };
button.simulate( event.type, event );

const popover = wrapper.find( 'Popover' );
expect( wrapper.state( 'isOver' ) ).toBe( false );
expect( popover ).toHaveLength( 0 );

event = new window.MouseEvent( 'mouseup' );
document.dispatchEvent( event );
expect( originalOnMouseUp ).toHaveBeenCalledWith( expect.objectContaining( {
type: event.type,
} ) );
} );

it( 'should show popover on delayed mouseenter', () => {
const originalMouseEnter = jest.fn();
const wrapper = TestUtils.renderIntoDocument(
Expand Down

0 comments on commit 3cb05d1

Please sign in to comment.