Bug 1548369 - Reimplement token previews without onMouseLeave.
☠☠ backed out by 9dd4eba320e6 ☠ ☠
authorjaril <jarilvalenciano@gmail.com>
Wed, 15 May 2019 16:37:23 +0000
changeset 532790 03b1762fb0734dc1f58ce5453a8aef8770745538
parent 532789 9e68d485f7ccea5d8f065fb5f833858e71b4b04a
child 532791 f94cc66e79ecbf3a072858aaa5611ca4ec035b9c
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1548369
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1548369 - Reimplement token previews without onMouseLeave. This implements editor previews in a way that doesn't rely on mouseLeave events. It does this actively by using the event loop to poll whether it should clear or not, and if it's not hovered on the token or the preview, it clears the preview. Differential Revision: https://phabricator.services.mozilla.com/D31112
devtools/client/debugger/src/actions/preview.js
devtools/client/debugger/src/components/Editor/Preview/Popup.js
devtools/client/debugger/src/components/Editor/Preview/index.js
devtools/client/debugger/src/components/shared/Popover.js
devtools/client/debugger/src/reducers/ast.js
--- a/devtools/client/debugger/src/actions/preview.js
+++ b/devtools/client/debugger/src/actions/preview.js
@@ -65,32 +65,37 @@ export function updatePreview(
     }
 
     const { expression, location } = match;
 
     if (isConsole(expression)) {
       return;
     }
 
-    dispatch(setPreview(cx, expression, location, tokenPos, cursorPos));
+    dispatch(setPreview(cx, expression, location, tokenPos, cursorPos, target));
   };
 }
 
 export function setPreview(
   cx: Context,
   expression: string,
   location: AstLocation,
   tokenPos: Position,
-  cursorPos: ClientRect
+  cursorPos: ClientRect,
+  target: HTMLElement
 ) {
   return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
     await dispatch({
       type: "SET_PREVIEW",
       cx,
       [PROMISE]: (async function() {
+        if (getPreview(getState())) {
+          dispatch(clearPreview(cx));
+        }
+
         const source = getSelectedSource(getState());
         if (!source) {
           return;
         }
 
         const thread = getCurrentThread(getState());
         const selectedFrame = getSelectedFrame(getState(), thread);
 
@@ -126,24 +131,32 @@ export function setPreview(
 
         const root = {
           name: expression,
           path: expression,
           contents: { value: result }
         };
         const properties = await client.loadObjectProperties(root);
 
+        // The first time a popup is rendered, the mouse should be hovered
+        // on the token. If it happens to be hovered on whitespace, it should
+        // not render anything
+        if (!target.matches(":hover")) {
+          return;
+        }
+
         return {
           expression,
           result,
           properties,
           root,
           location,
           tokenPos,
-          cursorPos
+          cursorPos,
+          target
         };
       })()
     });
   };
 }
 
 export function clearPreview(cx: Context) {
   return ({ dispatch, getState, client }: ThunkArgs) => {
--- a/devtools/client/debugger/src/components/Editor/Preview/Popup.js
+++ b/devtools/client/debugger/src/components/Editor/Preview/Popup.js
@@ -16,92 +16,90 @@ const {
 
 const { ObjectInspector, utils } = objectInspector;
 
 const {
   node: { nodeIsPrimitive, nodeIsFunction, nodeIsObject }
 } = utils;
 
 import actions from "../../../actions";
-import { getThreadContext } from "../../../selectors";
+import { getThreadContext, getPreview } from "../../../selectors";
 import Popover from "../../shared/Popover";
 import PreviewFunction from "../../shared/PreviewFunction";
 
 import { createObjectClient } from "../../../client/firefox";
 
 import "./Popup.css";
 
 import type { Coords } from "../../shared/Popover";
 import type { ThreadContext } from "../../../types";
 import type { PreviewValue } from "../../../reducers/types";
 
 type Props = {
   cx: ThreadContext,
   preview: PreviewValue,
-  onClose: () => void,
   editor: any,
   editorRef: ?HTMLDivElement,
   addExpression: typeof actions.addExpression,
   selectSourceURL: typeof actions.selectSourceURL,
   openLink: typeof actions.openLink,
   openElementInInspector: typeof actions.openElementInInspectorCommand,
   highlightDomElement: typeof actions.highlightDomElement,
-  unHighlightDomElement: typeof actions.unHighlightDomElement
+  unHighlightDomElement: typeof actions.unHighlightDomElement,
+  clearPreview: typeof actions.clearPreview
 };
 
 type State = {
   top: number
 };
 
-function inPreview(event) {
-  const relatedTarget: Element = (event.relatedTarget: any);
-
-  if (
-    !relatedTarget ||
-    (relatedTarget.classList &&
-      relatedTarget.classList.contains("preview-expression"))
-  ) {
-    return true;
-  }
-
-  // $FlowIgnore
-  const inPreviewSelection = document
-    .elementsFromPoint(event.clientX, event.clientY)
-    .some(el => el.classList.contains("preview-selection"));
-
-  return inPreviewSelection;
-}
-
 export class Popup extends Component<Props, State> {
   marker: any;
   pos: any;
+  popup: ?HTMLDivElement;
+  timerId: ?IntervalID;
 
   constructor(props: Props) {
     super(props);
     this.state = {
       top: 0
     };
   }
 
-  onMouseLeave = (e: SyntheticMouseEvent<HTMLDivElement>) => {
-    const relatedTarget: Element = (e.relatedTarget: any);
+  componentDidMount() {
+    this.startTimer();
+  }
+
+  componentWillUnmount() {
+    if (this.timerId) {
+      clearInterval(this.timerId);
+    }
+  }
 
-    if (!relatedTarget) {
-      return this.props.onClose();
+  startTimer() {
+    this.timerId = setInterval(this.onInterval, 300);
+  }
+
+  onInterval = () => {
+    const { preview, clearPreview, cx } = this.props;
+
+    // Don't clear the current preview if mouse is hovered on
+    // the current preview's element (target) or the popup element
+    const currentTarget = preview.target;
+    if (
+      currentTarget.matches(":hover") ||
+      (this.popup && this.popup.matches(":hover"))
+    ) {
+      return;
     }
 
-    if (!inPreview(e)) {
-      this.props.onClose();
-    }
-  };
-
-  onKeyDown = (e: KeyboardEvent) => {
-    if (e.key === "Escape") {
-      this.props.onClose();
-    }
+    // Clear the interval and the preview if it is not hovered
+    // on the current preview's element or the popup element
+    clearInterval(this.timerId);
+    return clearPreview(cx);
   };
 
   calculateMaxHeight = () => {
     const { editorRef } = this.props;
     if (!editorRef) {
       return "auto";
     }
     return editorRef.getBoundingClientRect().height - this.state.top;
@@ -112,16 +110,17 @@ export class Popup extends Component<Pro
       cx,
       selectSourceURL,
       preview: { result }
     } = this.props;
 
     return (
       <div
         className="preview-popup"
+        ref={a => (this.popup = a)}
         onClick={() =>
           selectSourceURL(cx, result.location.url, {
             line: result.location.line
           })
         }
       >
         <PreviewFunction func={result} />
       </div>
@@ -136,16 +135,17 @@ export class Popup extends Component<Pro
       highlightDomElement,
       unHighlightDomElement
     } = this.props;
 
     return (
       <div
         className="preview-popup"
         style={{ maxHeight: this.calculateMaxHeight() }}
+        ref={a => (this.popup = a)}
       >
         <ObjectInspector
           roots={properties}
           autoExpandDepth={0}
           disableWrap={true}
           focusable={false}
           openLink={openLink}
           createObjectClient={grip => createObjectClient(grip)}
@@ -159,17 +159,17 @@ export class Popup extends Component<Pro
   }
 
   renderSimplePreview() {
     const {
       openLink,
       preview: { result }
     } = this.props;
     return (
-      <div className="preview-popup">
+      <div className="preview-popup" ref={a => (this.popup = a)}>
         {Rep({
           object: result,
           mode: MODE.LONG,
           openLink
         })}
       </div>
     );
   }
@@ -218,46 +218,47 @@ export class Popup extends Component<Pro
 
     if (typeof result == "undefined" || result.optimizedOut) {
       return null;
     }
 
     return (
       <Popover
         targetPosition={cursorPos}
-        onMouseLeave={this.onMouseLeave}
-        onKeyDown={this.onKeyDown}
         type={type}
         onPopoverCoords={this.onPopoverCoords}
         editorRef={editorRef}
       >
         {this.renderPreview()}
       </Popover>
     );
   }
 }
 
 const mapStateToProps = state => ({
-  cx: getThreadContext(state)
+  cx: getThreadContext(state),
+  preview: getPreview(state)
 });
 
 const {
   addExpression,
   selectSourceURL,
   openLink,
   openElementInInspectorCommand,
   highlightDomElement,
-  unHighlightDomElement
+  unHighlightDomElement,
+  clearPreview
 } = actions;
 
 const mapDispatchToProps = {
   addExpression,
   selectSourceURL,
   openLink,
   openElementInInspector: openElementInInspectorCommand,
   highlightDomElement,
-  unHighlightDomElement
+  unHighlightDomElement,
+  clearPreview
 };
 
 export default connect(
   mapStateToProps,
   mapDispatchToProps
 )(Popup);
--- a/devtools/client/debugger/src/components/Editor/Preview/index.js
+++ b/devtools/client/debugger/src/components/Editor/Preview/index.js
@@ -25,29 +25,16 @@ type Props = {
   addExpression: typeof actions.addExpression,
   updatePreview: typeof actions.updatePreview
 };
 
 type State = {
   selecting: boolean
 };
 
-function inPopup(e) {
-  const { relatedTarget } = e;
-
-  if (!relatedTarget) {
-    return true;
-  }
-
-  const pop =
-    relatedTarget.closest(".tooltip") || relatedTarget.closest(".popover");
-
-  return pop;
-}
-
 function getElementFromPos(pos: DOMRect) {
   // We need to use element*s*AtPoint because the tooltip overlays
   // the token and thus an undesirable element may be returned
   const elementsAtPoint = [
     // $FlowIgnore
     ...document.elementsFromPoint(pos.x + pos.width / 2, pos.y + pos.height / 2)
   ];
 
@@ -64,60 +51,67 @@ class Preview extends PureComponent<Prop
   componentDidMount() {
     this.updateListeners();
   }
 
   componentWillUnmount() {
     const { codeMirror } = this.props.editor;
     const codeMirrorWrapper = codeMirror.getWrapperElement();
 
+    codeMirror.off("tokenenter", this.onTokenEnter);
     codeMirror.off("scroll", this.onScroll);
-    codeMirror.off("tokenenter", this.onTokenEnter);
-    codeMirror.off("tokenleave", this.onTokenLeave);
     codeMirrorWrapper.removeEventListener("mouseup", this.onMouseUp);
     codeMirrorWrapper.removeEventListener("mousedown", this.onMouseDown);
   }
 
   componentDidUpdate(prevProps) {
     this.updateHighlight(prevProps);
   }
 
   updateListeners(prevProps: ?Props) {
     const { codeMirror } = this.props.editor;
     const codeMirrorWrapper = codeMirror.getWrapperElement();
+    codeMirror.on("tokenenter", this.onTokenEnter);
     codeMirror.on("scroll", this.onScroll);
-    codeMirror.on("tokenenter", this.onTokenEnter);
-    codeMirror.on("tokenleave", this.onTokenLeave);
     codeMirrorWrapper.addEventListener("mouseup", this.onMouseUp);
     codeMirrorWrapper.addEventListener("mousedown", this.onMouseDown);
   }
 
   updateHighlight(prevProps) {
     const { preview } = this.props;
 
-    if (preview && !preview.updating) {
+    if (preview && !preview.updating && preview.target.matches(":hover")) {
       const target = getElementFromPos(preview.cursorPos);
       target && target.classList.add("preview-selection");
     }
 
-    if (prevProps.preview && !prevProps.preview.updating) {
+    if (
+      prevProps.preview &&
+      !prevProps.preview.updating &&
+      prevProps.preview !== preview
+    ) {
       const target = getElementFromPos(prevProps.preview.cursorPos);
       target && target.classList.remove("preview-selection");
     }
   }
 
   onTokenEnter = ({ target, tokenPos }) => {
-    const { cx, updatePreview, editor } = this.props;
+    const { cx, editor, updatePreview, preview } = this.props;
+
     if (cx.isPaused) {
+      if (preview && target === preview.target) {
+        return;
+      }
+
       updatePreview(cx, target, tokenPos, editor.codeMirror);
     }
   };
 
-  onTokenLeave = e => {
-    if (this.props.cx.isPaused && !inPopup(e)) {
+  onScroll = () => {
+    if (this.props.cx.isPaused) {
       this.props.clearPreview(this.props.cx);
     }
   };
 
   onMouseUp = () => {
     if (this.props.cx.isPaused) {
       this.setState({ selecting: false });
       return true;
@@ -126,40 +120,27 @@ class Preview extends PureComponent<Prop
 
   onMouseDown = () => {
     if (this.props.cx.isPaused) {
       this.setState({ selecting: true });
       return true;
     }
   };
 
-  onScroll = () => {
-    if (this.props.cx.isPaused) {
-      this.props.clearPreview(this.props.cx);
-    }
-  };
-
-  onClose = e => {
-    if (this.props.cx.isPaused) {
-      this.props.clearPreview(this.props.cx);
-    }
-  };
-
   render() {
     const { preview } = this.props;
     if (!preview || preview.updating || this.state.selecting) {
       return null;
     }
 
     return (
       <Popup
         preview={preview}
         editor={this.props.editor}
         editorRef={this.props.editorRef}
-        onClose={this.onClose}
       />
     );
   }
 }
 
 const mapStateToProps = state => ({
   cx: getThreadContext(state),
   preview: getPreview(state)
--- a/devtools/client/debugger/src/components/shared/Popover.js
+++ b/devtools/client/debugger/src/components/shared/Popover.js
@@ -9,18 +9,16 @@ import BracketArrow from "./BracketArrow
 
 import "./Popover.css";
 
 type Props = {
   editorRef: ?HTMLDivElement,
   targetPosition: Object,
   children: ?React$Element<any>,
   onPopoverCoords: Function,
-  onMouseLeave: (e: SyntheticMouseEvent<HTMLDivElement>) => void,
-  onKeyDown: (e: KeyboardEvent) => void,
   type?: "popover" | "tooltip"
 };
 
 type Orientation = "up" | "down" | "right";
 type TargetMid = {
   x: number,
   y: number
 };
@@ -41,19 +39,17 @@ class Popover extends Component<Props, S
       left: 0,
       top: 0,
       orientation: "down",
       targetMid: { x: 0, y: 0 }
     }
   };
 
   static defaultProps = {
-    onMouseLeave: () => {},
     onPopoverCoords: () => {},
-    onKeyDown: () => {},
     type: "popover"
   };
 
   componentDidMount() {
     const { type } = this.props;
     const coords =
       type == "popover" ? this.getPopoverCoords() : this.getTooltipCoords();
 
@@ -224,43 +220,37 @@ class Popover extends Component<Props, S
       Object.assign(arrowProps, { orientation: "left", top, left: -9 });
     }
 
     return <BracketArrow {...arrowProps} />;
   }
 
   renderPopover() {
     const { top, left, orientation, targetMid } = this.state.coords;
-    const { onMouseLeave, onKeyDown } = this.props;
     const arrow = this.getPopoverArrow(orientation, targetMid.x, targetMid.y);
 
     return (
       <div
         className={classNames("popover", `orientation-${orientation}`, {
           up: orientation === "up"
         })}
-        onMouseLeave={onMouseLeave}
-        onKeyDown={onKeyDown}
         style={{ top, left }}
         ref={c => (this.$popover = c)}
       >
         {arrow}
         {this.getChildren()}
       </div>
     );
   }
 
   renderTooltip() {
     const { top, left } = this.state.coords;
-    const { onMouseLeave, onKeyDown } = this.props;
     return (
       <div
         className="tooltip"
-        onMouseLeave={onMouseLeave}
-        onKeyDown={onKeyDown}
         style={{ top, left }}
         ref={c => (this.$tooltip = c)}
       >
         {this.getChildren()}
       </div>
     );
   }
 
--- a/devtools/client/debugger/src/reducers/ast.js
+++ b/devtools/client/debugger/src/reducers/ast.js
@@ -34,17 +34,18 @@ export type Preview = {| updating: true 
 export type PreviewValue = {|
   expression: string,
   result: Grip,
   root: Node,
   properties: GripProperties,
   location: AstLocation,
   cursorPos: any,
   tokenPos: AstLocation,
-  updating: false
+  updating: false,
+  target: HTMLDivElement
 |};
 
 export type ASTState = {
   +symbols: SymbolsMap,
   +outOfScopeLocations: ?Array<AstLocation>,
   +inScopeLines: ?Array<number>,
   +preview: Preview
 };