Bug 1490954 - Review Connect page CSS. r=jdescottes,daisuke
authorBelén Albeza <balbeza@mozilla.com>
Wed, 03 Oct 2018 13:53:46 +0000
changeset 495127 93467731a34be3601ec78af984b3d7f4d0af45cd
parent 495126 fde09c00acd82c394072742d1ac36dc829903415
child 495128 a243b88f852a73cc9ef3ca1ac9caa4665900da0e
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes, daisuke
bugs1490954
milestone64.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 1490954 - Review Connect page CSS. r=jdescottes,daisuke - Fixed some values so they match what it's being used in about:preferences (font sizes, margins, etc.) - Fixed page width in Connect page - Refactored some rules to be more generic and not tied up to this specific component - Now a default message appears instead of an empty <ul> when there are no network locations in the list - Adjusted icon & alignment in the Connect sections' headings Differential Revision: https://phabricator.services.mozilla.com/D7103
devtools/client/aboutdebugging-new/aboutdebugging.css
devtools/client/aboutdebugging-new/src/components/App.css
devtools/client/aboutdebugging-new/src/components/App.js
devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.css
devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.js
devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.css
devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.js
devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.css
devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.js
devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.css
devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.js
devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.css
devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.js
devtools/client/aboutdebugging-new/src/components/connect/moz.build
devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.css
devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.js
devtools/client/aboutdebugging-new/tmp-locale/en-US/aboutdebugging.notftl
--- a/devtools/client/aboutdebugging-new/aboutdebugging.css
+++ b/devtools/client/aboutdebugging-new/aboutdebugging.css
@@ -1,33 +1,45 @@
 /* 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/. */
 
 @import "chrome://global/skin/in-content/common.css";
 @import "resource://devtools/client/themes/variables.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/App.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/RuntimeInfo.css";
-@import "resource://devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.css";
-@import "resource://devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/debugtarget/DebugTargetItem.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/debugtarget/DebugTargetList.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/debugtarget/DebugTargetPane.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/debugtarget/ExtensionDetail.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/debugtarget/WorkerDetail.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/sidebar/DeviceSidebarItemAction.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/sidebar/SidebarItem.css";
 
 :root {
   /* Import css variables from common.css */
   --text-color: var(--in-content-page-color);
+
+  /* */
+  /* Variables with values from common.css, which are hardcoded there */
+  /* */
+
+  /* global layout vars */
+  --page-width: 664px;
+  --base-distance: 4px;
+
+  /* global styles */
+  --base-font-size: 15px; /* root font of 11px * 1.36em = 15px */
+  --base-line-height: 1.8;
+  --button-form-width: 150px;
+  --input-hpadding: 5px;
 }
 
 html, body {
   margin: 0;
   padding: 0;
   color: var(--text-color);
 }
 
@@ -43,16 +55,38 @@ ul {
 }
 
 .ellipsis-text {
   overflow: hidden;
   text-overflow: ellipsis;
   white-space: nowrap;
 }
 
+.separator {
+  margin: calc(var(--base-distance) * 4) 0;
+}
+
+.std-button {
+  box-sizing: border-box;
+  margin: 0;
+  min-width: var(--button-form-width);
+}
+
+.std-input,
+/* NOTE: this is here to override the rules in common.css, which have higher
+specificity. Once we stop importing that stylesheet, this extra selector can
+be removed. */
+.std-input[type=text] {
+  box-sizing: border-box;
+  line-height: unset;
+  padding: 0 var(--input-hpadding);
+  height: 100%;
+}
+
+/* TODO: check these values */
 .aboutdebugging-button {
   height: 36px;
   margin-block-start: 0;
   margin-block-end: 0;
   margin-inline-start: 4px;
   margin-inline-end: 4px;
   min-width: 100px;
   padding-inline-start: 20px;
--- a/devtools/client/aboutdebugging-new/src/components/App.css
+++ b/devtools/client/aboutdebugging-new/src/components/App.css
@@ -5,24 +5,86 @@
 /*
  * The current layout of about:debugging is
  *
  *  +-------------+-------------------------------+
  *  |   Sidebar   | Page (Runtime or Connect)     |
  *  |   (240px)   |                               |
  *  |             |                               |
  *  +-------------+-------------------------------+
+ *
+ * Some of the values (font sizes, widths, etc.) are the same as
+ * about:preferences, which uses the shared common.css
  */
+
 .app {
+  /* from common */
+  --sidebar-width: 240px;
+  --app-top-padding: 70px;
+  --app-bottom-padding: 40px;
+  --app-left-padding: 34px;
+
+  box-sizing: border-box;
+  width: 100vw;
+  height: 100vh;
+  overflow: hidden; /* we don't want the sidebar to scroll, only the main content */
+
   display: grid;
-  font-size: 15px;
   grid-column-gap: 40px;
-  grid-template-columns: 240px auto;
-  height: 100vh;
-  overflow: hidden;
-  width: 100vw;
+  grid-template-columns: var(--sidebar-width) auto;
+
+  font-size: var(--base-font-size);
+  line-height: var(--base-line-height);
+}
+
+.app__sidebar {
+  padding-block-start: var(--app-top-padding);
+  padding-block-end: var(--app-bottom-padding);
+  padding-inline-start: var(--app-left-padding);
+}
+
+.app__content {
+  padding-block-start: var(--app-top-padding);
+  padding-block-end: var(--app-bottom-padding);
+
+  /* we want to scroll only the main content, not the sidebar */
+  overflow-y: auto;
 }
 
 .page {
-  padding-block-start: 60px;
-  padding-inline-end: 40px;
-  overflow-y: auto;
+  width: var(--page-width);
+}
+
+.page__title {
+  /* from common */
+  font-weight: 300;
+  font-size: 1.46em;
+  line-height: 1.3;
+
+  margin-block-end: calc(var(--base-distance) * 4);
 }
+
+.page__section {
+  /* from common */
+  margin-block-end: calc(var(--base-distance) * 12);
+
+  --icon-size: calc(var(--base-distance) * 6);
+  --icon-gap: var(--base-distance);
+  --section-inline-margin: calc(var(--icon-size) + var(--icon-gap));
+}
+
+.page__section__title {
+  /* from common */
+  margin-block-start: calc(var(--base-distance) * 4);
+  font-weight: 600;
+  font-size: 1.14em;
+
+  display: grid;
+  grid-template-columns: var(--icon-size) 1fr;
+  grid-column-gap: var(--icon-gap);
+  align-items: center;
+}
+
+.page__section__icon {
+  width: 100%;
+  fill: currentColor;
+  -moz-context-properties: fill;
+}
--- a/devtools/client/aboutdebugging-new/src/components/App.js
+++ b/devtools/client/aboutdebugging-new/src/components/App.js
@@ -56,18 +56,21 @@ class App extends PureComponent {
       runtimes,
       selectedPage,
     } = this.props;
 
     return LocalizationProvider(
       { messages: messageContexts },
       dom.div(
         { className: "app" },
-        Sidebar({ dispatch, runtimes, selectedPage }),
-        this.getSelectedPageComponent()
+        Sidebar({ className: "app__sidebar", dispatch, runtimes, selectedPage }),
+        dom.main(
+          { className: "app__content" },
+          this.getSelectedPageComponent()
+        )
       )
     );
   }
 }
 
 const mapStateToProps = state => {
   return {
     runtimes: state.runtimes,
deleted file mode 100644
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.css
+++ /dev/null
@@ -1,12 +0,0 @@
-/* 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/. */
-
-.connect-page__network {
-  max-width: 600px;
-}
-
-.connect-page__network__separator {
-  margin-block-start: 20px;
-  margin-block-end: 20px;
-}
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.js
+++ b/devtools/client/aboutdebugging-new/src/components/connect/ConnectPage.js
@@ -86,34 +86,34 @@ class ConnectPage extends PureComponent 
       },
       ConnectSection(
         {
           className: "connect-page__network",
           icon: GLOBE_ICON_SRC,
           title: "Via Network Location",
         },
         NetworkLocationsList({ dispatch, networkLocations }),
-        dom.hr({ className: "connect-page__network__separator" }),
+        dom.hr({ className: "separator" }),
         NetworkLocationsForm({ dispatch }),
       )
     );
   }
 
   render() {
     return dom.article(
       {
         className: "page connect-page js-connect-page",
       },
       Localized(
         {
           id: "about-debugging-connect-title"
         },
         dom.h1(
           {
-            className: "connect-page__title"
+            className: "page__title"
           },
           "Connect a Device"
         )
       ),
       this.renderWifi(),
       this.renderUsb(),
       this.renderNetwork(),
     );
deleted file mode 100644
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.css
+++ /dev/null
@@ -1,15 +0,0 @@
-/* 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/. */
-
-.connect-page__section__title {
-  display: flex;
-}
-
-.connect-page__section__icon {
-  fill: currentColor;
-  height: 32px;
-  margin-inline-end: 5px;
-  width: 32px;
-  -moz-context-properties: fill;
-}
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.js
+++ b/devtools/client/aboutdebugging-new/src/components/connect/ConnectSection.js
@@ -16,25 +16,25 @@ class ConnectSection extends PureCompone
       icon: PropTypes.string.isRequired,
       title: PropTypes.string.isRequired,
     };
   }
 
   render() {
     return dom.section(
       {
-        className: this.props.className,
+        className: `page__section ${this.props.className || ""}`,
       },
       dom.h2(
         {
-          className: "connect-page__section__title"
+          className: "page__section__title"
         },
         dom.img(
           {
-            className: "connect-page__section__icon",
+            className: "page__section__icon",
             src: this.props.icon
           }
         ),
         this.props.title
       ),
       this.props.children
     );
   }
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.css
+++ b/devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.css
@@ -1,14 +1,13 @@
 /* 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/. */
 
-.connect-page__steps {
-  line-height: 1.5em;
+.connect-page__step-list {
   list-style-type: decimal;
-  margin-block-end: 20px;
-  margin-inline-start: 40px;
+  list-style-position: outside;
+  margin-inline-start: calc(var(--section-inline-margin) + var(--base-distance) * 4);
 }
 
 .connect-page__step {
-  padding-inline-start: 20px;
+  padding-inline-start: var(--base-distance);
 }
--- a/devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.js
+++ b/devtools/client/aboutdebugging-new/src/components/connect/ConnectSteps.js
@@ -13,17 +13,17 @@ class ConnectSteps extends PureComponent
     return {
       steps: PropTypes.arrayOf(PropTypes.string).isRequired,
     };
   }
 
   render() {
     return dom.ul(
       {
-        className: "connect-page__steps"
+        className: "connect-page__step-list"
       },
       this.props.steps.map(step => dom.li(
         {
           className: "connect-page__step",
           key: step,
         },
         step)
       )
--- a/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.css
+++ b/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.css
@@ -6,16 +6,12 @@
  * Layout of a network location form
  *
  *  +-------------+--------------------+------------+
  *  | "Host:port" | Input              | Add button |
  *  +-------------+--------------------+------------+
  */
 .connect-page__network-form {
   display: grid;
-  grid-column-gap: 10px;
-  grid-template-columns: 100px auto max-content;
-  line-height: 36px;
+  grid-column-gap: calc(var(--base-distance) * 2);
+  grid-template-columns: auto 1fr auto;
+  align-items: center;
 }
-
-.connect-page__network-form__input {
-  box-sizing: border-box;
-}
--- a/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.js
+++ b/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsForm.js
@@ -39,35 +39,41 @@ class NetworkLocationsForm extends PureC
           }
           e.preventDefault();
         }
       },
       Localized(
         {
           id: "about-debugging-network-locations-host-input-label"
         },
-        dom.span({}, "Host")
+        dom.label(
+          {
+            for: "about-debugging-network-locations-host-input",
+          },
+          "Host",
+        )
       ),
       dom.input({
-        className: "connect-page__network-form__input js-network-form-input",
+        id: "about-debugging-network-locations-host-input",
+        className: "std-input js-network-form-input",
         placeholder: "localhost:6080",
         type: "text",
         value: this.state.value,
         onChange: (e) => {
           const value = e.target.value;
           this.setState({ value });
         }
       }),
       Localized(
         {
           id: "about-debugging-network-locations-add-button"
         },
         dom.button(
           {
-            className: "aboutdebugging-button js-network-form-submit-button"
+            className: "std-button js-network-form-submit-button"
           },
           "Add"
         )
       )
     );
   }
 }
 
--- a/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.css
+++ b/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.css
@@ -7,12 +7,10 @@
  *
  *  +-------------------------------------+---------------+
  *  | Location (eg localhost:8080)        | Remove button |
  *  +-------------------------------------+---------------+
  */
 .connect-page__network-location {
   display: grid;
   grid-template-columns: auto max-content;
-  line-height: 36px;
-  margin-block-start: 4px;
-  margin-block-end: 4px;
+  margin: calc(var(--base-distance) * 2) 0;
 }
\ No newline at end of file
--- a/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.js
+++ b/devtools/client/aboutdebugging-new/src/components/connect/NetworkLocationsList.js
@@ -16,17 +16,17 @@ const Actions = require("../../actions/i
 class NetworkLocationsList extends PureComponent {
   static get propTypes() {
     return {
       dispatch: PropTypes.func.isRequired,
       networkLocations: PropTypes.arrayOf(PropTypes.string).isRequired,
     };
   }
 
-  render() {
+  renderList() {
     return dom.ul(
       {},
       this.props.networkLocations.map(location =>
         dom.li(
           {
             className: "connect-page__network-location js-network-location",
             key: location,
           },
@@ -37,23 +37,40 @@ class NetworkLocationsList extends PureC
             location
           ),
           Localized(
             {
               id: "about-debugging-network-locations-remove-button"
             },
             dom.button(
               {
-                className: "aboutdebugging-button js-network-location-remove-button",
+                className: "std-button js-network-location-remove-button",
                 onClick: () => {
                   this.props.dispatch(Actions.removeNetworkLocation(location));
                 }
               },
               "Remove"
             )
           )
         )
       )
     );
   }
+
+  renderEmpty() {
+    return Localized(
+      {
+        id: "about-debugging-network-locations-empty-text"
+      },
+      dom.p(
+        {},
+        "No network locations have been added yet."
+      )
+    );
+  }
+
+  render() {
+    return this.props.networkLocations.length > 0 ?
+      this.renderList() : this.renderEmpty();
+  }
 }
 
 module.exports = NetworkLocationsList;
--- a/devtools/client/aboutdebugging-new/src/components/connect/moz.build
+++ b/devtools/client/aboutdebugging-new/src/components/connect/moz.build
@@ -1,16 +1,14 @@
 # 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/.
 
 DevToolsModules(
-    'ConnectPage.css',
     'ConnectPage.js',
-    'ConnectSection.css',
     'ConnectSection.js',
     'ConnectSteps.css',
     'ConnectSteps.js',
     'NetworkLocationsForm.css',
     'NetworkLocationsForm.js',
     'NetworkLocationsList.css',
     'NetworkLocationsList.js',
 )
--- a/devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.css
+++ b/devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.css
@@ -1,16 +1,11 @@
 /* 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/. */
 
-.sidebar {
-  padding-block-start: 70px;
-  padding-inline-start: 24px;
-}
-
 .sidebar__devices__no-devices-message {
   color: var(--grey-40);
   display: inline-block;
   padding: 12px 0;
   text-align: center;
   width: 100%;
 }
\ No newline at end of file
--- a/devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.js
+++ b/devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.js
@@ -18,16 +18,17 @@ const SidebarItem = createFactory(requir
 const FIREFOX_ICON = "chrome://devtools/skin/images/aboutdebugging-firefox-logo.svg";
 const CONNECT_ICON = "chrome://devtools/skin/images/aboutdebugging-connect-icon.svg";
 const GLOBE_ICON = "chrome://devtools/skin/images/aboutdebugging-globe-icon.svg";
 const USB_ICON = "chrome://devtools/skin/images/aboutdebugging-connect-icon.svg";
 
 class Sidebar extends PureComponent {
   static get propTypes() {
     return {
+      className: PropTypes.string,
       dispatch: PropTypes.func.isRequired,
       runtimes: PropTypes.object.isRequired,
       selectedPage: PropTypes.string,
     };
   }
 
   renderDevices() {
     const { runtimes } = this.props;
@@ -77,17 +78,17 @@ class Sidebar extends PureComponent {
     });
   }
 
   render() {
     const { dispatch, selectedPage } = this.props;
 
     return dom.aside(
       {
-        className: "sidebar",
+        className: `sidebar ${this.props.className || ""}`,
       },
       dom.ul(
         {},
         Localized(
           { id: "about-debugging-sidebar-this-firefox", attrs: { name: true } },
           SidebarItem({
             id: PAGES.THIS_FIREFOX,
             dispatch,
--- a/devtools/client/aboutdebugging-new/tmp-locale/en-US/aboutdebugging.notftl
+++ b/devtools/client/aboutdebugging-new/tmp-locale/en-US/aboutdebugging.notftl
@@ -117,16 +117,19 @@ about-debugging-extension-location = Loc
 # Text displayed for extensions in "runtime" pages, before displaying the extension's ID.
 # For instance "geckoprofiler@mozilla.com" or "{ed26ddcb-5611-4512-a89a-51b8db81cfb2}".
 about-debugging-extension-id = Extension ID
 
 # Text of a button displayed after the network locations "Host" input.
 # Clicking on it will add the new network location to the list.
 about-debugging-network-locations-add-button = Add
 
+# Text to display when there are no locations to show.
+about-debugging-network-locations-empty-text = No network locations have been added yet.
+
 # Text of the label for the text input that allows users to add new network locations in
 # the Connect page. A host is a hostname and a port separated by a colon, as suggested by
 # the input's placeholder "localhost:6080".
 about-debugging-network-locations-host-input-label = Host
 
 # Text of a button displayed next to existing network locations in the Connect page.
 # Clicking on it removes the network location from the list.
 about-debugging-network-locations-remove-button