From f4000e224f058edffd75894ae5f5643d88081c8d Mon Sep 17 00:00:00 2001 From: Les Orchard Date: Fri, 22 Sep 2017 14:58:17 -0400 Subject: [PATCH] Initial work toward new component bundle directories (#2859) * Tweaks to support component directory bundles - Storybook now also finds stories in frontend/src/app/**/stories.js - `npm run test` now also finds tests in frontend/src/app/**/tests.js - New /static/app/app.js.css stylesheet built from Sass styles imported by components - Hacks to ignore .scss imports when using component modules outside Webpack for static page build & tests - Update flowconfig to ignore .scss imports - Small eslint upgrade so CLI matches gulp-eslint * Reorganize ExperimentRowCard component into bundle directory * Reorganize UpdateList component into bundle directory * Begin to extract IncompatibleAddons component from ExperimentPage container * Rename stories.jsx -> stories.js; See also: https://github.com/airbnb/javascript/pull/985#issuecomment-239145468 * Quick & dirty revision to Storybook docs to incorporate new component bundle proposal Issue #2807 --- .eslintrc | 8 +- .storybook/config.js | 2 + .storybook/webpack.config.js | 23 +++ docs/development/storybook.md | 159 +++++++++++++++--- frontend/.flowconfig | 5 + .../index.js} | 6 +- .../components/ExperimentRowCard/stories.js} | 4 +- .../components/ExperimentRowCard/tests.js} | 17 +- .../components/IncompatibleAddons/index.js | 59 +++++++ .../components/IncompatibleAddons/index.scss | 2 + .../components/IncompatibleAddons/stories.js | 44 +++++ .../components/IncompatibleAddons/tests.js | 35 ++++ .../{UpdateList.js => UpdateList/index.js} | 4 +- .../app/components/UpdateList/stories.js} | 4 +- .../app/components/UpdateList/tests.js} | 9 +- frontend/src/app/containers/ExperimentPage.js | 47 +----- frontend/tasks/pages.js | 1 + frontend/test-setup.js | 3 + .../app/containers/ExperimentPage-test.js | 13 -- gulpfile.babel.js | 3 + package.json | 12 +- webpack.config.js | 23 ++- 22 files changed, 376 insertions(+), 107 deletions(-) create mode 100644 .storybook/webpack.config.js rename frontend/src/app/components/{ExperimentRowCard.js => ExperimentRowCard/index.js} (97%) rename frontend/{stories/app/components/ExperimentRowCard-story.jsx => src/app/components/ExperimentRowCard/stories.js} (95%) rename frontend/{test/app/components/ExperimentRowCard-test.js => src/app/components/ExperimentRowCard/tests.js} (95%) create mode 100644 frontend/src/app/components/IncompatibleAddons/index.js create mode 100644 frontend/src/app/components/IncompatibleAddons/index.scss create mode 100644 frontend/src/app/components/IncompatibleAddons/stories.js create mode 100644 frontend/src/app/components/IncompatibleAddons/tests.js rename frontend/src/app/components/{UpdateList.js => UpdateList/index.js} (97%) rename frontend/{stories/app/components/UpdateList-story.jsx => src/app/components/UpdateList/stories.js} (94%) rename frontend/{test/app/components/UpdateList-test.js => src/app/components/UpdateList/tests.js} (98%) diff --git a/.eslintrc b/.eslintrc index 3ce3c13682..32bb267f11 100644 --- a/.eslintrc +++ b/.eslintrc @@ -34,6 +34,8 @@ rules: comma-dangle: [2, never] indent: [2, 2, {SwitchCase: 1}] max-len: [0] + one-var: off + one-var-declaration-per-line: off no-console: 1 no-multi-spaces: [0] no-param-reassign: [0] @@ -44,4 +46,8 @@ rules: linebreak-style: [0] "import/no-extraneous-dependencies": - error - - { devDependencies: [ "./frontend/{stories,tasks,tests}/**/*.{js,jsx}" ] } + - + devDependencies: + - '**/tests.js' + - '**/stories.{js,jsx}' + - "./frontend/{stories,tasks,tests}/**/*.{js,jsx}" diff --git a/.storybook/config.js b/.storybook/config.js index cf24fab7bd..3932395745 100644 --- a/.storybook/config.js +++ b/.storybook/config.js @@ -6,9 +6,11 @@ import '../frontend/build/static/styles/experiments.css'; import '../frontend/build/static/styles/main.css'; const req = require.context('../frontend/stories', true, /\-story\.jsx?$/); +const reqInSrcTree = require.context('../frontend/src/app', true, /\/stories.jsx?$/); function loadStories() { req.keys().forEach((filename) => req(filename)); + reqInSrcTree.keys().forEach((filename) => reqInSrcTree(filename)); } configure(loadStories, module); diff --git a/.storybook/webpack.config.js b/.storybook/webpack.config.js new file mode 100644 index 0000000000..46a73495a5 --- /dev/null +++ b/.storybook/webpack.config.js @@ -0,0 +1,23 @@ +const path = require('path'); + +// Export a function. Accept the base config as the only param. +module.exports = (storybookBaseConfig, configType) => { + // configType has a value of 'DEVELOPMENT' or 'PRODUCTION' + // You can change the configuration based on that. + // 'PRODUCTION' is used when building the static version of storybook. + + // Make whatever fine-grained changes you need + storybookBaseConfig.module.rules.push({ + test: /\.s?css$/, + loaders: ["style-loader", "css-loader", "sass-loader"], + include: path.resolve(__dirname, '../') + }); + + storybookBaseConfig.module.rules.push({ + test: /\.(png|jpg|gif|svg)$/, + loaders: ['url-loader'] + }); + + // Return the altered config + return storybookBaseConfig; +}; diff --git a/docs/development/storybook.md b/docs/development/storybook.md index 172e3f3e60..32e6d7ce5e 100644 --- a/docs/development/storybook.md +++ b/docs/development/storybook.md @@ -26,26 +26,6 @@ All of these are conditions that are difficult or tedious to reproduce, so Storybook offers us a way to preview how components work without having to do the manual steps. -## Stories - -[The Storybook site has good documentation on writing stories][storydocs], -so we won't reproduce them here. - -Stories for Test Pilot are located at [`frontend/stories`][storydir]. The -directory structure mostly mirrors what's found under `frontend/src`. For -example, here's the path to the news updates feed and its associated -collection of stories: - -* [`frontend/src/app/components/UpdateList.js`](../frontend/src/app/components/UpdateList.js) -* [`frontend/stories/app/components/UpdateList-story.jsx`](../frontend/src/app/components/UpdateList.js) - -When making changes to a component, you should ensure that: -1. a module of stories for that component exists; and -1. that the module contains stories capturing the full range of your component's behavior. - -Storybook is a pretty new technology for us at the time of this writing, so -it's likely that new work will need to backfill both of the above. - ## Local development If you've got a local development environment working [per the quickstart @@ -97,3 +77,142 @@ Where `{git-commit-id}` is the hash of the latest commit to the branch. [storydir]: ../../frontend/stories/ [quickstart]: quickstart.md [repo]: https://github.com/mozilla/testpilot/ + +## Definitions + +- **Component** - an independent, reusable piece of UI. May contain additional components. +- **Story** - A variation of a component’s state, such as a hover, loading, or error state. +- **Container** - a component that is [`connect`](https://github.com/reactjs/react-redux/blob/master/docs/api.md#connectmapstatetoprops-mapdispatchtoprops-mergeprops-options)ed to the Redux store. +- **Dependent components** - components that are only used in the context of a parent component. For example, a `ListItem` component may only be used by a `List` component. + +## Changes to code + +In order to idiomatically accommodate a shift to use Storybook, substantial pieces of the Test Pilot site will be refactored. The following changes will be made: + +- Components will be refactored to be more self-containing (see Appendix 1). + - Dependent components will be defined in the same file as the parent component. + - The parent component should be the default export for the module, but dependent components should also be exported. + - CSS will be tightly coupled with components. + - Each component will have its own CSS file that is imported into the component source. + - Each component’s styles will be namespaced with a single CSS class, which will be used as a prefix for any children classes. + - Any reuse of styles will be implemented as SCSS mix-ins, rather than with utility CSS classes. + - Stories will be tightly coupled with components. + - Each component will have its own `stories.js` file in the component directory. + - That file will contain each story for the component itself, as well as any dependent components. + - Connected versions of container components will be their default export. + - Unconnected versions will also be exported, for use in stories. +- There will be a single top-level `` component that serves as the entry point to the application. It will contain any static components (such as the header and footer), and exactly one store-connected component for the current view. + - All connected components will live in `src/containers`. + - Each container will have its own `mapStateToProps` and `mapDispatchToProps` that provide the bare minimum state and actions required for the view to function, eliminate any potentially-expensive changes to the DOM from irrelevant changes to the store. + - All other components will live in `src/components`. + +## Changes to process + +This change will necessitate changes to both the deployment process and in how UX and engineering interact. + +### Deployment and release + +In addition to [other, unrelated changes](https://public.etherpad-mozilla.org/p/testpilot-new-branch-process) to our build and deployment processes, Storybook will be deployed for most commits to the `mozilla/testpilot` repository. + +It will work like this: + +1. For each commit on any branch of `mozilla/testpilot`, CircleCI will build the site. +2. If the tests pass, CI will deploy a static copy of the Storybook for that commit to a special S3 bucket. +3. If there is an associated pull request for the commit, a bot will comment on the pull requests with links to both the Storybook for the commit and for the latest commit for the pull request. + +A dashboard will be built with links to the Storybook for each pull request and commit. Deployed Storybooks will be pruned every 3 months. + +### UX and engineering + +With the UX and engineering teams on opposite sides of the globe, a process to work on UX changes or new features is designed to minimize back-and-forth between teams. It works like this: + +1. UX proposes new designs and delivers them will full-page mocks highlighting any implied changes. + - This includes any variations at smaller breakpoints (especially for containers). + - This includes a set of suggested states: hovers, errors, loading, etc. These do not need to be visually specified at this stage, just indicated as desired. +2. Engineering reviews the mocks, and delivers back to UX: + - A list of all existing components and stories that are changed by the mocks. + - A list of any new components implied by the mockups. + - A list of all the stories to be defined for each new component, including the ones suggested +3. UX provides redline-level specifications for each component and story that is either new or affected by the proposed change, in isolation. + - This will allow UX to create and maintain an accurate component of source files for each component. + - Each component and story should be clearly named. +4. Engineering builds the proposed components and stories and opens a pull request with them. + - The names of each component (i.e. `storiesOf(‘a component name’)` and story (`.add(‘story name’)` should match the ones provided in the UX source material for easy cross-referencing. + - The UX contact should be marked as a reviewer for the pull request. +5. UX reviews the storybook for the pull requests, requesting any appropriate changes before signing off on and merging the new feature. + +## Appendix 1: Example component structure. + +``` +src +└ containers +| └ homepage +| | └ index.js +| | └ index.scss +| | └ stories.js +| | └ tests.js +└ components +| └ install-button +| | └ index.js +| | └ index.scss +| | └ stories.js +| | └ tests.js +└ util.scss +``` + +### `components/install-button/index.js` + +``` javascript +import './index.scss'; + +export default class InstallButton extends Component { + renderIcon() { + const { icon } = this.props; + if (icon) { + return ( + + ); + } + return null; + } + + render() { + const { text } = this.props; + return ( + + ); + } +} +``` + +### `components/install-button/index.scss` + +``` css +@import '../../util.scss'; + +.install-button { + @include button(); + + background: #000; + color: #FFF; + + .install-button--icon { + display: inline-block; + margin-right: 4px; + } +} +``` + +### `containers/homepage/index.js` + +``` javascript +export class Homepage extends Component { + render() { + return

Hi!

+ } +} + +export default connect(Homepage)(mapStateToProps, mapDispatchToProps); +``` + + diff --git a/frontend/.flowconfig b/frontend/.flowconfig index 2f9a67ba3a..877195ee4d 100644 --- a/frontend/.flowconfig +++ b/frontend/.flowconfig @@ -11,4 +11,9 @@ [libs] ../flow-typed +[options] +module.file_ext=.scss +module.file_ext=.js +module.file_ext=.jsx +module.name_mapper='.*\.scss$' -> 'empty/object' diff --git a/frontend/src/app/components/ExperimentRowCard.js b/frontend/src/app/components/ExperimentRowCard/index.js similarity index 97% rename from frontend/src/app/components/ExperimentRowCard.js rename to frontend/src/app/components/ExperimentRowCard/index.js index 6c634e8564..5b83a4cc7c 100644 --- a/frontend/src/app/components/ExperimentRowCard.js +++ b/frontend/src/app/components/ExperimentRowCard/index.js @@ -4,11 +4,11 @@ import classnames from 'classnames'; import { Localized } from 'fluent-react/compat'; import React from 'react'; -import { buildSurveyURL, experimentL10nId } from '../lib/utils'; +import { buildSurveyURL, experimentL10nId } from '../../lib/utils'; -import type { InstalledExperiments } from '../reducers/addon'; +import type { InstalledExperiments } from '../../reducers/addon'; -import ExperimentPlatforms from './ExperimentPlatforms'; +import ExperimentPlatforms from '../ExperimentPlatforms'; const ONE_DAY = 24 * 60 * 60 * 1000; const ONE_WEEK = 7 * ONE_DAY; diff --git a/frontend/stories/app/components/ExperimentRowCard-story.jsx b/frontend/src/app/components/ExperimentRowCard/stories.js similarity index 95% rename from frontend/stories/app/components/ExperimentRowCard-story.jsx rename to frontend/src/app/components/ExperimentRowCard/stories.js index c470072cb5..57c7362161 100644 --- a/frontend/stories/app/components/ExperimentRowCard-story.jsx +++ b/frontend/src/app/components/ExperimentRowCard/stories.js @@ -4,8 +4,8 @@ import { storiesOf } from '@storybook/react'; import { action } from '@storybook/addon-actions'; import { withKnobs, object, boolean } from '@storybook/addon-knobs'; -import ExperimentRowCard from '../../../src/app/components/ExperimentRowCard'; -import LayoutWrapper from '../../../src/app/components/LayoutWrapper'; +import ExperimentRowCard from './index'; +import LayoutWrapper from '../LayoutWrapper'; const ONE_DAY = 24 * 60 * 60 * 1000; const ONE_WEEK = 7 * ONE_DAY; diff --git a/frontend/test/app/components/ExperimentRowCard-test.js b/frontend/src/app/components/ExperimentRowCard/tests.js similarity index 95% rename from frontend/test/app/components/ExperimentRowCard-test.js rename to frontend/src/app/components/ExperimentRowCard/tests.js index 0be95dd988..45d000a45f 100644 --- a/frontend/test/app/components/ExperimentRowCard-test.js +++ b/frontend/src/app/components/ExperimentRowCard/tests.js @@ -1,11 +1,12 @@ +/* global describe, beforeEach, it */ import React from 'react'; import { expect } from 'chai'; import sinon from 'sinon'; import { shallow } from 'enzyme'; -import { findLocalizedById } from '../util'; import moment from 'moment'; +import { findLocalizedById } from '../../../../test/app/util'; -import ExperimentRowCard from '../../../src/app/components/ExperimentRowCard'; +import ExperimentRowCard from './index'; describe('app/components/ExperimentRowCard', () => { let mockExperiment, mockClickEvent, props, subject; @@ -112,7 +113,7 @@ describe('app/components/ExperimentRowCard', () => { subject.setProps({ enabled: false, - getExperimentLastSeen: sinon.spy(experiment => moment().valueOf()) + getExperimentLastSeen: sinon.spy(() => moment().valueOf()) }); expect(props.getExperimentLastSeen.called).to.be.true; @@ -127,7 +128,7 @@ describe('app/components/ExperimentRowCard', () => { props = { ...props, enabled: false, getExperimentLastSeen: - sinon.spy(experiment => moment().subtract(1.5, 'week').valueOf()), + sinon.spy(() => moment().subtract(1.5, 'week').valueOf()), experiment: { ...mockExperiment, modified: moment().subtract(1, 'week').utc() } @@ -145,7 +146,7 @@ describe('app/components/ExperimentRowCard', () => { subject.setProps({ enabled: false, - getExperimentLastSeen: sinon.spy(experiment => moment().valueOf()) + getExperimentLastSeen: sinon.spy(() => moment().valueOf()) }); expect(props.getExperimentLastSeen.called).to.be.true; @@ -157,7 +158,7 @@ describe('app/components/ExperimentRowCard', () => { expect(findLocalizedById(subject, 'experimentListEndingTomorrow')).to.have.property('length', 0); subject.setProps({ experiment: { ...mockExperiment, completed: moment().add(23, 'hours').utc() - }}); + } }); expect(findLocalizedById(subject, 'experimentListEndingTomorrow')).to.have.property('length', 1); }); @@ -165,7 +166,7 @@ describe('app/components/ExperimentRowCard', () => { expect(findLocalizedById(subject, 'experimentListEndingSoon')).to.have.property('length', 0); subject.setProps({ experiment: { ...mockExperiment, completed: moment().add(6, 'days').utc() - }}); + } }); expect(findLocalizedById(subject, 'experimentListEndingSoon')).to.have.property('length', 1); }); @@ -198,7 +199,7 @@ describe('app/components/ExperimentRowCard', () => { hasAddon: true }); expect(findLocalizedById(subject, 'experimentCardManage')).to.have.property('length', 1); - }) + }); it('should ping GA when manage is clicked', () => { subject.find('.experiment-summary').simulate('click', mockClickEvent); diff --git a/frontend/src/app/components/IncompatibleAddons/index.js b/frontend/src/app/components/IncompatibleAddons/index.js new file mode 100644 index 0000000000..dab1fe1c5a --- /dev/null +++ b/frontend/src/app/components/IncompatibleAddons/index.js @@ -0,0 +1,59 @@ +// @flow + +import React from 'react'; +import { Localized } from 'fluent-react/compat'; +import LocalizedHtml from '../LocalizedHtml'; + +import './index.scss'; + +type IncompatibleAddonsProps = { + experiment: Object, + installedAddons: Array +}; + +export default class IncompatibleAddons extends React.Component { + props: IncompatibleAddonsProps + + render() { + const { incompatible } = this.props.experiment; + const installed = this.getIncompatibleInstalled(incompatible); + if (installed.length === 0) return null; + + const helpUrl = 'https://support.mozilla.org/kb/disable-or-remove-add-ons'; + + return ( +
+
+ +

+ This experiment may not be compatible with add-ons you have installed. +

+ + +

+ We recommend disabling these add-ons before activating this experiment: +

+ +
+
+
    + {installed.map(guid => ( +
  • {incompatible[guid]}
    • + ))} +
+
+
+ ); + } + + getIncompatibleInstalled(incompatible: Object) { + if (!incompatible) { + return []; + } + const installed = this.props.installedAddons || []; + return Object.keys(incompatible).filter(guid => ( + installed.indexOf(guid) !== -1 + )); + } + +} diff --git a/frontend/src/app/components/IncompatibleAddons/index.scss b/frontend/src/app/components/IncompatibleAddons/index.scss new file mode 100644 index 0000000000..c7ca1aa9a6 --- /dev/null +++ b/frontend/src/app/components/IncompatibleAddons/index.scss @@ -0,0 +1,2 @@ +// TODO: migrate styles from frontend/src/styles/components/_Warning.scss + diff --git a/frontend/src/app/components/IncompatibleAddons/stories.js b/frontend/src/app/components/IncompatibleAddons/stories.js new file mode 100644 index 0000000000..8f3f40728c --- /dev/null +++ b/frontend/src/app/components/IncompatibleAddons/stories.js @@ -0,0 +1,44 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; +import { action } from '@storybook/addon-actions'; +import IncompatibleAddons from './index'; +import LayoutWrapper from '../LayoutWrapper'; + +const experiment = { + title: 'Sample experiment', + description: 'This is an example experiment', + subtitle: '', + slug: 'snooze-tabs', + survey_url: 'https://example.com', + created: '2010-06-21T12:12:12Z', + modified: '2010-06-21T12:12:12Z', + incompatible: { + 'foo@bar.com': 'Foo from BarCorp' + } +}; + +const installedAddons = [ + 'foo@bar.com' +]; + +const baseProps = { + experiment, + installedAddons +}; + +storiesOf('IncompatibleAddons', module) + .addDecorator(story => +
+
+ + {story()} + +
+ ) + .add('base state', () => + + ) + .add('none installed', () => + + ) + ; diff --git a/frontend/src/app/components/IncompatibleAddons/tests.js b/frontend/src/app/components/IncompatibleAddons/tests.js new file mode 100644 index 0000000000..d170a22072 --- /dev/null +++ b/frontend/src/app/components/IncompatibleAddons/tests.js @@ -0,0 +1,35 @@ +/* global describe, beforeEach, it */ +import React from 'react'; +import { expect } from 'chai'; +import { shallow } from 'enzyme'; + +import IncompatibleAddons from './index.js'; + +describe('app/components/IncompatibleAddons', () => { + let mockExperiment, props, subject; + beforeEach(() => { + mockExperiment = { + slug: 'testing', + title: 'Testing', + incompatible: {} + }; + props = { + experiment: mockExperiment, + installedAddons: [] + }; + subject = shallow(); + }); + + it('should render a warning only if incompatible add-ons are installed', () => { + expect(subject.find('.incompatible-addons')).to.have.property('length', 0); + + const experiment = { ...mockExperiment, incompatible: { foo: 1, bar: 2 } }; + subject.setProps({ experiment }); + + subject.setProps({ installedAddons: ['baz'] }); + expect(subject.find('.incompatible-addons')).to.have.property('length', 0); + + subject.setProps({ installedAddons: ['baz', 'bar'] }); + expect(subject.find('.incompatible-addons')).to.have.property('length', 1); + }); +}); diff --git a/frontend/src/app/components/UpdateList.js b/frontend/src/app/components/UpdateList/index.js similarity index 97% rename from frontend/src/app/components/UpdateList.js rename to frontend/src/app/components/UpdateList/index.js index 363ceedb2d..4e378aad46 100644 --- a/frontend/src/app/components/UpdateList.js +++ b/frontend/src/app/components/UpdateList/index.js @@ -5,8 +5,8 @@ import { Localized } from 'fluent-react/compat'; import moment from 'moment'; import React from 'react'; -import LayoutWrapper from './LayoutWrapper'; -import { newsUpdateL10nId } from '../lib/utils'; +import LayoutWrapper from '../LayoutWrapper'; +import { newsUpdateL10nId } from '../../lib/utils'; export function prettyDate(date: string) { return moment(date).format('MMMM Do, YYYY'); diff --git a/frontend/stories/app/components/UpdateList-story.jsx b/frontend/src/app/components/UpdateList/stories.js similarity index 94% rename from frontend/stories/app/components/UpdateList-story.jsx rename to frontend/src/app/components/UpdateList/stories.js index 35561e1f12..f1e6494fa9 100644 --- a/frontend/stories/app/components/UpdateList-story.jsx +++ b/frontend/src/app/components/UpdateList/stories.js @@ -4,8 +4,8 @@ import { storiesOf } from '@storybook/react'; import { action } from '@storybook/addon-actions'; import { withKnobs } from '@storybook/addon-knobs'; -import UpdateList from '../../../src/app/components/UpdateList'; -import LayoutWrapper from '../../../src/app/components/LayoutWrapper'; +import UpdateList from './index'; +import LayoutWrapper from '../LayoutWrapper'; const time = Date.now(); diff --git a/frontend/test/app/components/UpdateList-test.js b/frontend/src/app/components/UpdateList/tests.js similarity index 98% rename from frontend/test/app/components/UpdateList-test.js rename to frontend/src/app/components/UpdateList/tests.js index a4d6c3f529..b3ef2c0efa 100644 --- a/frontend/test/app/components/UpdateList-test.js +++ b/frontend/src/app/components/UpdateList/tests.js @@ -1,8 +1,10 @@ +/* global describe, beforeEach, it */ import React from 'react'; import sinon from 'sinon'; import { expect } from 'chai'; import { shallow } from 'enzyme'; -import UpdateList, { Update, prettyDate } from '../../../src/app/components/UpdateList'; + +import UpdateList, { Update, prettyDate } from './index'; describe('app/components/UpdateList', () => { it('should display nothing if no updates are available', () => { @@ -73,7 +75,7 @@ describe('app/components/UpdateList', () => { { slug: 'foo', experimentSlug: 'exp1' }, { slug: 'bar' } ], - staleNewsUpdates: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].map(idx => ({ slug: `stale-${idx}` })), + staleNewsUpdates: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].map(idx => ({ slug: `stale-${idx}` })) }; const mockClickEvent = { @@ -94,7 +96,6 @@ describe('app/components/UpdateList', () => { }); describe('Update', () => { - it('should display expected basic content', () => { const subject = shallow( { eventLabel: `news-item-${slug}` }]); }); - }); - }); diff --git a/frontend/src/app/containers/ExperimentPage.js b/frontend/src/app/containers/ExperimentPage.js index 56494ced2d..12e82d1ba4 100644 --- a/frontend/src/app/containers/ExperimentPage.js +++ b/frontend/src/app/containers/ExperimentPage.js @@ -19,6 +19,7 @@ import ExperimentCardList from '../components/ExperimentCardList'; import ExperimentPreFeedbackDialog from '../components/ExperimentPreFeedbackDialog'; import View from '../components/View'; import Warning from '../components/Warning'; +import IncompatibleAddons from '../components/IncompatibleAddons'; import ExperimentPlatforms from '../components/ExperimentPlatforms'; import Banner from '../components/Banner'; @@ -126,48 +127,6 @@ export class ExperimentDetail extends React.Component { } } - getIncompatibleInstalled(incompatible) { - if (!incompatible) { - return []; - } - const installed = this.props.installedAddons || []; - return Object.keys(incompatible).filter(guid => ( - installed.indexOf(guid) !== -1 - )); - } - - renderIncompatibleAddons() { - const { incompatible } = this.props.experiment; - const installed = this.getIncompatibleInstalled(incompatible); - if (installed.length === 0) return null; - - const helpUrl = 'https://support.mozilla.org/kb/disable-or-remove-add-ons'; - - return ( -
-
- -

- This experiment may not be compatible with add-ons you have installed. -

- - -

- We recommend disabling these add-ons before activating this experiment: -

- -
-
-
    - {installed.map(guid => ( -
  • {incompatible[guid]}
    • - ))} -
-
-
- ); - } - renderLocaleWarning() { const { experiment, locale, hasAddon } = this.props; if (hasAddon !== null && locale && ((experiment.locales && !experiment.locales.includes(locale)) || (experiment.locale_blocklist && experiment.locale_blocklist.includes(locale)))) { @@ -210,7 +169,7 @@ export class ExperimentDetail extends React.Component { render() { const { experiment, experiments, installed, isAfterCompletedDate, isDev, - hasAddon, setExperimentLastSeen, clientUUID, + hasAddon, setExperimentLastSeen, clientUUID, installedAddons, setPageTitleL10N } = this.props; // Loading handled in static with react router; don't return anything if no experiments @@ -452,7 +411,7 @@ export class ExperimentDetail extends React.Component { {!graduated &&
{this.renderEolBlock()} - {this.renderIncompatibleAddons()} + {this.renderLocaleWarning()} {experiment.video_url && diff --git a/frontend/tasks/pages.js b/frontend/tasks/pages.js index 73b26148b4..c66035a41f 100644 --- a/frontend/tasks/pages.js +++ b/frontend/tasks/pages.js @@ -196,6 +196,7 @@ function generateStaticPage(prepareForClient, pageName, pageParam, component, { + diff --git a/frontend/test-setup.js b/frontend/test-setup.js index 80c1c10577..a9fbf14f96 100644 --- a/frontend/test-setup.js +++ b/frontend/test-setup.js @@ -1,5 +1,8 @@ // Global setup for all tests +// HACK: Ignore non-JS imports used for asset dependencies in Webpack +require.extensions['.scss'] = function () {} + // We need jsdom for enzyme mount()'ed components - mainly the sticky header // scroll handler stuff on the experiments page. diff --git a/frontend/test/app/containers/ExperimentPage-test.js b/frontend/test/app/containers/ExperimentPage-test.js index 6fa9327920..d84a2b50b6 100644 --- a/frontend/test/app/containers/ExperimentPage-test.js +++ b/frontend/test/app/containers/ExperimentPage-test.js @@ -248,19 +248,6 @@ describe('app/containers/ExperimentPage:ExperimentDetail', () => { expect(subject.find('ExperimentTourDialog')).to.have.property('length', 1); }); - it('should render a warning only if incompatible add-ons are installed', () => { - expect(subject.find('.incompatible-addons')).to.have.property('length', 0); - - const experiment = { ...mockExperiment, incompatible: { foo: 1, bar: 2 } }; - subject.setProps({ experiment }); - - subject.setProps({ installedAddons: [ 'baz' ] }); - expect(subject.find('.incompatible-addons')).to.have.property('length', 0); - - subject.setProps({ installedAddons: [ 'baz', 'bar' ] }); - expect(subject.find('.incompatible-addons')).to.have.property('length', 1); - }); - it('should display a call-to-action to install Test Pilot', () => { expect(subject.find('#testpilot-promo')).to.have.property('length', 1); expect(subject.find('MainInstallButton')).to.have.property('length', 1); diff --git a/gulpfile.babel.js b/gulpfile.babel.js index df985d293f..9fc5bd2ad7 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -1,6 +1,9 @@ /* eslint-disable import/no-extraneous-dependencies*/ require('babel-polyfill'); +// HACK: Ignore non-JS imports used for asset dependencies in Webpack +require.extensions['.scss'] = function () {} + const gulp = require('gulp'); const config = require('./frontend/config.js'); diff --git a/package.json b/package.json index c2b507a04b..2d99fba57d 100644 --- a/package.json +++ b/package.json @@ -55,14 +55,16 @@ "cross-spawn": "^5.1.0", "del": "2.2.2", "doctoc": "1.3.0", + "empty": "0.10.1", "enzyme": "2.7.1", - "eslint": "3.17.1", + "eslint": "3.19.0", "eslint-config-airbnb": "10.0.1", "eslint-config-react": "1.1.7", "eslint-plugin-flowtype": "2.30.0", "eslint-plugin-import": "2.2.0", "eslint-plugin-jsx-a11y": "5.0.3", "eslint-plugin-react": "7.0.1", + "extract-text-webpack-plugin": "3.0.0", "feed": "1.1.0", "fetch-mock": "5.9.4", "flow-bin": "0.47.0", @@ -95,11 +97,13 @@ "require-globify": "1.4.1", "run-sequence": "1.2.2", "sass-lint": "1.10.2", + "sass-loader": "6.0.6", "sinon": "2.0.0", "through2": "2.0.3", "uglifyjs-webpack-plugin": "^0.4.6", + "url-loader": "^0.5.9", "vinyl-source-stream": "1.1.0", - "webpack": "^3.5.5", + "webpack": "3.5.6", "webpack-bundle-analyzer": "^2.9.0", "webpack-stream": "^4.0.0", "yamljs": "0.2.8" @@ -122,11 +126,11 @@ "static": "gulp dist-build", "docs": "doctoc README.md && doctoc addon/README.md", "lint": "gulp scripts-lint styles-lint", - "test": "cd frontend && mocha --require babel-register --require test-setup.js --recursive", + "test": "cd frontend && mocha --require babel-register --require test-setup.js --recursive 'test' 'src/**/tests.js'", "flow": "flow frontend", "l10n:extract": "gulp content-extract-strings", "l10n:check": "npm run l10n:extract && git diff-files --quiet --ignore-all-space locales && echo 'No missing strings'", - "test:ci": "cross-env NODE_ENV=test nyc mocha --recursive --reporter mocha-junit-reporter --require frontend/test-setup.js frontend/test", + "test:ci": "cross-env NODE_ENV=test nyc mocha --recursive --reporter mocha-junit-reporter --require frontend/test-setup.js frontend/test 'frontend/src/**/tests.js'", "test:watch": "npm test -- --watch", "test:all": "npm run lint && npm run flow && npm run test && npm run flow-coverage && cd addon && npm run lint && npm run test && tox -e ui-tests", "coverage": "cross-env NODE_ENV=test nyc mocha --recursive --require frontend/test-setup.js frontend/test", diff --git a/webpack.config.js b/webpack.config.js index 29a98dfc8f..ad6171a02f 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -7,6 +7,7 @@ const config = require('./frontend/config.js'); const UglifyJSPlugin = require('uglifyjs-webpack-plugin'); const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin; +const ExtractTextPlugin = require('extract-text-webpack-plugin'); const RUN_ANALYZER = !!process.env.ANALYZER; const NODE_ENV = process.env.NODE_ENV || 'development'; @@ -35,6 +36,10 @@ const vendorModules = Object.keys(packageJSON.dependencies) .filter(name => excludeVendorModules.indexOf(name) < 0) .concat(includeVendorModules); +const extractSass = new ExtractTextPlugin({ + filename: '[name].css' +}); + const plugins = [ new webpack.DefinePlugin({ 'process.env.NODE_ENV': `"${NODE_ENV}"`, @@ -46,7 +51,8 @@ const plugins = [ /moment[\/\\]locale$/, // eslint-disable-line no-useless-escape new RegExp(config.AVAILABLE_LOCALES.replace(/,/g, '|')) ), - new webpack.optimize.CommonsChunkPlugin('static/app/vendor.js') + new webpack.optimize.CommonsChunkPlugin('static/app/vendor.js'), + extractSass ]; if (!IS_DEV) { @@ -91,12 +97,23 @@ module.exports = { contentBase: 'dist' }, devtool: 'source-map', + resolve: { + extensions: ['.js', '.jsx'] + }, module: { - loaders: [ + rules: [ { test: /\.jsx?$/, exclude: /node_modules/, - loader: 'babel-loader' + use: 'babel-loader' + }, + { + test: /\.scss$/, + use: extractSass.extract({ + use: [{ loader: 'css-loader' }, { loader: 'sass-loader' }], + // use style-loader in development + fallback: 'style-loader' + }) } ] },