Bug 1847517 - [devtools] Tweak preview popup style. r=devtools-review…

…ers,ochameau.

Don't display the arrow on the root node, and remove the ability to collapse it.
Add a border at the bottom of the root node so it looks like a header
Add a new `HEADER` Rep mode that will be used for the root node, so we're
showing relevant information in this context (e.g. the object type/class name)
but not the preview when it would be redundant with the content of the tooltip.

For example, we don't want to show the preview for simple objects, arrays, maps, …
but we still have a "longer" description for objects where the properties in
the preview are handpicked (e.g. elements, window, document, …)

Differential Revision: https://phabricator.services.mozilla.com/D185770

devtools/client/debugger/src/components/Editor/Preview/Popup.css

@@ -11,8 +11,9 @@
 }
 
 .popover .preview-popup {
-  padding: 10px;
+  padding: 5px 10px;
   max-width: 450px;
+  min-width: 200px;
 }
 
 .tooltip .preview-popup {

devtools/client/debugger/src/components/Editor/Preview/Popup.js

@@ -118,8 +118,9 @@ export class Popup extends Component {
         roots: [root],
         autoExpandDepth: 1,
         autoReleaseObjectActors: false,
-        mode: usesCustomFormatter ? MODE.LONG : null,
+        mode: usesCustomFormatter ? MODE.LONG : MODE.SHORT,
         disableWrap: true,
+        displayRootNodeAsHeader: true,
         focusable: false,
         openLink: this.props.openLink,
         defaultRep: Grip,

devtools/client/debugger/test/mochitest/browser_dbg-preview-frame.js

@@ -22,7 +22,8 @@ add_task(async function () {
     {
       line: 8,
       column: 4,
-      fields: [["secondCall()"]],
+      header: `function secondCall()`,
+      fields: [["name", `"secondCall"`]],
       expression: "secondCall",
     },
   ]);
@@ -35,15 +36,17 @@ add_task(async function () {
     {
       line: 6,
       column: 12,
-      fields: [["firstCall()"]],
+      header: `function firstCall()`,
+      fields: [["name", `"firstCall"`]],
       expression: "firstCall",
     },
   ]);
   await assertPreviews(dbg, [
     {
       line: 8,
       column: 4,
-      fields: [["secondCall()"]],
+      header: `function secondCall()`,
+      fields: [["name", `"secondCall"`]],
       expression: "secondCall",
     },
   ]);

devtools/client/debugger/test/mochitest/browser_dbg-preview-getter.js

@@ -8,6 +8,9 @@
 "use strict";
 
 add_task(async function () {
+  // Make sure the toolbox is tall enough to be able to display the whole popup.
+  await pushPref("devtools.toolbox.footer.height", 500);
+
   const dbg = await initDebugger(
     "doc-preview-getter.html",
     "preview-getter.js"
@@ -20,11 +23,7 @@ add_task(async function () {
   info("Hovers over 'this' token to display the preview.");
   const { tokenEl } = await tryHovering(dbg, 5, 8, "previewPopup");
 
-  info("Wait for top level node to expand");
-  await waitForElementWithSelector(
-    dbg,
-    ".preview-popup .tree-node:first-of-type .arrow.expanded"
-  );
+  info("Wait for properties to be loaded");
   await waitUntil(
     () => dbg.win.document.querySelectorAll(".preview-popup .node").length > 1
   );

devtools/client/debugger/test/mochitest/shared-head.js

@@ -2311,7 +2311,7 @@ async function assertPreviewTextValue(
  * @param {Array} previews
  */
 async function assertPreviews(dbg, previews) {
-  for (const { line, column, expression, result, fields } of previews) {
+  for (const { line, column, expression, result, header, fields } of previews) {
     info(" # Assert preview on " + line + ":" + column);
 
     if (result) {
@@ -2329,19 +2329,24 @@ async function assertPreviews(dbg, previews) {
         "popup"
       );
 
-      info("Wait for top level node to expand and child nodes to load");
-      await waitForElementWithSelector(
-        dbg,
-        ".preview-popup .node:first-of-type .arrow.expanded"
-      );
+      info("Wait for child nodes to load");
       await waitUntil(
         () => popupEl.querySelectorAll(".preview-popup .node").length > 1
       );
+      ok(true, "child nodes loaded");
 
       const oiNodes = Array.from(
         popupEl.querySelectorAll(".preview-popup .node")
       );
 
+      if (header) {
+        is(
+          oiNodes[0].querySelector(".objectBox").textContent,
+          header,
+          "popup has expected value"
+        );
+      }
+
       for (const [field, value] of fields) {
         const node = oiNodes.find(
           oiNode => oiNode.querySelector(".object-label")?.textContent === field

devtools/client/shared/components/object-inspector/components/ObjectInspector.css

@@ -97,3 +97,19 @@ button.remove-watchpoint-getorset {
 .theme-light button[class*="remove-watchpoint-"]:hover {
   background-color: transparent;
 }
+
+
+/* Specific style for when root nodes are displayed as header (e.g. in debugger preview popup */
+.tree.object-inspector.header-root-node {
+  .tree-node[aria-level="1"] {
+    border-block-end: 1px solid var(--theme-splitter-color);
+    padding-block-end: 4px;
+    margin-block-end: 4px;
+    overflow-x: clip;
+    word-break: keep-all;
+  }
+
+  .tree-node:not([aria-level="1"]) .tree-indent:first-of-type {
+    width: 0;
+  }
+}

devtools/client/shared/components/object-inspector/components/ObjectInspector.js

@@ -45,6 +45,9 @@ const {
   nodeHasGetter,
   nodeHasSetter,
 } = Utils.node;
+const {
+  MODE,
+} = require("resource://devtools/client/shared/components/reps/reps/constants.js");
 
 // This implements a component that renders an interactive inspector
 // for looking at JavaScript objects. It expects descriptions of
@@ -224,7 +227,15 @@ class ObjectInspector extends Component {
   }
 
   setExpanded(item, expand) {
-    if (!this.isNodeExpandable(item)) {
+    if (
+      !this.isNodeExpandable(item) ||
+      // Don't allow to collapse header root node
+      (
+        this.props.displayRootNodeAsHeader &&
+        !expand &&
+        this.props.roots[0] == item
+      )
+    ) {
       return;
     }
 
@@ -293,6 +304,7 @@ class ObjectInspector extends Component {
       disableWrap = false,
       expandedPaths,
       inline,
+      displayRootNodeAsHeader = false,
     } = this.props;
 
     const classNames = ["object-inspector"];
@@ -302,6 +314,9 @@ class ObjectInspector extends Component {
     if (disableWrap) {
       classNames.push("nowrap");
     }
+    if (displayRootNodeAsHeader) {
+      classNames.push("header-root-node");
+    }
 
     return Tree({
       className: classNames.join(" "),
@@ -332,6 +347,7 @@ class ObjectInspector extends Component {
           depth,
           focused,
           arrow,
+          mode: displayRootNodeAsHeader && this.props.roots[0] == item ? MODE.HEADER : this.props.mode ,
           expanded,
           setExpanded: this.setExpanded,
         }),

devtools/client/shared/components/object-inspector/components/ObjectInspectorItem.js

@@ -273,7 +273,7 @@ class ObjectInspectorItem extends Component {
 
     return dom.div(
       this.getTreeItemProps(),
-      arrow,
+      this.props.mode === MODE.HEADER ? null : arrow,
       labelElement,
       delimiter,
       value,

devtools/client/shared/components/reps/reps/comment-node.js

@@ -33,7 +33,7 @@ define(function (require, exports, module) {
     const { object, mode = MODE.SHORT, shouldRenderTooltip } = props;
 
     let { textContent } = object.preview;
-    if (mode === MODE.TINY) {
+    if (mode === MODE.TINY || mode === MODE.HEADER) {
       textContent = cropMultipleLines(textContent, 30);
     } else if (mode === MODE.SHORT) {
       textContent = cropString(textContent, 50);

devtools/client/shared/components/reps/reps/constants.js

@@ -11,6 +11,8 @@ define(function (require, exports, module) {
       TINY: Symbol("TINY"),
       SHORT: Symbol("SHORT"),
       LONG: Symbol("LONG"),
+      // Used by Debugger Preview popup
+      HEADER: Symbol("HEADER"),
     },
   };
 });

devtools/client/shared/components/reps/reps/error.js

@@ -58,30 +58,15 @@ define(function (require, exports, module) {
   function ErrorRep(props) {
     const { object, mode, shouldRenderTooltip, depth } = props;
     const preview = object.preview;
-    const customFormat = props.customFormat && mode !== MODE.TINY && !depth;
-
-    let name;
-    if (
-      preview &&
-      preview.name &&
-      typeof preview.name === "string" &&
-      preview.kind
-    ) {
-      switch (preview.kind) {
-        case "Error":
-          name = preview.name;
-          break;
-        case "DOMException":
-          name = preview.kind;
-          break;
-        default:
-          throw new Error("Unknown preview kind for the Error rep.");
-      }
-    } else {
-      name = "Error";
-    }
-
-    const errorTitle = mode === MODE.TINY ? name : `${name}: `;
+    const customFormat =
+      props.customFormat &&
+      mode !== MODE.TINY &&
+      mode !== MODE.HEADER &&
+      !depth;
+
+    const name = getErrorName(props);
+    const errorTitle =
+      mode === MODE.TINY || mode === MODE.HEADER ? name : `${name}: `;
     const content = [];
 
     if (customFormat) {
@@ -92,7 +77,7 @@ define(function (require, exports, module) {
       );
     }
 
-    if (mode !== MODE.TINY) {
+    if (mode !== MODE.TINY && mode !== MODE.HEADER) {
       const {
         Rep,
       } = require("devtools/client/shared/components/reps/reps/rep");
@@ -131,6 +116,29 @@ define(function (require, exports, module) {
     );
   }
 
+  function getErrorName(props) {
+    const { object } = props;
+    const preview = object.preview;
+
+    let name;
+    if (typeof preview?.name === "string" && preview.kind) {
+      switch (preview.kind) {
+        case "Error":
+          name = preview.name;
+          break;
+        case "DOMException":
+          name = preview.kind;
+          break;
+        default:
+          throw new Error("Unknown preview kind for the Error rep.");
+      }
+    } else {
+      name = "Error";
+    }
+
+    return name;
+  }
+
   /**
    * Returns a React element reprensenting the Error stacktrace, i.e.
    * transform error.stack from:

devtools/client/shared/components/reps/reps/grip-array.js

@@ -84,6 +84,10 @@ define(function (require, exports, module) {
       );
     }
 
+    if (mode === MODE.HEADER) {
+      return span(config, title);
+    }
+
     const max = maxLengthMap.get(mode);
     const items = arrayIterator(props, object, max);
     brackets = needSpace(!!items.length);
@@ -150,6 +154,10 @@ define(function (require, exports, module) {
       return span({ className: "objectTitle" }, title, length, trailingSpace);
     }
 
+    if (props.mode === MODE.HEADER) {
+      return span({ className: "objectTitle" }, title, length);
+    }
+
     return span({ className: "objectTitle" }, title, length, " ");
   }
 

devtools/client/shared/components/reps/reps/grip-map.js

@@ -51,7 +51,7 @@ define(function (require, exports, module) {
     const title = getTitle(props, object);
     const isEmpty = getLength(object) === 0;
 
-    if (isEmpty || mode === MODE.TINY) {
+    if (isEmpty || mode === MODE.TINY || mode === MODE.HEADER) {
       return span(config, title);
     }
 

devtools/client/shared/components/reps/reps/grip.js

@@ -85,6 +85,11 @@ define(function (require, exports, module) {
       return span(config, ...tinyModeItems);
     }
 
+    if (mode === MODE.HEADER) {
+      config.title = shouldRenderTooltip ? getTitle(props, object) : null;
+      return span(config, getTitleElement(props, object));
+    }
+
     const propsArray = safePropIterator(props, object, maxLengthMap.get(mode));
 
     config.title = shouldRenderTooltip ? getTitle(props, object) : null;

devtools/client/shared/components/reps/reps/promise.js

@@ -59,7 +59,7 @@ define(function (require, exports, module) {
       };
     }
 
-    if (props.mode !== MODE.TINY) {
+    if (props.mode !== MODE.TINY && props.mode !== MODE.HEADER) {
       return Grip.rep(props);
     }
 
@@ -70,6 +70,10 @@ define(function (require, exports, module) {
       title: shouldRenderTooltip ? "Promise" : null,
     };
 
+    if (props.mode === MODE.HEADER) {
+      return span(config, getTitle(object));
+    }
+
     const { Rep } = require("devtools/client/shared/components/reps/reps/rep");
 
     return span(

devtools/client/shared/components/reps/reps/text-node.js

@@ -46,7 +46,7 @@ define(function (require, exports, module) {
     const config = getElementConfig({ ...props, isInTree });
     const inspectIcon = getInspectIcon({ ...props, isInTree });
 
-    if (mode === MODE.TINY) {
+    if (mode === MODE.TINY || mode === MODE.HEADER) {
       return span(config, getTitle(grip), inspectIcon);
     }
 

devtools/client/shared/components/test/node/components/reps/__snapshots__/error.test.js.snap

@@ -710,6 +710,21 @@ exports[`Error - URI error renders with expected text for URIError 1`] = `
 </span>
 `;
 
+exports[`Error - base-loader.sys.mjs renders as expected in HEADER mode 1`] = `
+<span
+  className="objectBox-stackTrace "
+  data-link-actor-id="server1.conn1.child1/obj1020"
+  title={null}
+>
+  <span
+    className="objectTitle"
+    key="title"
+  >
+    Error
+  </span>
+</span>
+`;
+
 exports[`Error - base-loader.sys.mjs renders as expected in tiny mode 1`] = `
 <span
   className="objectBox-stackTrace "

devtools/client/shared/components/test/node/components/reps/comment-node.test.js

@@ -67,6 +67,12 @@ describe("CommentNode", () => {
     );
     expectActorAttribute(component, object.actor);
 
+    component = renderRep({ mode: MODE.HEADER });
+    expect(component.text()).toEqual(
+      "<!-- test\\nand test\\na… test\\nand test -->"
+    );
+    expectActorAttribute(component, object.actor);
+
     component = renderRep({ mode: MODE.LONG });
     expect(component.text()).toEqual(`<!-- ${stub.preview.textContent} -->`);
     expectActorAttribute(component, object.actor);