Bug 1548369 - Reimplement token previews without onMouseLeave.
authorjaril <jarilvalenciano@gmail.com>
Thu, 16 May 2019 14:00:31 +0000
changeset 474204 f460289202a100a2efec62ee77a370594111706c
parent 474203 271c49dac12ccb1dc07467fa34d9f2c30ffeac3b
child 474205 8c0b5bf5f7b31fdf53961aa0f5837bdf4c22b7e3
push id113144
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:44:55 +0000
treeherdermozilla-inbound@f4c4b796f845 [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/D31452
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
@@ -4,16 +4,17 @@
 
 // @flow
 
 import { isConsole } from "../utils/preview";
 import { findBestMatchExpression } from "../utils/ast";
 import { PROMISE } from "./utils/middleware/promise";
 import { getExpressionFromCoords } from "../utils/editor/get-expression";
 import { isOriginal } from "../utils/source";
+import { isTesting } from "devtools-environment";
 
 import {
   getPreview,
   isLineInScope,
   isSelectedFrameVisible,
   getSelectedSource,
   getSelectedFrame,
   getSymbols,
@@ -65,32 +66,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 +132,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") && !isTesting()) {
+          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,63 @@ 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;
-    if (cx.isPaused) {
+    const { cx, editor, updatePreview, preview } = this.props;
+
+    if (cx.isPaused || (!preview || target !== preview.target)) {
       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 +116,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
 };