Skip to content

Commit

Permalink
Components: Avoid displaying tooltip on focus from mousedown (#16800)
Browse files Browse the repository at this point in the history
* Components: Avoid displaying tooltip on focus from mousedown

* Testing: Update Tooltip snapshots

* Components: Tooltip: Add JSDoc type for assigned cancelIsMouseDown
  • Loading branch information
aduth authored Jul 30, 2019
1 parent 87d5d5a commit 208ff09
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ exports[`ColorPicker should commit changes to all views on blur 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down Expand Up @@ -321,6 +322,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = DOWN 1`] =
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down Expand Up @@ -494,6 +496,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = ENTER 1`] =
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down Expand Up @@ -667,6 +670,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = UP 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down Expand Up @@ -840,6 +844,7 @@ exports[`ColorPicker should only update input view for draft changes 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down Expand Up @@ -1013,6 +1018,7 @@ exports[`ColorPicker should render color picker 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down
54 changes: 54 additions & 0 deletions packages/components/src/tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,31 @@ class Tooltip extends Component {
TOOLTIP_DELAY
);

/**
* Prebound `isInMouseDown` handler, created as a constant reference to
* assure ability to remove in component unmount.
*
* @type {Function}
*/
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 +89,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 +113,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 +160,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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exports[`MoreMenu should match snapshot 1`] = `
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
>
Expand All @@ -56,6 +57,7 @@ exports[`MoreMenu should match snapshot 1`] = `
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
type="button"
Expand Down

0 comments on commit 208ff09

Please sign in to comment.