Bug 1557694 - Use <button> instead of <a> for Start/Debug workers r=Ola

**NOTE: This depends on D35513, so if it has not landed yet, please `arc patch D35513` before patching this one on top.**

- `.devtools-button` styles in `common.css` are kinda broken , so I decided to roll out our own button component (`UIButton`) after consulting with Victoria colors, sizes, etc. The other downside to selectors in `common.css` is that they can have a high specificity :(
- Victoria said to use the "micro" style from Photon as the default style for buttons in the panels. So I created an even smaller "micro" styles (very similar to `.devtools-togglebutton`) for when we need smaller buttons.
- I created some light/dark variables in our stylesheets instead of on `variables.css` because `--theme-button-background` was already taken (only used in a single panel, but still…). Maybe after the buttons are fixed globally in the common folder, we could use the variables there. In the meantime, to avoid losing more time, I rolled out our own vars here.

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

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

devtools/client/application/application.css

@@ -14,14 +14,7 @@
 @import "resource://devtools/client/application/src/components/Worker.css";
 @import "resource://devtools/client/application/src/components/WorkerList.css";
 @import "resource://devtools/client/application/src/components/WorkerListEmpty.css";
-
-
-/*
-* Reset
-*/
-* {
-  box-sizing: border-box;
-}
+@import "resource://devtools/client/application/src/components/ui/UIButton.css";
 
 html,
 body,

devtools/client/application/src/base.css

@@ -5,6 +5,8 @@
 :root {
   /* Typography from Photon */
   /* See https://firefox-dev.tools/photon/visuals/typography.html */
+  --caption-10-font-size: 11px;
+  --caption-10-font-weight: 400;
   --body-10-font-size: 13px;
   --body-10-font-weight: 400;
   --body-20-font-size: 15px;
@@ -27,12 +29,19 @@
 
   /* Global layout vars */
   --base-unit: 4px;
+
+  /* extra, raw colors */
+  --blue-50-a30: rgba(10, 132, 255, 0.3);
 }
 
 /*
 * Reset some tags
 */
 
+* {
+  box-sizing: border-box;
+}
+
 body {
   margin: 0;
   padding: 0;

devtools/client/application/src/components/Worker.css

@@ -11,7 +11,7 @@
  *  |     "Source" | script_name debug_link       |
     |              | "Updated" update_time        |
  *  |--------------+-------------+----------------|
- *  |     "Status" | status start_link            |
+ *  |     "Status" | status start_button          |
  *  +---+----------+-------------+----------------|
  */
 
@@ -62,7 +62,7 @@
 .worker__data {
   display: grid;
   grid-template-columns: auto 1fr;
-  grid-column-gap: 1rem;
+  grid-gap: 1rem;
 }
 
 .worker__data > * {

devtools/client/application/src/components/Worker.js

@@ -8,11 +8,11 @@ const {
   createFactory,
   Component,
 } = require("devtools/client/shared/vendor/react");
+
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
 const {
-  a,
   br,
-  button,
   dd,
   dl,
   dt,
@@ -22,6 +22,7 @@ const {
   span,
   time,
 } = require("devtools/client/shared/vendor/react-dom-factories");
+
 const {
   getUnicodeUrl,
   getUnicodeUrlPath,
@@ -30,6 +31,8 @@ const {
 const FluentReact = require("devtools/client/shared/vendor/fluent-react");
 const Localized = createFactory(FluentReact.Localized);
 
+const UIButton = createFactory(require("./ui/UIButton"));
+
 loader.lazyRequireGetter(
   this,
   "DebuggerClient",
@@ -136,50 +139,51 @@ class Worker extends Component {
     return getUnicodeUrlPath(parts[parts.length - 1]);
   }
 
-  renderDebugLink() {
+  renderDebugButton() {
     const { isDebugEnabled } = this.props;
 
-    const shallDisableLink = !this.isRunning() || !isDebugEnabled;
-    const linkClass = shallDisableLink ? "disabled-link" : "";
+    const isDisabled = !this.isRunning() || !isDebugEnabled;
 
     const localizationId = isDebugEnabled
       ? "serviceworker-worker-debug"
       : "serviceworker-worker-debug-forbidden";
 
-    const link = Localized(
+    return Localized(
       {
         id: localizationId,
         // The localized title is only displayed if the debug link is disabled.
         attrs: {
-          title: shallDisableLink,
+          title: isDisabled,
         },
       },
-      a({
-        onClick: !shallDisableLink ? this.debug : null,
-        className: `${linkClass} worker__link-debug js-link-debug`,
+      UIButton({
+        onClick: this.debug,
+        className: `js-debug-button`,
+        disabled: isDisabled,
+        size: "micro",
       })
     );
-    return link;
   }
 
-  renderStartLink() {
+  renderStartButton() {
     const { isDebugEnabled } = this.props;
-    const linkClass = !isDebugEnabled ? "disabled-link" : "";
+    const isDisabled = !isDebugEnabled;
 
-    const link = Localized(
+    return Localized(
       {
         id: "serviceworker-worker-start2",
         // The localized title is only displayed if the debug link is disabled.
         attrs: {
-          title: !isDebugEnabled,
+          title: !isDisabled,
         },
       },
-      a({
+      UIButton({
         onClick: this.start,
-        className: `worker__link-start js-link-start ${linkClass}`,
+        className: `js-start-button`,
+        disabled: isDisabled,
+        size: "micro",
       })
     );
-    return link;
   }
 
   render() {
@@ -189,11 +193,9 @@ class Worker extends Component {
     const unregisterButton = this.isActive()
       ? Localized(
           { id: "serviceworker-worker-unregister" },
-          button({
+          UIButton({
             onClick: this.unregister,
-            className:
-              "devtools-button worker__unregister-button js-unregister-button",
-            "data-standalone": true,
+            className: "worker__unregister-button js-unregister-button",
           })
         )
       : null;
@@ -237,7 +239,8 @@ class Worker extends Component {
             },
             this.formatSource(worker.url)
           ),
-          this.renderDebugLink(),
+          " ",
+          this.renderDebugButton(),
           lastUpdated ? br({}) : null,
           lastUpdated ? lastUpdated : null
         ),
@@ -251,7 +254,8 @@ class Worker extends Component {
             { id: "serviceworker-worker-status-" + status },
             span({ className: "js-worker-status" })
           ),
-          !this.isRunning() ? this.renderStartLink() : null
+          " ",
+          !this.isRunning() ? this.renderStartButton() : null
         )
       )
     );

devtools/client/application/src/components/moz.build

@@ -2,6 +2,10 @@
 # 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/.
 
+DIRS += [
+    'ui',
+]
+
 DevToolsModules(
     'App.css',
     'App.js',

devtools/client/application/src/components/ui/UIButton.css

@@ -0,0 +1,71 @@
+/* these styles com from Photon. Keep in mind that the "default" style is not used
+   in panels, and we should use the "micro" instead for default, stand-alone buttons. */
+
+:root.theme-light {
+  --button-text-color: var(--grey-90);
+  --button-text-hover-color: var(--grey-90);
+  --button-text-pressed-color: var(--grey-90);
+  --button-background-color: var(--grey-90-a10);
+  --button-background-hover-color: var(--grey-90-a20);
+  --button-background-pressed-color: var(--grey-90-a30);
+}
+
+:root.theme-dark {
+  --button-text-color: var(--grey-40);
+  --button-text-hover-color: var(--grey-30);
+  --button-text-pressed-color: var(--grey-30);
+  --button-background-color: var(--grey-10-a20);
+  --button-background-hover-color: var(--grey-10-a25);
+  --button-background-pressed-color: var(--grey-10-a30);
+}
+
+.ui-button {
+  -moz-appearance: none;
+  transition: background-color 0.05s ease-in-out;
+
+  margin: 0;
+  height: calc(var(--base-unit) * 6);
+  padding-inline-start: calc(2 * var(--base-unit));
+  padding-inline-end: calc(2 * var(--base-unit));
+  border: none;
+  border-radius: calc(var(--base-unit) / 2);
+
+  color: var(--button-text-color);
+  background: var(--button-background-color);
+  font-size: var(--caption-10-font-size);
+}
+
+.ui-button:-moz-focusring {
+  outline: none;
+}
+.ui-button::-moz-focus-inner {
+  border: 0;
+  padding: 0;
+}
+
+.ui-button:not(:disabled):hover {
+  background: var(--button-background-hover-color);
+  color: var(--button-text-hover-color);
+}
+
+.ui-button:not(:disabled):active {
+  background: var(--button-background-pressed-color);
+  color: var(--button-text-pressed-color);
+}
+
+.ui-button:focus {
+  box-shadow: 0 0 0 1px var(--blue-50) inset,
+    0 0 0 1px var(--blue-50),
+    0 0 0 4px var(--blue-50-a30);
+}
+
+.ui-button:disabled {
+  opacity: 0.4;
+}
+
+/* Note: this "micro" variant here is not the same as the "micro" variant
+         in Photon docs (since we are using that one for our default size) */
+.ui-button--micro {
+  height: auto;
+  padding: calc(var(--base-unit) * 0.5) var(--base-unit);
+}

devtools/client/application/src/components/ui/UIButton.js

@@ -0,0 +1,33 @@
+"use strict";
+
+const { PureComponent } = require("devtools/client/shared/vendor/react");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+const { button } = require("devtools/client/shared/vendor/react-dom-factories");
+
+class UIButton extends PureComponent {
+  static get propTypes() {
+    return {
+      children: PropTypes.node,
+      className: PropTypes.string,
+      disabled: PropTypes.bool,
+      onClick: PropTypes.func,
+      size: PropTypes.oneOf(["micro"]),
+    };
+  }
+
+  render() {
+    const { className, disabled, onClick, size } = this.props;
+    const sizeClass = size ? `ui-button--${size}` : "";
+
+    return button(
+      {
+        className: `ui-button ${className || ""} ${sizeClass}`,
+        onClick,
+        disabled,
+      },
+      this.props.children
+    );
+  }
+}
+
+module.exports = UIButton;

devtools/client/application/src/components/ui/moz.build

@@ -0,0 +1,8 @@
+# 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(
+    'UIButton.css',
+    'UIButton.js',
+)

devtools/client/application/test/browser/browser_application_panel_debug-service-worker.js

@@ -27,16 +27,17 @@ add_task(async function() {
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
   const container = getWorkerContainers(doc)[0];
-  info("Wait until the debug link is displayed and enabled");
-  await waitUntil(() =>
-    container.querySelector(".js-link-debug:not(.worker__debug-link--disabled)")
-  );
+  info("Wait until the debug button is displayed and enabled");
+  await waitUntil(() => {
+    const button = container.querySelector(".js-debug-button");
+    return button && !button.disabled;
+  });
 
-  info("Click on the debug link and wait for the new toolbox to be ready");
+  info("Click on the debug button and wait for the new toolbox to be ready");
   const onToolboxReady = gDevTools.once("toolbox-ready");
 
-  const debugLink = container.querySelector(".js-link-debug");
-  debugLink.click();
+  const debugButton = container.querySelector(".js-debug-button");
+  debugButton.click();
 
   const serviceWorkerToolbox = await onToolboxReady;
   await serviceWorkerToolbox.selectTool("jsdebugger");

devtools/client/application/test/browser/browser_application_panel_start-service-worker.js

@@ -26,17 +26,22 @@ add_task(async function() {
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
-  info("Wait until the start link is displayed and enabled");
+  info("Wait until the start button is displayed and enabled");
   const container = getWorkerContainers(doc)[0];
-  await waitUntil(() =>
-    container.querySelector(".js-link-start:not(.disabled-link)")
-  );
-  info("Click the link and wait for the worker to start");
-  const link = container.querySelector(".js-link-start");
-  link.click();
-  await waitUntil(
-    () => container.querySelector(".js-worker-status").textContent === "Running"
-  );
+  await waitUntil(() => {
+    const button = container.querySelector(".js-start-button");
+    return button && !button.disabled;
+  });
+
+  info("Click the button and wait for the worker to start");
+  const button = container.querySelector(".js-start-button");
+  button.click();
+
+  info("Wait until status 'Running' is displayed");
+  await waitUntil(() => {
+    const statusEl = container.querySelector(".js-worker-status");
+    return statusEl && statusEl.textContent === "Running";
+  });
   ok(true, "Worker status is 'Running'");
 
   await unregisterAllWorkers(target.client);
@@ -59,12 +64,12 @@ add_task(async function() {
   info("Wait until the service worker appears in the application panel");
   await waitUntil(() => getWorkerContainers(doc).length === 1);
 
-  info("Wait until the start link is displayed");
+  info("Wait until the start button is displayed");
   const container = getWorkerContainers(doc)[0];
-  await waitUntil(() => container.querySelector(".js-link-start"));
+  await waitUntil(() => container.querySelector(".js-start-button"));
   ok(
-    container.querySelector(".js-link-start.disabled-link"),
-    "Start link is disabled"
+    container.querySelector(".js-start-button").disabled,
+    "Start button is disabled"
   );
 
   await unregisterAllWorkers(target.client);