Bug 1494543 - Part 1: Refactor sidebar items into different components. r=jdescottes,daisuke

This is a refactor of the components used in the sidebar. TL;DR: sidebar items now use the composition approach outlined here https://reactjs.org/docs/composition-vs-inheritance.html

Before we had a container `Sidebar` component, which in turn had `SidebarItem` components inside. The issue was that depending on what item is inside, the information and UX displayed is different. Before this patch, we had an optional commponent, `DeviceSidebarItemAction` –which was featuring a "Connect" button, and was only rendered in the runtime sidebar items. However, we now need to display even more info, so continue to pass optional components to `SidebarItem` was tricky.

What this patch does is to preserve `SidebarItem` and treat is a generic container of more specific content. This is passed via the `children` prop, which React automatically maps to the DOM content that we pass to that component (this is the same concept as slots in Web Components / Vue). `SidebarItem` now only contains the logic to select items in the sidebar and render them in `<li>` elements. Two new components, `SidebarFixedItem` (for our "static" pages) and `SidebarDeviceItem` are now the ones instancing `SidebarItem` with their specific contents.

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

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

devtools/client/aboutdebugging-new/aboutdebugging.css

@@ -14,9 +14,10 @@
 @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/SidebarFixedItem.css";
 @import "resource://devtools/client/aboutdebugging-new/src/components/sidebar/SidebarItem.css";
+@import "resource://devtools/client/aboutdebugging-new/src/components/sidebar/SidebarRuntimeItem.css";
 
 :root {
   /* Import css variables from common.css */

devtools/client/aboutdebugging-new/src/components/sidebar/DeviceSidebarItemAction.css

@@ -1,7 +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/. */
-
-.sidebar-item__connect-button {
-  font-size: 0.8em;
-}

devtools/client/aboutdebugging-new/src/components/sidebar/DeviceSidebarItemAction.js

@@ -1,57 +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/. */
-
-"use strict";
-
-const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
-const dom = require("devtools/client/shared/vendor/react-dom-factories");
-const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-
-const FluentReact = require("devtools/client/shared/vendor/fluent-react");
-const Localized = createFactory(FluentReact.Localized);
-
-const Actions = require("../../actions/index");
-
-/**
- * This component displays actions for sidebar items representing a device.
- */
-class DeviceSidebarItemAction extends PureComponent {
-  static get propTypes() {
-    return {
-      connected: PropTypes.bool.isRequired,
-      dispatch: PropTypes.func.isRequired,
-      runtimeId: PropTypes.string.isRequired,
-    };
-  }
-
-  render() {
-    const { connected } = this.props;
-    if (connected) {
-      return Localized(
-        {
-          id: "about-debugging-sidebar-item-connected-label"
-        },
-        dom.span({}, "Connected")
-      );
-    }
-
-    return Localized(
-      {
-        id: "about-debugging-sidebar-item-connect-button"
-      },
-      dom.button(
-        {
-          className: "sidebar-item__connect-button",
-          onClick: () => {
-            const { dispatch, runtimeId } = this.props;
-            dispatch(Actions.connectRuntime(runtimeId));
-          }
-        },
-        "Connect"
-      )
-    );
-  }
-}
-
-module.exports = DeviceSidebarItemAction;

devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.js

@@ -13,8 +13,8 @@ const Localized = createFactory(FluentReact.Localized);
 
 const { PAGES, RUNTIMES } = require("../../constants");
 
-const DeviceSidebarItemAction = createFactory(require("./DeviceSidebarItemAction"));
-const SidebarItem = createFactory(require("./SidebarItem"));
+const SidebarFixedItem = createFactory(require("./SidebarFixedItem"));
+const SidebarRuntimeItem = createFactory(require("./SidebarRuntimeItem"));
 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";
@@ -58,22 +58,15 @@ class Sidebar extends PureComponent {
       const pageId = runtime.type + "-" + runtime.id;
       const runtimeHasConnection = !!runtime.connection;
 
-      const connectComponent = DeviceSidebarItemAction({
-        connected: runtimeHasConnection,
-        dispatch,
-        runtimeId: runtime.id,
-      });
-
-      return SidebarItem({
-        connectComponent,
+      return SidebarRuntimeItem({
         id: pageId,
         dispatch,
         icon,
+        isConnected: runtimeHasConnection,
         isSelected: selectedPage === pageId,
         key: pageId,
         name: runtime.name,
         runtimeId: runtime.id,
-        selectable: runtimeHasConnection,
       });
     });
   }
@@ -89,25 +82,23 @@ class Sidebar extends PureComponent {
         {},
         Localized(
           { id: "about-debugging-sidebar-this-firefox", attrs: { name: true } },
-          SidebarItem({
+          SidebarFixedItem({
             id: PAGES.THIS_FIREFOX,
             dispatch,
             icon: FIREFOX_ICON,
             isSelected: PAGES.THIS_FIREFOX === selectedPage,
             name: "This Firefox",
             runtimeId: RUNTIMES.THIS_FIREFOX,
-            selectable: true,
           })
         ),
         Localized(
           { id: "about-debugging-sidebar-connect", attrs: { name: true } },
-          SidebarItem({
+          SidebarFixedItem({
             id: PAGES.CONNECT,
             dispatch,
             icon: CONNECT_ICON,
             isSelected: PAGES.CONNECT === selectedPage,
             name: "Connect",
-            selectable: true,
           })
         ),
         dom.hr(),

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarFixedItem.css

@@ -0,0 +1,31 @@
+/* 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/. */
+
+/*
+ * Layout of a fixed sidebar item
+ *
+ *  +--------+----------------+
+ *  | Icon   | Name           |
+ *  +--------+----------------+
+ */
+
+.sidebar-fixed-item {
+  align-items: center;
+  border-radius: 2px;
+  display: grid;
+  grid-template-columns: 34px 1fr;
+  font-size: 16px;
+  height: 48px;
+  padding-inline-end: 10px;
+  padding-inline-start: 10px;
+}
+
+.sidebar-fixed-item__icon {
+  fill: currentColor;
+  height: 24px;
+  margin-inline-end: 9px;
+  width: 24px;
+  -moz-context-properties: fill;
+}
+

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarFixedItem.js

@@ -0,0 +1,66 @@
+/* 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 { PureComponent, createFactory } = require("devtools/client/shared/vendor/react");
+const dom = require("devtools/client/shared/vendor/react-dom-factories");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
+const SidebarItem = createFactory(require("./SidebarItem"));
+
+const Actions = require("../../actions/index");
+
+/**
+ * This component displays a fixed item in the Sidebar component.
+ */
+class SidebarFixedItem extends PureComponent {
+  static get propTypes() {
+    return {
+      dispatch: PropTypes.func.isRequired,
+      icon: PropTypes.string.isRequired,
+      id: PropTypes.string.isRequired,
+      isSelected: PropTypes.bool.isRequired,
+      name: PropTypes.string.isRequired,
+      runtimeId: PropTypes.string,
+    };
+  }
+
+  render() {
+    const {
+      dispatch,
+      id,
+      icon,
+      isSelected,
+      name,
+      runtimeId,
+    } = this.props;
+
+    return SidebarItem(
+      {
+        isSelected,
+        selectable: true,
+        className: "sidebar-fixed-item",
+        onSelect: () => {
+          dispatch(Actions.selectPage(id, runtimeId));
+        }
+      },
+      dom.img(
+        {
+          className: "sidebar-fixed-item__icon",
+          src: icon,
+        }
+      ),
+      dom.span(
+        {
+          className: "ellipsis-text",
+          title: name,
+        },
+        name
+      )
+    );
+  }
+}
+
+module.exports = SidebarFixedItem;

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarItem.css

@@ -9,21 +9,9 @@
   --sidebar-background-hover: var(--in-content-category-background-hover);
 }
 
-/*
- * Layout of a sidebar item
- *
- *  +--------+----------------+---------------------------+
- *  | Icon   | Name           | Connect button (optional) |
- *  +--------+----------------+---------------------------+
- */
 .sidebar-item {
-  align-items: center;
-  border-radius: 2px;
   color: var(--sidebar-text-color);
-  display: grid;
-  grid-template-columns: 34px auto max-content;
-  font-size: 16px;
-  height: 48px;
+  border-radius: 2px;
   padding-inline-end: 10px;
   padding-inline-start: 10px;
   transition: background-color 150ms;
@@ -38,14 +26,6 @@
   background-color: var(--sidebar-background-hover);
 }
 
-.sidebar-item__icon {
-  fill: currentColor;
-  height: 24px;
-  margin-inline-end: 9px;
-  width: 24px;
-  -moz-context-properties: fill;
-}
-
 .sidebar-item--selected {
   color: var(--sidebar-selected-color);
 }

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarItem.js

@@ -8,36 +8,31 @@ const { PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
-const Actions = require("../../actions/index");
-
 /**
- * This component displays an item of the Sidebar component.
+ * This component is used as a wrapper by items in the sidebar.
  */
 class SidebarItem extends PureComponent {
   static get propTypes() {
     return {
-      connectComponent: PropTypes.any,
-      dispatch: PropTypes.func.isRequired,
-      icon: PropTypes.string.isRequired,
-      id: PropTypes.string.isRequired,
+      children: PropTypes.arrayOf(PropTypes.element).isRequired,
+      className: PropTypes.string,
       isSelected: PropTypes.bool.isRequired,
-      name: PropTypes.string.isRequired,
-      runtimeId: PropTypes.string,
       selectable: PropTypes.bool.isRequired,
+      onSelect: PropTypes.func.isRequired,
     };
   }
 
   onItemClick() {
-    const { id, dispatch, runtimeId } = this.props;
-    dispatch(Actions.selectPage(id, runtimeId));
+    this.props.onSelect();
   }
 
   render() {
-    const { connectComponent, icon, isSelected, name, selectable } = this.props;
+    const {children, className, isSelected, selectable } = this.props;
 
     return dom.li(
       {
         className: "sidebar-item js-sidebar-item" +
+                   (className ? ` ${className}` : "") +
                    (isSelected ?
                       " sidebar-item--selected js-sidebar-item-selected" :
                       ""
@@ -45,17 +40,7 @@ class SidebarItem extends PureComponent {
                    (selectable ? " sidebar-item--selectable" : ""),
         onClick: selectable ? () => this.onItemClick() : null
       },
-      dom.img({
-        className: "sidebar-item__icon",
-        src: icon,
-      }),
-      dom.span(
-        {
-          className: "ellipsis-text",
-          title: name,
-        },
-        name),
-      connectComponent ? connectComponent : null
+      children
     );
   }
 }

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarRuntimeItem.css

@@ -0,0 +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/. */
+
+/*
+ * Layout of a runtime sidebar item
+ *
+ *  +--------+----------------+---------------------------+
+ *  | Icon   | Runtime name   | Connect button            |
+ *  +--------+----------------+---------------------------+
+ */
+
+.sidebar-runtime-item {
+  align-items: center;
+  display: grid;
+  grid-template-columns: 1fr max-content;
+}

devtools/client/aboutdebugging-new/src/components/sidebar/SidebarRuntimeItem.js

@@ -0,0 +1,92 @@
+/* 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 { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
+const dom = require("devtools/client/shared/vendor/react-dom-factories");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
+const FluentReact = require("devtools/client/shared/vendor/fluent-react");
+const Localized = createFactory(FluentReact.Localized);
+
+const SidebarItem = createFactory(require("./SidebarItem"));
+const Actions = require("../../actions/index");
+
+/**
+ * This component displays a runtime item of the Sidebar component.
+ */
+class SidebarRuntimeItem extends PureComponent {
+  static get propTypes() {
+    return {
+      dispatch: PropTypes.func.isRequired,
+      icon: PropTypes.string.isRequired,
+      id: PropTypes.string.isRequired,
+      isConnected: PropTypes.bool.isRequired,
+      isSelected: PropTypes.bool.isRequired,
+      name: PropTypes.string.isRequired,
+      runtimeId: PropTypes.string.isRequired,
+    };
+  }
+
+  renderConnectButton() {
+    return Localized(
+      {
+        id: "about-debugging-sidebar-item-connect-button"
+      },
+      dom.button(
+        {
+          className: "sidebar-item__connect-button",
+          onClick: () => {
+            const { dispatch, runtimeId } = this.props;
+            dispatch(Actions.connectRuntime(runtimeId));
+          }
+        },
+        "Connect"
+      )
+    );
+  }
+
+  render() {
+    const {
+      dispatch,
+      icon,
+      id,
+      isConnected,
+      isSelected,
+      name,
+      runtimeId,
+    } = this.props;
+
+    return SidebarItem(
+      {
+        isSelected,
+        selectable: isConnected,
+        className: "sidebar-runtime-item",
+        onSelect: () => {
+          dispatch(Actions.selectPage(id, runtimeId));
+        }
+      },
+      dom.span(
+        { className: "ellipsis-text" },
+        dom.img(
+          {
+            className: "sidebar-runtime-item__icon " +
+             `${isConnected ? "sidebar-runtime-item__icon--connected" : "" }`,
+            src: icon,
+          }
+        ),
+        dom.span(
+          {
+            title: name,
+          },
+          ` ${name}`
+        ),
+      ),
+      !isConnected ? this.renderConnectButton() : null
+    );
+  }
+}
+
+module.exports = SidebarRuntimeItem;

devtools/client/aboutdebugging-new/src/components/sidebar/moz.build

@@ -3,10 +3,12 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DevToolsModules(
-    'DeviceSidebarItemAction.css',
-    'DeviceSidebarItemAction.js',
     'Sidebar.css',
     'Sidebar.js',
+    'SidebarFixedItem.css',
+    'SidebarFixedItem.js',
     'SidebarItem.css',
     'SidebarItem.js',
+    'SidebarRuntimeItem.css',
+    'SidebarRuntimeItem.js',
 )