Bug 1355267 - Polish the properties list and remove the position info at the top; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 24 Apr 2017 18:42:06 +0200
changeset 355996 0cb6f9f404dcda29cfbca9c2414831c9e2f01af6
parent 355995 b1eaeab8bc35aacda9a61d224c5bfb3e6887e8ef
child 355997 21ea46a299d520aa733c71432d2fe0fd4e33d04e
push id31754
push userkwierso@gmail.com
push dateWed, 03 May 2017 00:28:51 +0000
treeherdermozilla-central@5eaf2d70eded [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1355267
milestone55.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 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;