Bug 1565213 - Add routing for sidebar component, r=ladybenko

Add routing for sidebar component Differential Revision: https://phabricator.services.mozilla.com/D43726 --HG-- extra : moz-landing-system : lando
dothq · Sep 12, 2019 · caf9aff · caf9aff
1 parent 8bc66c2
commit caf9aff
Show file tree

Hide file tree

Showing 28 changed files with 367 additions and 51 deletions.
diff --git a/devtools/client/application/src/components/manifest/ManifestIssueList.css b/devtools/client/application/src/components/manifest/ManifestIssueList.css
@@ -1,3 +1,7 @@
+/* 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/. */
+
 .manifest-issues {
   list-style-type: none;
   padding-inline-start: 0;

diff --git a/devtools/client/application/src/components/manifest/ManifestItem.css b/devtools/client/application/src/components/manifest/ManifestItem.css
@@ -8,8 +8,8 @@
 
 .manifest-item__label {
   color: var(--theme-text-color-alt);
-  font-weight: var(--title-30-font-weight);
   width: calc(var(--base-unit) * 28);
+  font-weight: var(--title-10-font-weight);
   text-align: right;
 }
 

diff --git a/devtools/client/application/src/components/manifest/ManifestPage.js b/devtools/client/application/src/components/manifest/ManifestPage.js
@@ -48,7 +48,9 @@ class ManifestPage extends PureComponent {
 
     return section(
       {
-        className: `app-page ${!manifest ? "app-page--empty" : ""}`,
+        className: `app-page js-manifest-page ${
+          !manifest ? "app-page--empty" : ""
+        }`,
       },
       this.shouldShowLoader ? ManifestLoader({}) : this.renderManifest()
     );

diff --git a/devtools/client/application/src/components/manifest/ManifestSection.css b/devtools/client/application/src/components/manifest/ManifestSection.css
@@ -6,6 +6,8 @@
   padding-block: calc(var(--base-unit) * 2);
   width: 100%;
   border-spacing: calc(var(--base-unit) * 2) 0;
+  font-size: var(--body-10-font-size);
+  font-weight: var(--body-10-font-weight);
 }
 
 .manifest-section--empty {

diff --git a/devtools/client/application/src/components/routing/Sidebar.js b/devtools/client/application/src/components/routing/Sidebar.js
@@ -12,18 +12,32 @@ const {
   aside,
   ul,
 } = require("devtools/client/shared/vendor/react-dom-factories");
+const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+
+const { connect } = require("devtools/client/shared/vendor/react-redux");
 
 const SidebarItem = createFactory(require("./SidebarItem"));
 
 const { PAGE_TYPES } = require("../../constants");
 
 class Sidebar extends PureComponent {
+  static get propTypes() {
+    return {
+      // this prop is automatically injected via connect
+      selectedPage: PropTypes.oneOf(Object.values(PAGE_TYPES)),
+    };
+  }
+
   render() {
     const navItems = Object.values(PAGE_TYPES);
 
+    const isSelected = page => {
+      return page === this.props.selectedPage;
+    };
+
     return aside(
       {
-        className: "sidebar",
+        className: "sidebar js-sidebar",
       },
       ul(
         {
@@ -33,12 +47,18 @@ class Sidebar extends PureComponent {
           return SidebarItem({
             page: page,
             key: `sidebar-item-${page}`,
+            isSelected: isSelected(page),
           });
         })
       )
     );
   }
 }
 
-// Exports
-module.exports = Sidebar;
+function mapStateToProps(state) {
+  return {
+    selectedPage: state.ui.selectedPage,
+  };
+}
+
+module.exports = connect(mapStateToProps)(Sidebar);
diff --git a/devtools/client/application/src/components/routing/SidebarItem.css b/devtools/client/application/src/components/routing/SidebarItem.css
@@ -12,9 +12,16 @@
   grid-template-columns: auto 1fr;
   grid-gap: var(--base-unit);
   padding: calc(var(--base-unit)) calc(var(--base-unit) * 6);
+  user-select: none;
+  cursor: pointer;
 }
 
-.sidebar-item:hover {
+.sidebar-item--selected {
+	background-color: var(--theme-selection-background);
+	color: var(--theme-selection-color);
+}
+
+.sidebar-item:not(.sidebar-item--selected):hover {
 	background-color: var(--highlight-color);
 }
 
@@ -23,5 +30,4 @@
 	width: calc(var(--base-unit) * 4);
 	-moz-context-properties: fill;
 	fill: currentColor;
-
 }
diff --git a/devtools/client/application/src/components/routing/SidebarItem.js b/devtools/client/application/src/components/routing/SidebarItem.js
@@ -16,6 +16,9 @@ const {
 
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 
+const Actions = require("./../../actions/index");
+const { connect } = require("devtools/client/shared/vendor/react-redux");
+
 const FluentReact = require("devtools/client/shared/vendor/fluent-react");
 const Localized = createFactory(FluentReact.Localized);
 
@@ -37,12 +40,26 @@ class SidebarItem extends PureComponent {
   static get propTypes() {
     return {
       page: PropTypes.oneOf(Object.values(PAGE_TYPES)),
+      isSelected: PropTypes.bool.isRequired,
+      // this prop is automatically injected via connect
+      dispatch: PropTypes.func.isRequired,
     };
   }
+
   render() {
-    const { page } = this.props;
+    const { isSelected, page } = this.props;
+
     return li(
-      { className: "sidebar-item" },
+      {
+        className: `sidebar-item js-sidebar-${page} ${
+          isSelected ? "sidebar-item--selected" : ""
+        }`,
+        onClick: () => {
+          const { dispatch } = this.props;
+          dispatch(Actions.updateSelectedPage(page));
+        },
+        role: "link",
+      },
       Localized(
         {
           id: LOCALIZATION_IDS[page],
@@ -69,5 +86,5 @@ class SidebarItem extends PureComponent {
   }
 }
 
-// Exports
-module.exports = SidebarItem;
+const mapDispatchToProps = dispatch => ({ dispatch });
+module.exports = connect(mapDispatchToProps)(SidebarItem);
diff --git a/devtools/client/application/src/components/service-workers/WorkersPage.js b/devtools/client/application/src/components/service-workers/WorkersPage.js
@@ -38,7 +38,9 @@ class WorkersPage extends PureComponent {
 
     return section(
       {
-        className: `app-page ${isWorkerListEmpty ? "app-page--empty" : ""}`,
+        className: `app-page js-service-workers-page ${
+          isWorkerListEmpty ? "app-page--empty" : ""
+        }`,
       },
       isWorkerListEmpty
         ? WorkerListEmpty({})

diff --git a/devtools/client/application/src/constants.js b/devtools/client/application/src/constants.js
@@ -19,6 +19,7 @@ const actionTypes = {
   UPDATE_WORKERS: "UPDATE_WORKERS",
 };
 
+// NOTE: these const values are used as part of CSS selectors - be mindful of the characters used
 const PAGE_TYPES = {
   MANIFEST: "manifest",
   SERVICE_WORKERS: "service-workers",

diff --git a/devtools/client/application/test/browser/browser.ini b/devtools/client/application/test/browser/browser.ini
@@ -23,6 +23,7 @@ support-files =
   !/devtools/client/shared/test/shared-head.js
   !/devtools/client/shared/test/telemetry-test-helpers.js
 
+[browser_application_panel_sidebar.js]
 [browser_application_panel_debug-service-worker.js]
 skip-if = debug # Bug 1559591
 [browser_application_panel_list-domain-workers.js]

diff --git a/devtools/client/application/test/browser/browser_application_panel_debug-service-worker.js b/devtools/client/application/test/browser/browser_application_panel_debug-service-worker.js
@@ -23,7 +23,6 @@ add_task(async function() {
   const { panel, tab, target } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   info("Wait until the service worker appears in the application panel");

diff --git a/devtools/client/application/test/browser/browser_application_panel_list-domain-workers.js b/devtools/client/application/test/browser/browser_application_panel_list-domain-workers.js
@@ -21,7 +21,6 @@ add_task(async function() {
   const { panel, target } = await openNewTabAndApplicationPanel(SIMPLE_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   info("Wait until the service worker appears in the application panel");

diff --git a/devtools/client/application/test/browser/browser_application_panel_list-several-workers.js b/devtools/client/application/test/browser/browser_application_panel_list-several-workers.js
@@ -17,7 +17,6 @@ add_task(async function() {
   const { panel, target } = await openNewTabAndApplicationPanel(SIMPLE_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   info("Wait until the service worker appears in the application panel");

diff --git a/devtools/client/application/test/browser/browser_application_panel_list-single-worker.js b/devtools/client/application/test/browser/browser_application_panel_list-single-worker.js
@@ -12,9 +12,9 @@ add_task(async function() {
   const { panel, tab } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
+  info("Check for non-existing service worker");
   const isWorkerListEmpty = !!doc.querySelector(".worker-list-empty");
   ok(isWorkerListEmpty, "No Service Worker displayed");
 

diff --git a/devtools/client/application/test/browser/browser_application_panel_list-unicode.js b/devtools/client/application/test/browser/browser_application_panel_list-unicode.js
@@ -18,7 +18,6 @@ add_task(async function() {
   const { panel, target } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   info("Wait until the service worker appears in the application panel");

diff --git a/devtools/client/application/test/browser/browser_application_panel_list-workers-empty.js b/devtools/client/application/test/browser/browser_application_panel_list-workers-empty.js
@@ -16,7 +16,6 @@ add_task(async function() {
   const { panel, tab } = await openNewTabAndApplicationPanel(EMPTY_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   await waitUntil(() => doc.querySelector(".js-worker-list-empty") !== null);

diff --git a/devtools/client/application/test/browser/browser_application_panel_open-links.js b/devtools/client/application/test/browser/browser_application_panel_open-links.js
@@ -17,7 +17,6 @@ add_task(async function() {
   const { panel, toolbox } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   // detach devtools in a separate window

diff --git a/devtools/client/application/test/browser/browser_application_panel_sidebar.js b/devtools/client/application/test/browser/browser_application_panel_sidebar.js
@@ -0,0 +1,80 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Check that the manifest is being properly shown
+ */
+
+add_task(async function() {
+  info("Test that we are displaying correctly the sidebar");
+
+  await enableApplicationPanel();
+  const { panel, tab } = await openNewTabAndApplicationPanel();
+  const doc = panel.panelWin.document;
+
+  info("Waiting for the sidebar to be displayed");
+  await waitUntil(() => doc.querySelector(".js-sidebar") !== null);
+  ok(true, "Sidebar is being displayed");
+
+  await waitUntil(() => doc.querySelector(".js-manifest-page") !== null);
+  ok(true, "Manifest page was loaded per default.");
+
+  // close the tab
+  info("Closing the tab.");
+  await BrowserTestUtils.removeTab(tab);
+});
+
+add_task(async function() {
+  info("Test that we are displaying correctly the selected page - manifest");
+
+  await enableApplicationPanel();
+  const { panel, tab, target } = await openNewTabAndApplicationPanel();
+  const doc = panel.panelWin.document;
+
+  info("Select service worker page");
+  selectPage(panel, "service-workers");
+  await waitUntil(() => doc.querySelector(".js-service-workers-page") !== null);
+
+  info("Select manifest page in the sidebar");
+  const link = doc.querySelector(".js-sidebar-manifest");
+  link.click();
+
+  await waitUntil(() => doc.querySelector(".js-manifest-page") !== null);
+  ok(true, "Manifest page was selected.");
+
+  await unregisterAllWorkers(target.client);
+
+  // close the tab
+  info("Closing the tab.");
+  await BrowserTestUtils.removeTab(tab);
+});
+
+add_task(async function() {
+  info(
+    "Test that we are displaying correctly the selected page - service workers"
+  );
+  const url = URL_ROOT + "resources/manifest/load-ok.html";
+
+  await enableApplicationPanel();
+  const { panel, tab } = await openNewTabAndApplicationPanel(url);
+  const doc = panel.panelWin.document;
+
+  selectPage(panel, "manifest");
+
+  info("Waiting for the manifest to load");
+  await waitUntil(() => doc.querySelector(".js-manifest-page") !== null);
+  ok(true, "Manifest page was selected.");
+
+  info("Select service worker page in the sidebar");
+  const link = doc.querySelector(".js-sidebar-service-workers");
+  link.click();
+
+  await waitUntil(() => doc.querySelector(".js-service-workers-page") !== null);
+  ok(true, "Service workers page was selected.");
+
+  // close the tab
+  info("Closing the tab.");
+  await BrowserTestUtils.removeTab(tab);
+});
diff --git a/devtools/client/application/test/browser/browser_application_panel_start-service-worker.js b/devtools/client/application/test/browser/browser_application_panel_start-service-worker.js
@@ -21,7 +21,6 @@ add_task(async function() {
   const { panel, tab, target } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   await waitForWorkerRegistration(tab);
@@ -63,7 +62,6 @@ add_task(async function() {
   const { panel, tab, target } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   await waitForWorkerRegistration(tab);

diff --git a/devtools/client/application/test/browser/browser_application_panel_unregister-worker.js b/devtools/client/application/test/browser/browser_application_panel_unregister-worker.js
@@ -11,7 +11,6 @@ add_task(async function() {
   const { panel, tab, target } = await openNewTabAndApplicationPanel(TAB_URL);
   const doc = panel.panelWin.document;
 
-  // select service worker view
   selectPage(panel, "service-workers");
 
   info("Wait until the service worker appears in the application panel");

diff --git a/devtools/client/application/test/browser/head.js b/devtools/client/application/test/browser/head.js
@@ -82,11 +82,13 @@ async function waitForWorkerRegistration(swTab) {
   );
 }
 
-// TODO: update this function once the sidebar links are implemented (See bug
-// https: //bugzilla.mozilla.org/show_bug.cgi?id=1565213), and switch to to
-// click those links instead, since it's more representative of what users do
 function selectPage(panel, page) {
+  /**
+   * Select a page by simulating a user click in the sidebar.
+   * @param {string} page The page we want to select (see `PAGE_TYPES`)
+   **/
   info(`Selecting application page: ${page}`);
-  const actions = panel.panelWin.Application.actions;
-  actions.updateSelectedPage(page);
+  const doc = panel.panelWin.document;
+  const navItem = doc.querySelector(`.js-sidebar-${page}`);
+  navItem.click();
 }
diff --git a/...t/application/test/components/__snapshots__/components_application_panel-App.test.js.snap b/...t/application/test/components/__snapshots__/components_application_panel-App.test.js.snap
@@ -5,7 +5,7 @@ exports[`App renders the expected snapshot 1`] = `
   <main
     className="app"
   >
-    <Sidebar />
+    <Connect(Sidebar) />
     <Connect(PageSwitcher) />
   </main>
 </LocalizationProvider>