Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Commit

Permalink
Fix button tooltip
Browse files Browse the repository at this point in the history
Summary:
This fixes an issue with D10276808. It turns out that the function returned from `addTooltip()` closes over the node so we can't simply re-call it.

The reason `addTooltip()` normally works at all is *very* subtle. It relies on the fact that the returned value changes each render, causing it to be invoked with `null` and then the same element in succession each time. The cleanup is then based on the time between those two (it sets a timeout each render).

Reviewed By: Goom11

Differential Revision: D10435657

fbshipit-source-id: b487d6050d0b953fb917e72cc5a5b1a6f21eb6d3
  • Loading branch information
matthewwithanm authored and Aman Agarwal committed Oct 19, 2018
1 parent c5d98c5 commit 509dee4
Showing 1 changed file with 11 additions and 10 deletions.
21 changes: 11 additions & 10 deletions modules/nuclide-commons-ui/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const ButtonTypeClassnames = Object.freeze({
*/
export class Button extends React.Component<Props> {
_button: ?HTMLButtonElement;
_lastTooltipOptions: ?atom$TooltipsAddOptions = null;
_removeTooltip: ?() => void;

focus(): void {
const node = ReactDOM.findDOMNode(this);
Expand All @@ -94,17 +94,18 @@ export class Button extends React.Component<Props> {
onButtonDOMNodeChange(this._button);
}

// If the element goes away (e.g. on unmount), remove the tooltip.
if (button == null && this._lastTooltipOptions != null) {
addTooltip(this._lastTooltipOptions)(null);
this._lastTooltipOptions = null;
// When the element goes away (e.g. on unmount), remove the tooltip.
if (button == null && this._removeTooltip != null) {
this._removeTooltip();
}

if (!disabled) {
if (tooltip && !disabled) {
addTooltip(tooltip)(button);
this._lastTooltipOptions = tooltip;
}
if (!disabled && tooltip && button != null) {
const updateTooltip = addTooltip(tooltip);
updateTooltip(button);
this._removeTooltip = () => {
updateTooltip(null);
this._removeTooltip = null;
};
}
};

Expand Down

0 comments on commit 509dee4

Please sign in to comment.