Bug 1503180 - Making the flex item sizing UI more compact and simple; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 06 Nov 2018 14:58:36 +0000
changeset 444674 63975abdead1d3520fb44711f43496d2ffd11c56
parent 444673 1fafb01078dbb3602bcdac29d40f91f67f71fba3
child 444675 2380a1a98a0e96557eb1713fe7d42abba9e7ccb6
push id35001
push userncsoregi@mozilla.com
push dateWed, 07 Nov 2018 09:52:11 +0000
treeherdermozilla-central@bc83ec5a338d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1503180
milestone65.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 1503180 - Making the flex item sizing UI more compact and simple; r=gl Differential Revision: https://phabricator.services.mozilla.com/D10371
devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
devtools/client/inspector/flexbox/test/browser.ini
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_exists.js
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_different_writing_modes.js
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_has_correct_sections.js
devtools/client/themes/layout.css
--- a/devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
+++ b/devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
@@ -15,74 +15,59 @@ class FlexItemSizingProperties extends P
   static get propTypes() {
     return {
       flexDirection: PropTypes.string.isRequired,
       flexItem: PropTypes.shape(Types.flexItem).isRequired,
     };
   }
 
   /**
-   * Rounds some dimension in pixels and returns a string to be displayed to the user.
-   * The string will end with 'px'. If the number is 0, the string "0" is returned.
+   * Rounds some size in pixels and render it.
+   * The rendered value will end with 'px' (unless the dimension is 0 in which case the
+   * unit will be omitted)
    *
    * @param  {Number} value
    *         The number to be rounded
-   * @return {String}
-   *         Representation of the rounded number
+   * @param  {Boolean} prependPlusSign
+   *         If set to true, the + sign will be printed before a positive value
+   * @return {Object}
+   *         The React component representing this rounded size
    */
-  getRoundedDimension(value) {
+  renderSize(value, prependPlusSign) {
     if (value == 0) {
-      return "0";
-    }
-    return (Math.round(value * 100) / 100) + "px";
-  }
-
-  /**
-   * Format the flexibility value into a meaningful value for the UI.
-   * If the item grew, then prepend a + sign, if it shrank, prepend a - sign.
-   * If it didn't flex, return "0".
-   *
-   * @param  {Boolean} grew
-   *         Whether the item grew or not
-   * @param  {Number} value
-   *         The amount of pixels the item flexed
-   * @return {String}
-   *         Representation of the flexibility value
-   */
-  getFlexibilityValueString(grew, mainDeltaSize) {
-    const value = this.getRoundedDimension(mainDeltaSize);
-
-    if (grew) {
-      return "+" + value;
+      return dom.span({ className: "value" }, "0");
     }
 
-    return value;
+    value = (Math.round(value * 100) / 100);
+    if (prependPlusSign && value > 0) {
+      value = "+" + value;
+    }
+
+    return (
+      dom.span({ className: "value" },
+        value,
+        dom.span({ className: "unit" }, "px")
+      )
+    );
   }
 
   /**
    * Render an authored CSS property.
    *
    * @param  {String} name
    *         The name for this CSS property
    * @param  {String} value
    *         The property value
    * @param  {Booleam} isDefaultValue
    *         Whether the value come from the browser default style
    * @return {Object}
    *         The React component representing this CSS property
    */
   renderCssProperty(name, value, isDefaultValue) {
-    return (
-      dom.span({ className: "css-property-link" },
-        dom.span({ className: "theme-fg-color5" }, name),
-        ": ",
-        dom.span({ className: "theme-fg-color1" }, value),
-        ";"
-      )
-    );
+    return dom.span({ className: "css-property-link" }, `(${name}: ${value})`);
   }
 
   /**
    * Render a list of sentences to be displayed in the UI as reasons why a certain sizing
    * value happened.
    *
    * @param  {Array} sentences
    *         The list of sentences as Strings
@@ -115,22 +100,20 @@ class FlexItemSizingProperties extends P
       reason = this.renderReasons(
         [getStr("flexbox.itemSizing.itemBaseSizeFromContent")]);
     }
 
     const className = "section base";
     return (
       dom.li({ className: className + (property ? "" : " no-property") },
         dom.span({ className: "name" },
-          getStr("flexbox.itemSizing.baseSizeSectionHeader")
+          getStr("flexbox.itemSizing.baseSizeSectionHeader"),
+          property
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getRoundedDimension(mainBaseSize)
-        ),
-        property,
+        this.renderSize(mainBaseSize),
         reason
       )
     );
   }
 
   renderFlexibilitySection(flexItemSizing, properties, computedStyle) {
     const {
       mainDeltaSize,
@@ -211,22 +194,20 @@ class FlexItemSizingProperties extends P
     if (!property && !reasons.length) {
       return null;
     }
 
     const className = "section flexibility";
     return (
       dom.li({ className: className + (property ? "" : " no-property") },
         dom.span({ className: "name" },
-          getStr("flexbox.itemSizing.flexibilitySectionHeader")
+          getStr("flexbox.itemSizing.flexibilitySectionHeader"),
+          property
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getFlexibilityValueString(grew, mainDeltaSize)
-        ),
-        property,
+        this.renderSize(mainDeltaSize, true),
         this.renderReasons(reasons)
       )
     );
   }
 
   renderMinimumSizeSection({ clampState, mainMinSize }, properties, dimension) {
     // We only display the minimum size when the item actually violates that size during
     // layout & is clamped.
@@ -234,55 +215,49 @@ class FlexItemSizingProperties extends P
       return null;
     }
 
     const minDimensionValue = properties[`min-${dimension}`];
 
     return (
       dom.li({ className: "section min" },
         dom.span({ className: "name" },
-          getStr("flexbox.itemSizing.minSizeSectionHeader")
+          getStr("flexbox.itemSizing.minSizeSectionHeader"),
+          this.renderCssProperty(`min-${dimension}`, minDimensionValue)
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getRoundedDimension(mainMinSize)
-        ),
-        this.renderCssProperty(`min-${dimension}`, minDimensionValue)
+        this.renderSize(mainMinSize)
       )
     );
   }
 
   renderMaximumSizeSection({ clampState, mainMaxSize }, properties, dimension) {
     if (clampState !== "clamped_to_max") {
       return null;
     }
 
     const maxDimensionValue = properties[`max-${dimension}`];
 
     return (
       dom.li({ className: "section max" },
         dom.span({ className: "name" },
-          getStr("flexbox.itemSizing.maxSizeSectionHeader")
+          getStr("flexbox.itemSizing.maxSizeSectionHeader"),
+          this.renderCssProperty(`max-${dimension}`, maxDimensionValue)
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getRoundedDimension(mainMaxSize)
-        ),
-        this.renderCssProperty(`max-${dimension}`, maxDimensionValue)
+        this.renderSize(mainMaxSize)
       )
     );
   }
 
   renderFinalSizeSection({ mainFinalSize }) {
     return (
       dom.li({ className: "section final no-property" },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.finalSizeSectionHeader")
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getRoundedDimension(mainFinalSize)
-        )
+        this.renderSize(mainFinalSize)
       )
     );
   }
 
   render() {
     const {
       flexItem,
     } = this.props;
--- a/devtools/client/inspector/flexbox/test/browser.ini
+++ b/devtools/client/inspector/flexbox/test/browser.ini
@@ -6,16 +6,17 @@ support-files =
   doc_flexbox_simple.html
   doc_flexbox_specific_cases.html
   doc_flexbox_text_nodes.html
   doc_flexbox_writing_modes.html
   head.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/shared/test/shared-head.js
+  !/devtools/client/shared/test/shared-redux-head.js
   !/devtools/client/shared/test/telemetry-test-helpers.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_flexbox_highlighter_color_picker_on_ESC.js]
 [browser_flexbox_highlighter_color_picker_on_RETURN.js]
 [browser_flexbox_item_outline_exists.js]
 [browser_flexbox_item_outline_has_correct_layout.js]
--- a/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_exists.js
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_exists.js
@@ -20,13 +20,14 @@ add_task(async function() {
   await selectNode(".container.growing .item", inspector);
   const [flexSizingContainer] = await onFlexItemSizingRendered;
 
   ok(flexSizingContainer, "The flex sizing exists in the DOM");
 
   info("Check that the base, flexibility and final sizes are displayed");
   const allSections = [...flexSizingContainer.querySelectorAll(".section .name")];
   const allSectionTitles = allSections.map(el => el.textContent);
-  const expectedTitles = ["Base Size", "Flexibility", "Final Size"];
 
-  ok(expectedTitles.every(title => allSectionTitles.includes(title)),
-     "The 3 main sizing sections where found");
+  ["Base Size", "Flexibility", "Final Size"].forEach((expectedTitle, i) => {
+    ok(allSectionTitles[i].includes(expectedTitle),
+       `Sizing section #${i + 1} (${expectedTitle}) was found`);
+  });
 });
--- a/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_different_writing_modes.js
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_different_writing_modes.js
@@ -15,20 +15,20 @@ async function checkFlexItemDimension(in
   info("Select the container's flex item.");
   const onFlexItemSizingRendered = waitForDOM(doc, "ul.flex-item-sizing");
   await selectNode(selector, inspector);
   const [flexItemSizingContainer] = await onFlexItemSizingRendered;
 
   info("Check that the minimum size section shows the correct dimension.");
   const [sectionMinRowItem] = [...flexItemSizingContainer.querySelectorAll(
     ".section.min")];
-  const minDimension = sectionMinRowItem.querySelector(".theme-fg-color5");
+  const minDimension = sectionMinRowItem.querySelector(".css-property-link");
 
-  is(minDimension.textContent, expectedDimension,
-    "The flex item sizing has the correct dimension value.");
+  ok(minDimension.textContent.includes(expectedDimension),
+     "The flex item sizing has the correct dimension value.");
 }
 
 add_task(async function() {
   await addTab(TEST_URI);
   const { inspector, flexboxInspector } = await openLayoutView();
   const { document: doc } = flexboxInspector;
 
   await checkFlexItemDimension(inspector, doc, ".row.vertical.item", "min-height");
--- a/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_has_correct_sections.js
+++ b/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_has_correct_sections.js
@@ -30,17 +30,18 @@ add_task(async function() {
   const { document: doc } = flexboxInspector;
 
   for (const { selector, expectedSections } of TEST_DATA) {
     info(`Checking the list of sections for the flex item ${selector}`);
     const sections = await selectNodeAndGetFlexSizingSections(selector, inspector, doc);
 
     is(sections.length, expectedSections.length, "Correct number of sections found");
     expectedSections.forEach((expectedSection, i) => {
-      is(sections[i], expectedSection, `The ${expectedSection} section was found`);
+      ok(sections[i].includes(expectedSection),
+         `The ${expectedSection} section was found`);
     });
   }
 });
 
 async function selectNodeAndGetFlexSizingSections(selector, inspector, doc) {
   const onFlexItemSizingRendered = waitForDOM(doc, "ul.flex-item-sizing");
   await selectNode(selector, inspector);
   const [flexSizingContainer] = await onFlexItemSizingRendered;
--- a/devtools/client/themes/layout.css
+++ b/devtools/client/themes/layout.css
@@ -384,55 +384,52 @@
   border-width: 0 1px 0 0;
 }
 
 /**
  * Flex Item Sizing Properties
  */
 
 .flex-item-sizing {
-  margin: 20px;
+  margin: 0;
   padding: 0;
   list-style: none;
 }
 
 .flex-item-sizing .section {
   --padding: 10px;
-  margin-block-start: var(--padding);
-  padding: var(--padding) 0 0 0;
+  padding: var(--padding);
   border-block-start: 1px solid var(--theme-splitter-color);
   display: grid;
   grid-template-columns: 1fr max-content;
   grid-column-gap: var(--padding);
 }
 
-/* If the outline isn't displayed before the sizing information, then no need for a first
-   top border or padding */
-:not(.flex-outline-container) + .flex-item-sizing > .section:first-child {
+.flex-item-sizing .section:first-child {
   border: 0;
-  padding-block-start: 0;
-}
-
-.flex-item-sizing .section:first-child {
-  margin: 0;
 }
 
 .flex-item-sizing .name {
   font-weight: 600;
   grid-column: 1;
 }
 
 .flex-item-sizing .value {
   text-align: end;
   font-weight: 600;
 }
 
+.flex-item-sizing .value .unit {
+  color: var(--theme-comment);
+  font-weight: normal;
+}
+
 .flex-item-sizing .css-property-link {
-  grid-column: 2;
-  text-align: end;
+  font-weight: normal;
+  margin-inline-start: .5em;
 }
 
 .flex-item-sizing .reasons,
 .flex-item-sizing .reasons li {
   grid-column: 1 / 3;
   margin: 0;
   padding: 0;
   list-style: none;