Bug 1459581 - CSS and class names refactor, and fix UI glitch when sw scope is too long. r=jdescottes draft
authorBelén Albeza <balbeza@mozilla.com>
Tue, 08 May 2018 17:24:52 +0200
changeset 796742 68f0a2df08f8c1c1c958f17096bf12fbca8f6f62
parent 796662 54063deb2f1caf8c4acf6461d3ba779805835c96
push id110356
push userbalbeza@mozilla.com
push dateFri, 18 May 2018 08:57:10 +0000
reviewersjdescottes
bugs1459581
milestone62.0a1
Bug 1459581 - CSS and class names refactor, and fix UI glitch when sw scope is too long. r=jdescottes This was a good time to revamp of the CSS layout we were using in the Worker component. Changes that have been made: - Use CSS Grid to layout the Worker component - Rename classes to use BEM style (this is compatible with the current CSS guidelines for the DevTools). - Use classes with the js- prefix for JS hooks - Rename classes / use js- class names for hooks in other components, so everything is consistent. - Some HTML markup has been fixed, where it wasn't in conflict with the current styling. MozReview-Commit-ID: Doyd9mBLOMd
devtools/client/application/src/components/App.css
devtools/client/application/src/components/App.js
devtools/client/application/src/components/Worker.css
devtools/client/application/src/components/Worker.js
devtools/client/application/src/components/WorkerList.css
devtools/client/application/src/components/WorkerList.js
devtools/client/application/src/components/WorkerListEmpty.css
devtools/client/application/src/components/WorkerListEmpty.js
devtools/client/application/test/browser_application_panel_list-domain-workers.js
devtools/client/application/test/browser_application_panel_list-several-workers.js
devtools/client/application/test/browser_application_panel_list-single-worker.js
devtools/client/application/test/browser_application_panel_list-unicode.js
devtools/client/application/test/head.js
--- a/devtools/client/application/src/components/App.css
+++ b/devtools/client/application/src/components/App.css
@@ -1,48 +1,60 @@
 /* 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/. */
 
 /*
+ * General styles
+ */
+
+:root[platform="linux"],
+:root[platform="linux"] button {
+  font-size: 11px;
+}
+
+h1 {
+  font-size: 22px;
+  font-weight: normal;
+}
+
+a {
+  margin: 0 10px;
+}
+
+a,
+a:hover,
+a:visited {
+  color: var(--blue-60) !important;
+  cursor: pointer;
+}
+
+a.disabled,
+a.disabled:hover,
+a.disabled:visited {
+  color: var(--grey-30) !important;
+  cursor: default;
+}
+
+/*
  * The current layout of the application panel is
  *
  *  +---------------------------------------------+
  *  | (header) "Service workers"                  |
  *  +---------------------------------------------+
  *  | Service worker 1                            |
  *  |   (...)                                     |
  *  | Service worker N           (see Worker.css) |
  *  +---------------------------------------------+
  *  |                     Link to about:debugging |
  *  +---------------------------------------------+
  */
 .application {
   height: 100%;
-  padding: 0 0 0 20px;
+  padding: 0 20px;
   overflow: auto;
   display: flex;
   flex-direction: column;
 }
 
-.application.empty {
+.application--empty {
   background-color: var(--grey-30);
 }
-
-h1 {
-  font-size: 22px;
-  font-weight: normal;
-}
-
-a,
-a:hover,
-a:visited {
-  color: var(--blue-60) !important;
-  margin: 0 10px;
-  cursor: pointer;
-}
-
-a.disabled,
-a.disabled:hover,
-a.disabled:visited {
-  color: var(--grey-30) !important;
-  cursor: default;
-}
--- a/devtools/client/application/src/components/App.js
+++ b/devtools/client/application/src/components/App.js
@@ -2,17 +2,17 @@
  * 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 PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { createFactory, Component } = require("devtools/client/shared/vendor/react");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
-const { div } = require("devtools/client/shared/vendor/react-dom-factories");
+const { main } = require("devtools/client/shared/vendor/react-dom-factories");
 
 const WorkerList = createFactory(require("./WorkerList"));
 const WorkerListEmpty = createFactory(require("./WorkerListEmpty"));
 
 /**
  * This is the main component for the application panel.
  */
 class App extends Component {
@@ -28,18 +28,18 @@ class App extends Component {
   render() {
     let { workers, domain, client, serviceContainer } = this.props;
 
     // Filter out workers from other domains
     workers = workers.filter((x) => new URL(x.url).hostname === domain);
     const isEmpty = workers.length === 0;
 
     return (
-      div(
-        { className: `application ${isEmpty ? "empty" : ""}` },
+      main(
+        { className: `application ${isEmpty ? "application--empty" : ""}` },
         isEmpty
           ? WorkerListEmpty({ serviceContainer })
           : WorkerList({ workers, client, serviceContainer })
       )
     );
   }
 }
 
--- a/devtools/client/application/src/components/Worker.css
+++ b/devtools/client/application/src/components/Worker.css
@@ -1,47 +1,70 @@
 /* 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/. */
 
  /*
  * The current layout of a service worker item is
  *
  *  +----------------------------+----------------+
- *  | Service worker scope       | Unregister btn |
+ *  | Service worker scope       | Unregister_btn |
  *  +---+----------+-------------+----------------|
- *  |   | "Source" | script name | debug link     |
- *  |   |----------+-------------+----------------|
- *  |   | "Status" | status      | start link     |
+ *  |     "Source" | script_name debug_link       |
+ *  |--------------+-------------+----------------|
+ *  |     "Status" | status start_link            |
  *  +---+----------+-------------+----------------|
  */
-.service-worker-container {
-  margin-bottom: 20px;
+
+.worker {
+  display: grid;
+  grid-template-rows: auto auto auto;
+  grid-template-columns: auto 1fr;
   width: 100%;
-  max-width: 600px;
-  position: relative;
+  grid-column-gap: 0;
+  padding: 1rem 0;
+
   line-height: 1.5;
-  font-size: 13px;
+  font-size: 1.2rem;
 }
 
-.service-worker-container > .service-worker-scope {
-  padding-inline-start: 30px;
+.worker:first-child {
+  padding-top: 0;
+}
+
+.worker:not(:last-child) {
+  border-bottom: 1px solid var(--grey-30);
 }
 
-.service-worker-container > :not(.service-worker-scope) {
-  padding-inline-start: 70px;
+.worker__header {
+  grid-column: 1/3;
+  display: grid;
+  grid-template-columns: 1fr auto;
+  grid-column-gap: 2rem;
+  align-items: center;
 }
 
-.service-worker-scope {
+.worker__scope {
   font-weight: bold;
+  text-overflow: ellipsis;
+  overflow: hidden;
+  white-space: nowrap;
 }
 
-.service-worker-meta-name {
-  color: var(--grey-50);
-  min-width: 50px;
-  margin-inline-end: 10px;
-  display: inline-block;
+.worker__data {
+  display: grid;
+  grid-template-columns: auto 1fr;
+  grid-column-gap: 1rem;
+}
+
+.worker__data > * {
+  margin: 0;
 }
 
-.unregister-button {
-  position: absolute;
-  right: 0;
+.worker__meta-name {
+  color: var(--grey-50);
+  padding-inline-start: 4.5rem;
 }
+
+.worker__unregister-button {
+  /* TODO: remove this once/if we have proper capitalization in the strings file */
+  text-transform: capitalize;
+}
--- a/devtools/client/application/src/components/Worker.js
+++ b/devtools/client/application/src/components/Worker.js
@@ -1,17 +1,17 @@
 /* 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 { Component } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const { a, button, div, li, span } = require("devtools/client/shared/vendor/react-dom-factories");
+const { a, button, dd, dl, dt, header, li, section, span } = require("devtools/client/shared/vendor/react-dom-factories");
 const Services = require("Services");
 const { getUnicodeUrl, getUnicodeUrlPath } = require("devtools/client/shared/unicode-url");
 
 loader.lazyRequireGetter(this, "DebuggerClient",
   "devtools/shared/client/debugger-client", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser",
   "devtools/client/framework/devtools-browser", true);
 
@@ -112,48 +112,53 @@ class Worker extends Component {
 
   render() {
     let { worker } = this.props;
     let status = this.getServiceWorkerStatus();
 
     const unregisterButton = this.isActive() ?
       button({
         onClick: this.unregister,
-        className: "devtools-button unregister-button",
+        className: "devtools-button worker__unregister-button js-unregister-button",
         "data-standalone": true
       },
         Strings.GetStringFromName("unregister"))
       : null;
 
-    const debugLinkDisabled = this.isRunning() ? "" : "disabled";
+    const debugLinkDisabled = this.isRunning() ? "" : "worker__debug-link--disabled";
     const debugLink = a({
       onClick: this.isRunning() ? this.debug : null,
       title: this.isRunning() ? null : "Only running service workers can be debugged",
-      className: `${debugLinkDisabled} debug-link`
+      className: `${debugLinkDisabled} worker__debug-link`
     },
       Strings.GetStringFromName("debug"));
 
     const startLink = !this.isRunning() ?
-      a({ onClick: this.start, className: "start-link" },
+      a({ onClick: this.start, className: "worker__start-link" },
         Strings.GetStringFromName("start"))
       : null;
 
-    return li({ className: "service-worker-container" },
-      div(
-        { className: "service-worker-scope" },
-        span({ title: worker.scope }, this.formatScope(worker.scope)),
-        unregisterButton),
-      div(
-        { className: "service-worker-source" },
-        span({ className: "service-worker-meta-name" }, "Source"),
-        span({ className: "js-source-url", title: worker.scope },
-          this.formatSource(worker.url)),
-        debugLink),
-      div(
-        { className: `service-worker-status service-worker-status-${status}` },
-        span({ className: "service-worker-meta-name" }, "Status"),
-        Strings.GetStringFromName(status).toLowerCase(),
-        startLink)
+    return li({ className: "worker js-sw-container" },
+      header(
+        { className: "worker__header" },
+        span({ title: worker.scope, className: "worker__scope js-sw-scope" },
+          this.formatScope(worker.scope)),
+        section(
+          { className: "worker__controls" },
+          unregisterButton),
+      ),
+      dl(
+        { className: "worker__data" },
+        dt({ className: "worker__meta-name" }, "Source"),
+        dd({},
+            span({ title: worker.scope, className: "js-source-url" },
+              this.formatSource(worker.url)),
+            debugLink),
+        dt({ className: "worker__meta-name" }, "Status"),
+        dd({},
+          Strings.GetStringFromName(status).toLowerCase(),
+          startLink)
+      )
     );
   }
 }
 
 module.exports = Worker;
--- a/devtools/client/application/src/components/WorkerList.css
+++ b/devtools/client/application/src/components/WorkerList.css
@@ -1,12 +1,16 @@
 /* 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/. */
 
-.application-aboutdebugging-plug {
+.aboutdebugging-plug {
   text-align: right;
-  padding: 5px 0;
+  padding: 1rem 0;
 }
 
-.application-workers-container {
+.aboutdebugging-plug__link {
+  margin-right: 0;
+}
+
+.workers-container {
   flex-grow: 1;
 }
--- a/devtools/client/application/src/components/WorkerList.js
+++ b/devtools/client/application/src/components/WorkerList.js
@@ -1,17 +1,17 @@
 /* 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 PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { createFactory, Component } = require("devtools/client/shared/vendor/react");
-const { a, div, h1, ul, li } = require("devtools/client/shared/vendor/react-dom-factories");
+const { a, article, footer, h1, ul } = require("devtools/client/shared/vendor/react-dom-factories");
 const Worker = createFactory(require("./Worker"));
 
 /**
  * This component handles the list of service workers displayed in the application panel
  * and also displays a suggestion to use about debugging for debugging other service
  * workers.
  */
 class WorkerList extends Component {
@@ -23,30 +23,29 @@ class WorkerList extends Component {
     };
   }
 
   render() {
     const { workers, client, serviceContainer } = this.props;
     const { openTrustedLink } = serviceContainer;
 
     return [
-      ul({ className: "application-workers-container" },
-        li({},
-          h1({ className: "application-title" }, "Service Workers")
-        ),
-        workers.map(worker => Worker({
-          client,
-          debugDisabled: false,
-          worker,
-        }))
+      article({ className: "workers-container" },
+        h1({}, "Service Workers"),
+        ul({},
+          workers.map(worker => Worker({
+            client,
+            debugDisabled: false,
+            worker,
+          })))
       ),
-      div({ className: "application-aboutdebugging-plug" },
+      footer({ className: "aboutdebugging-plug" },
         "See about:debugging for Service Workers from other domains",
-        a(
-          { onClick: () => openTrustedLink("about:debugging#workers") },
+        a({ className: "aboutdebugging-plug__link",
+            onClick: () => openTrustedLink("about:debugging#workers") },
           "Open about:debugging"
         )
       )
     ];
   }
 }
 
 // Exports
--- a/devtools/client/application/src/components/WorkerListEmpty.css
+++ b/devtools/client/application/src/components/WorkerListEmpty.css
@@ -8,12 +8,12 @@
   left: 50%;
   top: 50%;
   margin-top: -100px;
   margin-left: -350px;
   position: absolute;
   font-size: 13px;
 }
 
-.worker-list-empty ul {
+.worker-list-empty__list {
   list-style: unset;
   padding: 0 40px;
 }
\ No newline at end of file
--- a/devtools/client/application/src/components/WorkerListEmpty.js
+++ b/devtools/client/application/src/components/WorkerListEmpty.js
@@ -1,17 +1,17 @@
 /* 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 PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { Component } = require("devtools/client/shared/vendor/react");
-const { a, div, li, ul } = require("devtools/client/shared/vendor/react-dom-factories");
+const { a, article, p, li, ul } = require("devtools/client/shared/vendor/react-dom-factories");
 const DOC_URL = "https://developer.mozilla.org/docs/Web/API/Service_Worker_API/Using_Service_Workers" +
   "?utm_source=devtools&utm_medium=sw-panel-blank";
 
 /**
  * This component displays help information when no service workers are found for the
  * current target.
  */
 class WorkerListEmpty extends Component {
@@ -33,33 +33,33 @@ class WorkerListEmpty extends Component 
     this.props.serviceContainer.openTrustedLink("about:debugging#workers");
   }
 
   openDocumentation() {
     this.props.serviceContainer.openWebLink(DOC_URL);
   }
 
   render() {
-    return div(
+    return article(
       { className: "worker-list-empty" },
-      div(
+      p(
         {},
         "You need to register a Service Worker to inspect it here.",
         a(
           { className: "external-link", onClick: () => this.openDocumentation() },
           "Learn More"
         )
       ),
-      div(
+      p(
         {},
         `If the current page should have a service worker, ` +
-        `here are some things you can try`,
+        `here are some things you can try:`,
       ),
       ul(
-        {},
+        { className: "worker-list-empty__list"},
         li(
           {},
           "Look for errors in the Console.",
           a(
             { className: "link", onClick: () => this.switchToConsole() },
             "Open the Console"
           )
         ),
--- a/devtools/client/application/test/browser_application_panel_list-domain-workers.js
+++ b/devtools/client/application/test/browser_application_panel_list-domain-workers.js
@@ -17,17 +17,17 @@ add_task(async function() {
   await enableApplicationPanel();
 
   let { panel, target } = await openNewTabAndApplicationPanel(SIMPLE_URL);
   let doc = panel.panelWin.document;
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
-  let scopeEl = getWorkerContainers(doc)[0].querySelector(".service-worker-scope");
+  let scopeEl = getWorkerContainers(doc)[0].querySelector(".js-sw-scope");
   ok(scopeEl.textContent.startsWith("example.com"),
     "First service worker registration is displayed for the correct domain");
 
   info(
     "Navigate to another page for a different domain with no service worker");
 
   await navigate(target, EMPTY_URL);
   await waitUntil(() => doc.querySelector(".worker-list-empty") !== null);
@@ -35,14 +35,14 @@ add_task(async function() {
 
   info(
     "Navigate to another page for a different domain with another service worker");
   await navigate(target, OTHER_URL);
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
-  scopeEl = getWorkerContainers(doc)[0].querySelector(".service-worker-scope");
+  scopeEl = getWorkerContainers(doc)[0].querySelector(".js-sw-scope");
   ok(scopeEl.textContent.startsWith("test1.example.com"),
     "Second service worker registration is displayed for the correct domain");
 
   await unregisterAllWorkers(target.client);
 });
--- a/devtools/client/application/test/browser_application_panel_list-several-workers.js
+++ b/devtools/client/application/test/browser_application_panel_list-several-workers.js
@@ -16,25 +16,27 @@ add_task(async function() {
 
   let { panel, target } = await openNewTabAndApplicationPanel(SIMPLE_URL);
   let doc = panel.panelWin.document;
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
   info("Wait until the unregister button is displayed for the service worker");
-  await waitUntil(() => getWorkerContainers(doc)[0].querySelector(".unregister-button"));
+  await waitUntil(() => getWorkerContainers(doc)[0]
+    .querySelector(".js-unregister-button"));
 
   ok(true, "First service worker registration is displayed");
 
   info("Navigate to another page for the same domain with another service worker");
   await navigate(target, OTHER_SCOPE_URL);
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 2);
 
   info("Wait until the unregister button is displayed for the service worker");
-  await waitUntil(() => getWorkerContainers(doc)[1].querySelector(".unregister-button"));
+  await waitUntil(() => getWorkerContainers(doc)[1]
+    .querySelector(".js-unregister-button"));
 
   ok(true, "Second service worker registration is displayed");
 
   await unregisterAllWorkers(target.client);
 });
--- a/devtools/client/application/test/browser_application_panel_list-single-worker.js
+++ b/devtools/client/application/test/browser_application_panel_list-single-worker.js
@@ -20,19 +20,19 @@ add_task(async function() {
   });
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length > 0);
 
   let workerContainer = getWorkerContainers(doc)[0];
 
   info("Wait until the unregister button is displayed for the service worker");
-  await waitUntil(() => workerContainer.querySelector(".unregister-button"));
+  await waitUntil(() => workerContainer.querySelector(".js-unregister-button"));
 
-  let scopeEl = workerContainer.querySelector(".service-worker-scope");
+  let scopeEl = workerContainer.querySelector(".js-sw-scope");
   let expectedScope = "example.com/browser/devtools/client/application/test/" +
                       "service-workers/";
   ok(scopeEl.textContent.startsWith(expectedScope),
     "Service worker has the expected scope");
 
   info("Unregister the service worker");
   await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
     let registration = await content.wrappedJSObject.sw;
--- a/devtools/client/application/test/browser_application_panel_list-unicode.js
+++ b/devtools/client/application/test/browser_application_panel_list-unicode.js
@@ -17,17 +17,17 @@ add_task(async function() {
   let { panel, target } = await openNewTabAndApplicationPanel(TAB_URL);
   let doc = panel.panelWin.document;
 
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
   let workerContainer = getWorkerContainers(doc)[0];
 
-  let scopeEl = workerContainer.querySelector(".service-worker-scope");
+  let scopeEl = workerContainer.querySelector(".js-sw-scope");
   ok(
     scopeEl.textContent.startsWith(
       "\u03C0\u03B1\u03C1\u03AC\u03B4\u03B5\u03B9\u03B3\u03BC\u03B1." +
       "\u03B4\u03BF\u03BA\u03B9\u03BC\u03AE"),
     "Service worker has the expected Unicode scope"
   );
   let urlEl = workerContainer.querySelector(".js-source-url");
   ok(
--- a/devtools/client/application/test/head.js
+++ b/devtools/client/application/test/head.js
@@ -32,17 +32,17 @@ async function enableApplicationPanel() 
   // Enable all preferences related to service worker debugging.
   await enableServiceWorkerDebugging();
 
   // Enable application panel in DevTools.
   await pushPref("devtools.application.enabled", true);
 }
 
 function getWorkerContainers(doc) {
-  return doc.querySelectorAll(".service-worker-container");
+  return doc.querySelectorAll(".js-sw-container");
 }
 
 function navigate(target, url, waitForTargetEvent = "navigate") {
   executeSoon(() => target.activeTab.navigateTo(url));
   return once(target, waitForTargetEvent);
 }
 
 async function openNewTabAndApplicationPanel(url) {