Bug 1503180 - Making the flex item sizing UI more compact and simple; r=gl

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

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js

@@ -20,41 +20,33 @@ class FlexItemSizingProperties extends PureComponent {
   }
 
   /**
-   * 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")
+      )
+    );
   }
 
   /**
@@ -70,14 +62,7 @@ class FlexItemSizingProperties extends PureComponent {
    *         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})`);
   }
 
   /**
@@ -120,12 +105,10 @@ class FlexItemSizingProperties extends PureComponent {
     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
       )
     );
@@ -216,12 +199,10 @@ class FlexItemSizingProperties extends PureComponent {
     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)
       )
     );
@@ -239,12 +220,10 @@ class FlexItemSizingProperties extends PureComponent {
     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)
       )
     );
   }
@@ -259,12 +238,10 @@ class FlexItemSizingProperties extends PureComponent {
     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)
       )
     );
   }
@@ -275,9 +252,7 @@ class FlexItemSizingProperties extends PureComponent {
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.finalSizeSectionHeader")
         ),
-        dom.span({ className: "value theme-fg-color1" },
-          this.getRoundedDimension(mainFinalSize)
-        )
+        this.renderSize(mainFinalSize)
       )
     );
   }

devtools/client/inspector/flexbox/test/browser.ini

@@ -11,6 +11,7 @@ support-files =
   !/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

devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_exists.js

@@ -25,8 +25,9 @@ add_task(async function() {
   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`);
+  });
 });

devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_for_different_writing_modes.js

@@ -20,10 +20,10 @@ async function checkFlexItemDimension(inspector, doc, selector, expectedDimensio
   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() {

devtools/client/inspector/flexbox/test/browser_flexbox_sizing_info_has_correct_sections.js

@@ -35,7 +35,8 @@ add_task(async function() {
 
     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`);
     });
   }
 });

devtools/client/themes/layout.css

@@ -389,30 +389,22 @@
  */
 
 .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 {
-  border: 0;
-  padding-block-start: 0;
-}
-
 .flex-item-sizing .section:first-child {
-  margin: 0;
+  border: 0;
 }
 
 .flex-item-sizing .name {
@@ -425,9 +417,14 @@
   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,