From e9487b11c944b14fd346e78b239493f5dec20a15 Mon Sep 17 00:00:00 2001 From: Michael Cooper Date: Tue, 11 Sep 2018 23:48:58 +0000 Subject: [PATCH] Bug 1447499 - Simplify about:studies code r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D5480 --HG-- extra : moz-landing-system : lando --- .../content/about-studies/about-studies.css | 8 +- .../content/about-studies/about-studies.html | 2 - .../content/about-studies/about-studies.js | 238 ++++++++++-------- .../normandy/content/about-studies/common.js | 139 ---------- .../content/about-studies/shield-studies.js | 207 --------------- .../test/browser/browser_about_studies.js | 4 +- 6 files changed, 143 insertions(+), 455 deletions(-) delete mode 100644 toolkit/components/normandy/content/about-studies/common.js delete mode 100644 toolkit/components/normandy/content/about-studies/shield-studies.js diff --git a/toolkit/components/normandy/content/about-studies/about-studies.css b/toolkit/components/normandy/content/about-studies/about-studies.css index d52f797b9c34..14c44caece2f 100644 --- a/toolkit/components/normandy/content/about-studies/about-studies.css +++ b/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; diff --git a/toolkit/components/normandy/content/about-studies/about-studies.html b/toolkit/components/normandy/content/about-studies/about-studies.html index edb02a95f0f4..0de72d4b186f 100644 --- a/toolkit/components/normandy/content/about-studies/about-studies.html +++ b/toolkit/components/normandy/content/about-studies/about-studies.html @@ -17,8 +17,6 @@ - - diff --git a/toolkit/components/normandy/content/about-studies/about-studies.js b/toolkit/components/normandy/content/about-studies/about-studies.js index dc5d63b61b28..dfaba1b34aeb 100644 --- a/toolkit/components/normandy/content/about-studies/about-studies.js +++ b/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")); diff --git a/toolkit/components/normandy/content/about-studies/common.js b/toolkit/components/normandy/content/about-studies/common.js deleted file mode 100644 index b2b25077d6e9..000000000000 --- a/toolkit/components/normandy/content/about-studies/common.js +++ /dev/null @@ -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); -} diff --git a/toolkit/components/normandy/content/about-studies/shield-studies.js b/toolkit/components/normandy/content/about-studies/shield-studies.js deleted file mode 100644 index 010ebe9de401..000000000000 --- a/toolkit/components/normandy/content/about-studies/shield-studies.js +++ /dev/null @@ -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]), - ) - ); - } -} diff --git a/toolkit/components/normandy/test/browser/browser_about_studies.js b/toolkit/components/normandy/test/browser/browser_about_studies.js index 96dc8a268893..d0098dad3094 100644 --- a/toolkit/components/normandy/test/browser/browser_about_studies.js +++ b/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();