Bug 1447499 - Simplify about:studies code r=Gijs

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

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

toolkit/components/normandy/content/about-studies/about-studies.css

@@ -30,11 +30,11 @@ button > .button-box {
 }
 
 .about-studies-container {
-  display: flex;
-  flex-direction: row;
   font-size: 1.25rem;
   min-height: 100%;
   width: 100%;
+  max-width: 960px;
+  margin: 0 auto;
 }
 
 #categories {
@@ -50,10 +50,6 @@ button > .button-box {
   flex-direction: row;
 }
 
-.main-content {
-  flex: 1;
-}
-
 .info-box {
   margin-bottom: 10px;
   text-align: center;

toolkit/components/normandy/content/about-studies/about-studies.html

@@ -17,8 +17,6 @@
     <script src="resource://normandy-vendor/ReactDOM.js"></script>
     <script src="resource://normandy-vendor/PropTypes.js"></script>
     <script src="resource://normandy-vendor/classnames.js"></script>
-    <script src="resource://normandy-content/about-studies/common.js"></script>
-    <script src="resource://normandy-content/about-studies/shield-studies.js"></script>
     <script src="resource://normandy-content/about-studies/about-studies.js"></script>
   </body>
 </html>

toolkit/components/normandy/content/about-studies/about-studies.js

@@ -1,23 +1,24 @@
 /* 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";
-/* global classnames PropTypes r React ReactDOM remoteValues ShieldStudies */
+/* global classnames PropTypes React ReactDOM */
 
 /**
- * Mapping of pages displayed on the sidebar. Keys are the value used in the
- * URL hash to identify the current page.
- *
- * Pages will appear in the sidebar in the order they are defined here. If the
- * URL doesn't contain a hash, the first page will be displayed in the content area.
+ * Shorthand for creating elements (to avoid using a JSX preprocessor)
  */
-const PAGES = new Map([
-  ["shieldStudies", {
-    name: "title",
-    component: ShieldStudies,
-    icon: "resource://normandy-content/about-studies/img/shield-logo.png",
-  }],
-]);
+const r = React.createElement;
+
+/**
+ * Dispatches a page event to the privileged frame script for this tab.
+ * @param {String} action
+ * @param {Object} data
+ */
+function sendPageEvent(action, data) {
+  const event = new CustomEvent("ShieldPageEvent", { bubbles: true, detail: { action, data } });
+  document.dispatchEvent(event);
+}
 
 /**
  * Handle basic layout and routing within about:studies.
@@ -26,133 +27,172 @@ class AboutStudies extends React.Component {
   constructor(props) {
     super(props);
 
-    let hash = new URL(window.location).hash.slice(1);
-    if (!PAGES.has(hash)) {
-      hash = "shieldStudies";
-    }
-
-    this.state = {
-      currentPageId: hash,
+    this.remoteValueNameMap = {
+      StudyList: "addonStudies",
+      ShieldLearnMoreHref: "learnMoreHref",
+      StudiesEnabled: "studiesEnabled",
+      ShieldTranslations: "translations",
     };
 
-    this.handleEvent = this.handleEvent.bind(this);
+    this.state = {};
+    for (const stateName of Object.values(this.remoteValueNameMap)) {
+      this.state[stateName] = null;
+    }
   }
 
-  componentDidMount() {
-    remoteValues.shieldTranslations.subscribe(this);
-    window.addEventListener("hashchange", this);
+  componentWillMount() {
+    for (const remoteName of Object.keys(this.remoteValueNameMap)) {
+      document.addEventListener(`ReceiveRemoteValue:${remoteName}`, this);
+      sendPageEvent(`GetRemoteValue:${remoteName}`);
+    }
   }
 
   componentWillUnmount() {
-    remoteValues.shieldTranslations.unsubscribe(this);
-    window.removeEventListener("hashchange", this);
-  }
-
-  receiveRemoteValue(name, value) {
-    switch (name) {
-      case "ShieldTranslations": {
-        this.setState({ translations: value });
-        break;
-      }
-      default: {
-        console.error(`Unknown remote value ${name}`);
-      }
+    for (const remoteName of Object.keys(this.remoteValueNameMap)) {
+      document.removeEventListener(`ReceiveRemoteValue:${remoteName}`, this);
     }
   }
 
-  handleEvent(event) {
-    const newHash = new URL(event.newURL).hash.slice(1);
-    if (PAGES.has(newHash)) {
-      this.setState({currentPageId: newHash});
+  /** Event handle to receive remote values from documentAddEventListener */
+  handleEvent({ type, detail: value }) {
+    const prefix = "ReceiveRemoteValue:";
+    if (type.startsWith(prefix)) {
+      const name = type.substring(prefix.length);
+      this.setState({ [this.remoteValueNameMap[name]]: value });
     }
   }
 
   render() {
-    const currentPageId = this.state.currentPageId;
-    const pageEntries = Array.from(PAGES.entries());
-    const currentPage = PAGES.get(currentPageId);
-    const { translations } = this.state;
+    const { translations, learnMoreHref, studiesEnabled, addonStudies } = this.state;
+
+    // Wait for all values to be loaded before rendering. Some of the values may
+    // be falsey, so an explicit null check is needed.
+    if (Object.values(this.state).some(v => v === null)) {
+      return null;
+    }
 
     return (
-      r("div", {className: "about-studies-container"},
-        translations && r(Sidebar, {},
-          pageEntries.map(([id, page]) => (
-            r(SidebarItem, {
-              key: id,
-              pageId: id,
-              selected: id === currentPageId,
-              page,
-              translations,
-            })
-          )),
-        ),
-        r(Content, {},
-          translations && currentPage && r(currentPage.component, {translations})
-        ),
+      r("div", { className: "about-studies-container main-content" },
+        r(WhatsThisBox, { translations, learnMoreHref, studiesEnabled }),
+        r(StudyList, { translations, addonStudies }),
       )
     );
   }
 }
 
-class Sidebar extends React.Component {
+/**
+ * Explains the contents of the page, and offers a way to learn more and update preferences.
+ */
+class WhatsThisBox extends React.Component {
+  handleUpdateClick() {
+    sendPageEvent("NavigateToDataPreferences");
+  }
+
   render() {
-    return r("ul", {id: "categories"}, this.props.children);
+    const { learnMoreHref, studiesEnabled, translations } = this.props;
+
+    return (
+      r("div", { className: "info-box" },
+        r("div", { className: "info-box-content" },
+          r("span", {},
+            studiesEnabled ? translations.enabledList : translations.disabledList,
+          ),
+          r("a", { id: "shield-studies-learn-more", href: learnMoreHref }, translations.learnMore),
+
+          r("button", { id: "shield-studies-update-preferences", onClick: this.handleUpdateClick },
+            r("div", { className: "button-box" },
+              navigator.platform.includes("Win") ? translations.updateButtonWin : translations.updateButtonUnix
+            ),
+          )
+        )
+      )
+    );
   }
 }
-Sidebar.propTypes = {
-  children: PropTypes.node,
+
+/**
+ * Shows a list of studies, with an option to end in-progress ones.
+ */
+class StudyList extends React.Component {
+  render() {
+    const { addonStudies, translations } = this.props;
+
+    if (!addonStudies.length) {
+      return r("p", { className: "study-list-info" }, translations.noStudies);
+    }
+
+    addonStudies.sort((a, b) => {
+      if (a.active !== b.active) {
+        return a.active ? -1 : 1;
+      }
+      return b.studyStartDate - a.studyStartDate;
+    });
+
+    return (
+      r("ul", { className: "study-list" },
+        addonStudies.map(study => (
+          r(StudyListItem, { key: study.name, study, translations })
+        ))
+      )
+    );
+  }
+}
+StudyList.propTypes = {
+  addonStudies: PropTypes.array.isRequired,
   translations: PropTypes.object.isRequired,
 };
 
-class SidebarItem extends React.Component {
+/**
+ * Details about an individual study, with an option to end it if it is active.
+ */
+class StudyListItem extends React.Component {
   constructor(props) {
     super(props);
-    this.handleClick = this.handleClick.bind(this);
+    this.handleClickRemove = this.handleClickRemove.bind(this);
   }
 
-  handleClick() {
-    window.location = `#${this.props.pageId}`;
+  handleClickRemove() {
+    sendPageEvent("RemoveStudy", { recipeId: this.props.study.recipeId, reason: "individual-opt-out" });
   }
 
   render() {
-    const { page, selected, translations } = this.props;
+    const { study, translations } = this.props;
     return (
       r("li", {
-        className: classnames("category", {selected}),
-        onClick: this.handleClick,
+        className: classnames("study", { disabled: !study.active }),
+        "data-study-name": study.name,
       },
-        page.icon && r("img", {className: "category-icon", src: page.icon}),
-        r("span", {className: "category-name"}, translations[page.name]),
-      )
-    );
-  }
-}
-SidebarItem.propTypes = {
-  pageId: PropTypes.string.isRequired,
-  page: PropTypes.shape({
-    icon: PropTypes.string,
-    name: PropTypes.string.isRequired,
-  }).isRequired,
-  selected: PropTypes.bool,
-  translations: PropTypes.object.isRequired,
-};
-
-class Content extends React.Component {
-  render() {
-    return (
-      r("div", {className: "main-content"},
-        r("div", {className: "content-box"},
-          this.props.children,
+        r("div", { className: "study-icon" },
+          study.name.slice(0, 1)
+        ),
+        r("div", { className: "study-details" },
+          r("div", { className: "study-name" }, study.name),
+          r("div", { className: "study-description", title: study.description },
+            r("span", { className: "study-status" }, study.active ? translations.activeStatus : translations.completeStatus),
+            r("span", {}, "\u2022"), // •
+            r("span", {}, study.description),
+          ),
+        ),
+        r("div", { className: "study-actions" },
+          study.active &&
+          r("button", { className: "remove-button", onClick: this.handleClickRemove },
+            r("div", { className: "button-box" },
+              translations.removeButton
+            ),
+          )
         ),
       )
     );
   }
 }
-Content.propTypes = {
-  children: PropTypes.node,
+StudyListItem.propTypes = {
+  study: PropTypes.shape({
+    recipeId: PropTypes.number.isRequired,
+    name: PropTypes.string.isRequired,
+    active: PropTypes.boolean,
+    description: PropTypes.string.isRequired,
+  }).isRequired,
+  translations: PropTypes.object.isRequired,
 };
 
-ReactDOM.render(
-  r(AboutStudies),
-  document.getElementById("app"),
-);
+ReactDOM.render(r(AboutStudies), document.getElementById("app"));

toolkit/components/normandy/content/about-studies/common.js

@@ -1,139 +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";
-/* eslint-disable no-unused-vars */
-/* global PropTypes React */
-
-/**
- * Shorthand for creating elements (to avoid using a JSX preprocessor)
- */
-const r = React.createElement;
-
-/**
- * Information box used at the top of listings.
- */
-window.InfoBox = class InfoBox extends React.Component {
-  render() {
-    return (
-      r("div", {className: "info-box"},
-        r("div", {className: "info-box-content"},
-          this.props.children,
-        ),
-      )
-    );
-  }
-};
-window.InfoBox.propTypes = {
-  children: PropTypes.node,
-};
-
-/**
- * Button using in-product styling.
- */
-window.FxButton = class FxButton extends React.Component {
-  render() {
-    return (
-      r("button", Object.assign({}, this.props, {children: undefined}),
-        r("div", {className: "button-box"},
-          this.props.children,
-        ),
-      )
-    );
-  }
-};
-window.FxButton.propTypes = {
-  children: PropTypes.node,
-};
-
-/**
- * Wrapper class for a value that is provided by the frame script.
- *
- * Emits a "GetRemoteValue:{name}" page event on load to fetch the initial
- * value, and listens for "ReceiveRemoteValue:{name}" page callbacks to receive
- * the value when it updates.
- *
- * @example
- * const myRemoteValue = new RemoteValue("MyValue", 5);
- * class MyComponent extends React.Component {
- *   constructor(props) {
- *     super(props);
- *     this.state = {
- *       myValue: null,
- *     };
- *   }
- *
- *   componentWillMount() {
- *     myRemoteValue.subscribe(this);
- *   }
- *
- *   componentWillUnmount() {
- *     myRemoteValue.unsubscribe(this);
- *   }
- *
- *   receiveRemoteValue(name, value) {
- *     this.setState({myValue: value});
- *   }
- *
- *   render() {
- *     return r("div", {}, this.state.myValue);
- *   }
- * }
- */
-class RemoteValue {
-  constructor(name, defaultValue = null) {
-    this.name = name;
-    this.handlers = [];
-    this.value = defaultValue;
-
-    document.addEventListener(`ReceiveRemoteValue:${this.name}`, this);
-    sendPageEvent(`GetRemoteValue:${this.name}`);
-  }
-
-  /**
-   * Subscribe to this value as it updates. Handlers are called with the current
-   * value immediately after subscribing.
-   * @param {Object} handler
-   *   Object with a receiveRemoteValue(name, value) method that is called with
-   *   the name and value of this RemoteValue when it is updated.
-   */
-  subscribe(handler) {
-    this.handlers.push(handler);
-    handler.receiveRemoteValue(this.name, this.value);
-  }
-
-  /**
-   * Remove a previously-registered handler.
-   * @param {Object} handler
-   */
-  unsubscribe(handler) {
-    this.handlers = this.handlers.filter(h => h !== handler);
-  }
-
-  handleEvent(event) {
-    this.value = event.detail;
-    for (const handler of this.handlers) {
-      handler.receiveRemoteValue(this.name, this.value);
-    }
-  }
-}
-
-/**
- * Collection of RemoteValue instances used within the page.
- */
-const remoteValues = {
-  studyList: new RemoteValue("StudyList"),
-  shieldLearnMoreHref: new RemoteValue("ShieldLearnMoreHref"),
-  studiesEnabled: new RemoteValue("StudiesEnabled"),
-  shieldTranslations: new RemoteValue("ShieldTranslations"),
-};
-
-/**
- * Dispatches a page event to the privileged frame script for this tab.
- * @param {String} action
- * @param {Object} data
- */
-function sendPageEvent(action, data) {
-  const event = new CustomEvent("ShieldPageEvent", {bubbles: true, detail: {action, data}});
-  document.dispatchEvent(event);
-}

toolkit/components/normandy/content/about-studies/shield-studies.js

@@ -1,207 +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";
-/* global classnames FxButton InfoBox PropTypes r React remoteValues sendPageEvent */
-
-window.ShieldStudies = class ShieldStudies extends React.Component {
-
-  render() {
-    const { translations } = this.props;
-
-    return (
-      r("div", {},
-        r(WhatsThisBox, {translations}),
-        r(StudyList, {translations}),
-      )
-    );
-  }
-};
-
-class UpdatePreferencesButton extends React.Component {
-  constructor(props) {
-    super(props);
-    this.handleClick = this.handleClick.bind(this);
-  }
-
-  handleClick() {
-    sendPageEvent("NavigateToDataPreferences");
-  }
-
-  render() {
-    return r(
-      FxButton,
-      Object.assign({
-        id: "shield-studies-update-preferences",
-        onClick: this.handleClick,
-      }, this.props),
-    );
-  }
-}
-
-class StudyList extends React.Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      studies: null,
-    };
-  }
-
-  componentDidMount() {
-    remoteValues.studyList.subscribe(this);
-  }
-
-  componentWillUnmount() {
-    remoteValues.studyList.unsubscribe(this);
-  }
-
-  receiveRemoteValue(name, value) {
-    if (value) {
-      const studies = value.slice();
-
-      // Sort by active status, then by start date descending.
-      studies.sort((a, b) => {
-        if (a.active !== b.active) {
-          return a.active ? -1 : 1;
-        }
-        return b.studyStartDate - a.studyStartDate;
-      });
-
-      this.setState({studies});
-    } else {
-      this.setState({studies: value});
-    }
-  }
-
-  render() {
-    const { studies } = this.state;
-    const { translations } = this.props;
-
-    if (studies === null) {
-      // loading
-      return null;
-    }
-
-    let info = null;
-    if (studies.length === 0) {
-      info = r("p", {className: "study-list-info"}, translations.noStudies);
-    }
-
-    return (
-      r("div", {},
-        info,
-        r("ul", {className: "study-list"},
-          this.state.studies.map(study => (
-            r(StudyListItem, {key: study.name, study, translations})
-          ))
-        ),
-      )
-    );
-  }
-}
-
-class StudyListItem extends React.Component {
-  constructor(props) {
-    super(props);
-    this.handleClickRemove = this.handleClickRemove.bind(this);
-  }
-
-  handleClickRemove() {
-    sendPageEvent("RemoveStudy", {recipeId: this.props.study.recipeId, reason: "individual-opt-out"});
-  }
-
-  render() {
-    const {study, translations} = this.props;
-    return (
-      r("li", {
-        className: classnames("study", {disabled: !study.active}),
-        "data-study-name": study.name,
-      },
-        r("div", {className: "study-icon"},
-          study.name.slice(0, 1)
-        ),
-        r("div", {className: "study-details"},
-          r("div", {className: "study-name"}, study.name),
-          r("div", {className: "study-description", title: study.description},
-            r("span", {className: "study-status"}, study.active ? translations.activeStatus : translations.completeStatus),
-            r("span", {}, "\u2022"), // •
-            r("span", {}, study.description),
-          ),
-        ),
-        r("div", {className: "study-actions"},
-          study.active &&
-            r(FxButton, {className: "remove-button", onClick: this.handleClickRemove}, translations.removeButton),
-        ),
-      )
-    );
-  }
-}
-StudyListItem.propTypes = {
-  study: PropTypes.shape({
-    recipeId: PropTypes.number.isRequired,
-    name: PropTypes.string.isRequired,
-    active: PropTypes.boolean,
-    description: PropTypes.string.isRequired,
-  }).isRequired,
-  translations: PropTypes.object.isRequired,
-};
-
-class WhatsThisBox extends React.Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      learnMoreHref: null,
-      studiesEnabled: null,
-    };
-  }
-
-  componentDidMount() {
-    remoteValues.shieldLearnMoreHref.subscribe(this);
-    remoteValues.studiesEnabled.subscribe(this);
-  }
-
-  componentWillUnmount() {
-    remoteValues.shieldLearnMoreHref.unsubscribe(this);
-    remoteValues.studiesEnabled.unsubscribe(this);
-  }
-
-  receiveRemoteValue(name, value) {
-    switch (name) {
-      case "ShieldLearnMoreHref": {
-        this.setState({ learnMoreHref: value });
-        break;
-      }
-      case "StudiesEnabled": {
-        this.setState({ studiesEnabled: value });
-        break;
-      }
-      default: {
-        console.error(`Unknown remote value ${name}`);
-      }
-    }
-  }
-
-  render() {
-    const { learnMoreHref, studiesEnabled } = this.state;
-    const { translations } = this.props;
-
-    let message = null;
-
-    // studiesEnabled can be null, in which case do nothing
-    if (studiesEnabled === false) {
-      message = r("span", {}, translations.disabledList);
-    } else if (studiesEnabled === true) {
-      message = r("span", {}, translations.enabledList);
-    }
-
-    const updateButtonKey = navigator.platform.includes("Win") ? "updateButtonWin" : "updateButtonUnix";
-
-    return (
-      r(InfoBox, {},
-        message,
-        r("a", {id: "shield-studies-learn-more", href: learnMoreHref}, translations.learnMore),
-        r(UpdatePreferencesButton, {}, translations[updateButtonKey]),
-       )
-    );
-  }
-}

toolkit/components/normandy/test/browser/browser_about_studies.js

@@ -183,7 +183,7 @@ decorate_task(
 
       await ContentTask.spawn(browser, null, async () => {
         const doc = content.document;
-        await ContentTaskUtils.waitForCondition(() => !!doc.querySelector(".info-box-content > span"));
+        await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span"));
 
         is(
           doc.querySelector(".info-box-content > span").textContent,
@@ -196,4 +196,4 @@ decorate_task(
       RecipeRunner.checkPrefs();
     }
   }
-);
+).only();