Bug 1355267 - Polish the properties list and remove the position info at the top; r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 24 Apr 2017 18:42:06 +0200
changeset 571280 6a7e3d65aba99b8c680eae484c6d45cec37045cb
parent 571279 141a6bb737da6c2b559f5ee56496e447560c0bf5
child 626719 ffbcd259713183debbde964e88bf51a0ca9b3dec
push id56739
push userbmo:pbrosset@mozilla.com
push dateTue, 02 May 2017 10:18:15 +0000
reviewersgl
bugs1355267
milestone55.0a1
Bug 1355267 - Polish the properties list and remove the position info at the top; r=gl MozReview-Commit-ID: CoT9fO2w8Tb
devtools/client/inspector/boxmodel/components/BoxModel.js
devtools/client/inspector/boxmodel/components/BoxModelMain.js
devtools/client/inspector/boxmodel/components/BoxModelProperties.js
devtools/client/inspector/boxmodel/components/ComputedProperty.js
devtools/client/inspector/boxmodel/test/browser_boxmodel_offsetparent.js
devtools/client/locales/en-US/boxmodel.properties
devtools/client/themes/boxmodel.css
--- a/devtools/client/inspector/boxmodel/components/BoxModel.js
+++ b/devtools/client/inspector/boxmodel/components/BoxModel.js
@@ -57,30 +57,31 @@ module.exports = createClass({
         ref: div => {
           this.boxModelContainer = div;
         },
         onKeyDown: this.onKeyDown,
       },
       BoxModelMain({
         boxModel,
         boxModelContainer: this.boxModelContainer,
-        setSelectedNode,
         ref: boxModelMain => {
           this.boxModelMain = boxModelMain;
         },
         onHideBoxModelHighlighter,
         onShowBoxModelEditor,
         onShowBoxModelHighlighter,
-        onShowBoxModelHighlighterForNode,
       }),
       BoxModelInfo({
         boxModel,
         onToggleGeometryEditor,
       }),
       showBoxModelProperties ?
         BoxModelProperties({
           boxModel,
+          setSelectedNode,
+          onHideBoxModelHighlighter,
+          onShowBoxModelHighlighterForNode,
         })
         :
         null
     );
   },
 });
--- a/devtools/client/inspector/boxmodel/components/BoxModelMain.js
+++ b/devtools/client/inspector/boxmodel/components/BoxModelMain.js
@@ -8,40 +8,34 @@ const { addons, createClass, createFacto
   require("devtools/client/shared/vendor/react");
 const { findDOMNode } = require("devtools/client/shared/vendor/react-dom");
 const { KeyCodes } = require("devtools/client/shared/keycodes");
 
 const { LocalizationHelper } = require("devtools/shared/l10n");
 
 const BoxModelEditable = createFactory(require("./BoxModelEditable"));
 
-// Reps
-const { REPS, MODE } = require("devtools/client/shared/components/reps/reps");
-const { Rep } = REPS;
-
 const Types = require("../types");
 
 const BOXMODEL_STRINGS_URI = "devtools/client/locales/boxmodel.properties";
 const BOXMODEL_L10N = new LocalizationHelper(BOXMODEL_STRINGS_URI);
 
 const SHARED_STRINGS_URI = "devtools/client/locales/shared.properties";
 const SHARED_L10N = new LocalizationHelper(SHARED_STRINGS_URI);
 
 module.exports = createClass({
 
   displayName: "BoxModelMain",
 
   propTypes: {
     boxModel: PropTypes.shape(Types.boxModel).isRequired,
     boxModelContainer: PropTypes.object,
-    setSelectedNode: PropTypes.func.isRequired,
     onHideBoxModelHighlighter: PropTypes.func.isRequired,
     onShowBoxModelEditor: PropTypes.func.isRequired,
     onShowBoxModelHighlighter: PropTypes.func.isRequired,
-    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
   },
 
   mixins: [ addons.PureRenderMixin ],
 
   getInitialState() {
     return {
       activeDescendant: null,
       focusable: false,
@@ -249,48 +243,16 @@ module.exports = createClass({
       boxModelContainer.setAttribute("activedescendant", nextLayout.className);
     }
 
     this.setState({
       activeDescendant: nextLayout.getAttribute("data-box"),
     });
   },
 
-  /**
-   * While waiting for a reps fix in https://github.com/devtools-html/reps/issues/92,
-   * translate nodeFront to a grip-like object that can be used with an ElementNode rep.
-   *
-   * @param  {NodeFront} nodeFront
-   *         The NodeFront for which we want to create a grip-like object.
-   * @return {Object} a grip-like object that can be used with Reps.
-   */
-  translateNodeFrontToGrip(nodeFront) {
-    let {
-      attributes
-    } = nodeFront;
-
-    // The main difference between NodeFront and grips is that attributes are treated as
-    // a map in grips and as an array in NodeFronts.
-    let attributesMap = {};
-    for (let { name, value } of attributes) {
-      attributesMap[name] = value;
-    }
-
-    return {
-      actor: nodeFront.actorID,
-      preview: {
-        attributes: attributesMap,
-        attributesLength: attributes.length,
-        // nodeName is already lowerCased in Node grips
-        nodeName: nodeFront.nodeName.toLowerCase(),
-        nodeType: nodeFront.nodeType,
-      }
-    };
-  },
-
   onHighlightMouseOver(event) {
     let region = event.target.getAttribute("data-box");
 
     if (!region) {
       let el = event.target;
 
       do {
         el = el.parentNode;
@@ -299,22 +261,16 @@ module.exports = createClass({
           region = el.getAttribute("data-box");
           break;
         }
       } while (el.parentNode);
 
       this.props.onHideBoxModelHighlighter();
     }
 
-    if (region === "offset-parent") {
-      this.props.onHideBoxModelHighlighter();
-      this.props.onShowBoxModelHighlighterForNode(this.props.boxModel.offsetParent);
-      return;
-    }
-
     this.props.onShowBoxModelHighlighter({
       region,
       showOnly: region,
       onlyRegionArea: true,
     });
   },
 
   /**
@@ -404,25 +360,22 @@ module.exports = createClass({
     if (target && target._editable) {
       target.blur();
     }
   },
 
   render() {
     let {
       boxModel,
-      setSelectedNode,
       onShowBoxModelEditor,
     } = this.props;
-    let { layout, offsetParent } = boxModel;
-    let { height, width, position } = layout;
+    let { layout } = boxModel;
+    let { height, width } = layout;
     let { activeDescendant: level, focusable } = this.state;
 
-    let displayOffsetParent = offsetParent && layout.position === "absolute";
-
     let borderTop = this.getBorderOrPaddingValue("border-top-width");
     let borderRight = this.getBorderOrPaddingValue("border-right-width");
     let borderBottom = this.getBorderOrPaddingValue("border-bottom-width");
     let borderLeft = this.getBorderOrPaddingValue("border-left-width");
 
     let paddingTop = this.getBorderOrPaddingValue("padding-top");
     let paddingRight = this.getBorderOrPaddingValue("padding-right");
     let paddingBottom = this.getBorderOrPaddingValue("padding-bottom");
@@ -491,44 +444,16 @@ module.exports = createClass({
         ref: div => {
           this.positionLayout = div;
         },
         onClick: this.onLevelClick,
         onKeyDown: this.onKeyDown,
         onMouseOver: this.onHighlightMouseOver,
         onMouseOut: this.props.onHideBoxModelHighlighter,
       },
-      displayOffsetParent ?
-        dom.span(
-          {
-            className: "boxmodel-offset-parent",
-            "data-box": "offset-parent",
-          },
-          Rep(
-            {
-              defaultRep: offsetParent,
-              mode: MODE.TINY,
-              object: this.translateNodeFrontToGrip(offsetParent),
-              onInspectIconClick: () => setSelectedNode(offsetParent, "box-model"),
-            }
-          )
-        )
-        :
-        null,
-      displayPosition ?
-        dom.span(
-          {
-            className: "boxmodel-legend",
-            "data-box": "position",
-            title: BOXMODEL_L10N.getFormatStr("boxmodel.position", position),
-          },
-          BOXMODEL_L10N.getFormatStr("boxmodel.position", position)
-        )
-        :
-        null,
       dom.div(
         {
           className: "boxmodel-box"
         },
         dom.span(
           {
             className: "boxmodel-legend",
             "data-box": "margin",
--- a/devtools/client/inspector/boxmodel/components/BoxModelProperties.js
+++ b/devtools/client/inspector/boxmodel/components/BoxModelProperties.js
@@ -17,44 +17,87 @@ const BOXMODEL_STRINGS_URI = "devtools/c
 const BOXMODEL_L10N = new LocalizationHelper(BOXMODEL_STRINGS_URI);
 
 module.exports = createClass({
 
   displayName: "BoxModelProperties",
 
   propTypes: {
     boxModel: PropTypes.shape(Types.boxModel).isRequired,
+    setSelectedNode: PropTypes.func.isRequired,
+    onHideBoxModelHighlighter: PropTypes.func.isRequired,
+    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
   },
 
   mixins: [ addons.PureRenderMixin ],
 
   getInitialState() {
     return {
       isOpen: true,
     };
   },
 
+  /**
+   * Various properties can display a reference element. E.g. position displays an offset
+   * parent if its value is other than fixed or static. Or z-index displays a stacking
+   * context, etc.
+   * This returns the right element if there needs to be one, and one was passed in the
+   * props.
+   *
+   * @return {Object} An object with 2 properties:
+   * - referenceElement {NodeFront}
+   * - referenceElementType {String}
+   */
+  getReferenceElement(propertyName) {
+    let value = this.props.boxModel.layout[propertyName];
+
+    if (propertyName === "position" &&
+        value !== "static" && value !== "fixed" &&
+        this.props.boxModel.offsetParent) {
+      return {
+        referenceElement: this.props.boxModel.offsetParent,
+        referenceElementType: BOXMODEL_L10N.getStr("boxmodel.offsetParent")
+      };
+    }
+
+    return {};
+  },
+
   onToggleExpander() {
     this.setState({
       isOpen: !this.state.isOpen,
     });
   },
 
   render() {
-    let { boxModel } = this.props;
+    let {
+      boxModel,
+      setSelectedNode,
+      onHideBoxModelHighlighter,
+      onShowBoxModelHighlighterForNode,
+    } = this.props;
     let { layout } = boxModel;
 
     let layoutInfo = ["box-sizing", "display", "float",
                       "line-height", "position", "z-index"];
 
-    const properties = layoutInfo.map(info => ComputedProperty({
-      name: info,
-      key: info,
-      value: layout[info],
-    }));
+    const properties = layoutInfo.map(info => {
+      let { referenceElement, referenceElementType } = this.getReferenceElement(info);
+
+      return ComputedProperty({
+        name: info,
+        key: info,
+        value: layout[info],
+        referenceElement,
+        referenceElementType,
+        setSelectedNode,
+        onHideBoxModelHighlighter,
+        onShowBoxModelHighlighterForNode,
+      });
+    });
 
     return dom.div(
       {
         className: "boxmodel-properties",
       },
       dom.div(
         {
           className: "boxmodel-properties-header",
--- a/devtools/client/inspector/boxmodel/components/ComputedProperty.js
+++ b/devtools/client/inspector/boxmodel/components/ComputedProperty.js
@@ -1,32 +1,101 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { addons, createClass, DOM: dom, PropTypes } =
   require("devtools/client/shared/vendor/react");
+const { REPS, MODE } = require("devtools/client/shared/components/reps/reps");
+const { Rep } = REPS;
 
 module.exports = createClass({
 
   displayName: "ComputedProperty",
 
   propTypes: {
     name: PropTypes.string.isRequired,
     value: PropTypes.string,
+    referenceElement: PropTypes.object,
+    referenceElementType: PropTypes.string,
+    setSelectedNode: PropTypes.func.isRequired,
+    onHideBoxModelHighlighter: PropTypes.func.isRequired,
+    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
   },
 
   mixins: [ addons.PureRenderMixin ],
 
+  /**
+   * While waiting for a reps fix in https://github.com/devtools-html/reps/issues/92,
+   * translate nodeFront to a grip-like object that can be used with an ElementNode rep.
+   *
+   * @param  {NodeFront} nodeFront
+   *         The NodeFront for which we want to create a grip-like object.
+   * @return {Object} a grip-like object that can be used with Reps.
+   */
+  translateNodeFrontToGrip(nodeFront) {
+    let {
+      attributes
+    } = nodeFront;
+
+    // The main difference between NodeFront and grips is that attributes are treated as
+    // a map in grips and as an array in NodeFronts.
+    let attributesMap = {};
+    for (let { name, value } of attributes) {
+      attributesMap[name] = value;
+    }
+
+    return {
+      actor: nodeFront.actorID,
+      preview: {
+        attributes: attributesMap,
+        attributesLength: attributes.length,
+        // nodeName is already lowerCased in Node grips
+        nodeName: nodeFront.nodeName.toLowerCase(),
+        nodeType: nodeFront.nodeType,
+        isConnected: true,
+      }
+    };
+  },
+
   onFocus() {
     this.container.focus();
   },
 
+  renderReferenceElementPreview() {
+    let {
+      referenceElement,
+      referenceElementType,
+      setSelectedNode,
+      onShowBoxModelHighlighterForNode,
+      onHideBoxModelHighlighter
+    } = this.props;
+
+    if (!referenceElement) {
+      return null;
+    }
+
+    return dom.div(
+      {
+        className: "reference-element"
+      },
+      dom.span({ className: "reference-element-type" }, referenceElementType),
+      Rep({
+        defaultRep: referenceElement,
+        mode: MODE.TINY,
+        object: this.translateNodeFrontToGrip(referenceElement),
+        onInspectIconClick: () => setSelectedNode(referenceElement, "box-model"),
+        onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(referenceElement),
+        onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
+      })
+    );
+  },
+
   render() {
     const { name, value } = this.props;
 
     return dom.div(
       {
         className: "property-view",
         "data-property-name": name,
         tabIndex: "0",
@@ -55,14 +124,15 @@ module.exports = createClass({
         dom.div(
           {
             className: "property-value theme-fg-color1",
             dir: "ltr",
             tabIndex: "",
             onClick: this.onFocus,
           },
           value
-        )
+        ),
+        this.renderReferenceElementPreview()
       )
     );
   },
 
 });
--- a/devtools/client/inspector/boxmodel/test/browser_boxmodel_offsetparent.js
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_offsetparent.js
@@ -11,30 +11,30 @@ const TEST_URI = `
   <div id="relative_parent" style="position: relative">
     <div id="absolute_child" style="position: absolute"></div>
   </div>
   <div id="static"></div>
   <div id="no_parent" style="position: absolute"></div>
   <div id="fixed" style="position: fixed"></div>
 `;
 
-const OFFSET_PARENT_SELECTOR = ".boxmodel-offset-parent .objectBox-node";
+const OFFSET_PARENT_SELECTOR = ".property-value-container .objectBox-node";
 
 const res1 = [
   {
     selector: "#absolute_child",
     offsetParentValue: "div#relative_parent"
   },
   {
     selector: "#no_parent",
     offsetParentValue: "body"
   },
   {
     selector: "#relative_parent",
-    offsetParentValue: null
+    offsetParentValue: "body"
   },
   {
     selector: "#static",
     offsetParentValue: null
   },
   {
     selector: "#fixed",
     offsetParentValue: null
@@ -52,37 +52,37 @@ const res2 = [
   {
     selector: "#absolute_child",
     offsetParentValue: null
   },
 ];
 
 add_task(function* () {
   yield addTab("data:text/html," + encodeURIComponent(TEST_URI));
-  let { inspector, view, testActor } = yield openBoxModelView();
+  let { inspector, boxmodel, testActor } = yield openLayoutView();
 
-  yield testInitialValues(inspector, view);
-  yield testChangingValues(inspector, view, testActor);
+  yield testInitialValues(inspector, boxmodel);
+  yield testChangingValues(inspector, boxmodel, testActor);
 });
 
-function* testInitialValues(inspector, view) {
+function* testInitialValues(inspector, boxmodel) {
   info("Test that the initial values of the box model offset parent are correct");
-  let viewdoc = view.document;
+  let viewdoc = boxmodel.document;
 
   for (let { selector, offsetParentValue } of res1) {
     yield selectNode(selector, inspector);
 
     let elt = viewdoc.querySelector(OFFSET_PARENT_SELECTOR);
     is(elt && elt.textContent, offsetParentValue, selector + " has the right value.");
   }
 }
 
-function* testChangingValues(inspector, view, testActor) {
+function* testChangingValues(inspector, boxmodel, testActor) {
   info("Test that changing the document updates the box model");
-  let viewdoc = view.document;
+  let viewdoc = boxmodel.document;
 
   for (let { selector, update } of updates) {
     let onUpdated = waitForUpdate(inspector);
     yield testActor.setAttribute(selector, "style", update);
     yield onUpdated;
   }
 
   for (let { selector, offsetParentValue } of res2) {
--- a/devtools/client/locales/en-US/boxmodel.properties
+++ b/devtools/client/locales/en-US/boxmodel.properties
@@ -38,8 +38,14 @@ boxmodel.content=content
 # LOCALIZATION NOTE: (boxmodel.geometryButton.tooltip) This label is displayed as a
 # tooltip that appears when hovering over the button that allows users to edit the
 # position of an element in the page.
 boxmodel.geometryButton.tooltip=Edit position
 
 # LOCALIZATION NOTE: (boxmodel.propertiesLabel) This label is displayed as the header
 # for showing and collapsing the properties underneath the box model in the layout view
 boxmodel.propertiesLabel=Box Model Properties
+
+# LOCALIZATION NOTE: (boxmodel.offsetParent) This label is displayed inside the list of
+# properties, below the box model, in the layout view. It is displayed next to the
+# position property, when position is absolute, relative, sticky. This label tells users
+# what the DOM node previewed next to it is: an offset parent for the position element.
+boxmodel.offsetParent=offset
--- a/devtools/client/themes/boxmodel.css
+++ b/devtools/client/themes/boxmodel.css
@@ -4,16 +4,18 @@
 
 /**
  * This is the stylesheet of the Box Model view implemented in the layout panel.
  */
 
 .boxmodel-container {
   overflow: auto;
   padding-bottom: 4px;
+  max-width: 600px;
+  margin: 0 auto;
 }
 
 /* Header */
 
 .boxmodel-header,
 .boxmodel-info {
   display: flex;
   align-items: center;
@@ -319,26 +321,54 @@
   padding: 2px 3px;
 }
 
 .boxmodel-properties-expander {
   vertical-align: middle;
   display: inline-block;
 }
 
+.boxmodel-properties-wrapper {
+  column-width: 250px;
+  column-gap: 20px;
+  column-rule: 1px solid var(--theme-splitter-color);
+}
+
 .boxmodel-properties-wrapper .property-view {
   padding-inline-start: 17px;
 }
 
 .boxmodel-properties-wrapper .property-name-container {
   flex: 1;
 }
 
 .boxmodel-properties-wrapper .property-value-container {
   flex: 1;
+  display: block;
+}
+
+.boxmodel-container .reference-element {
+  margin-inline-start: 14px;
+  margin-block-start: 4px;
+  display: block;
+}
+
+/* Tag displayed next to DOM Node previews (used to display reference elements) */
+
+.boxmodel-container .reference-element-type {
+  background: var(--theme-highlight-purple);
+  color: white;
+  padding: 1px 2px;
+  border-radius: 2px;
+  font-size: 9px;
+  margin-inline-end: 5px;
+}
+
+.theme-dark .boxmodel-container .reference-element-type {
+  color: black;
 }
 
 /* Box Model Main - Offset Parent */
 
 .boxmodel-offset-parent {
   position: absolute;
   top: -20px;
   right: -10px;