mozilla · lmorchard · Sep 22, 2017 · Sep 15, 2017 · Sep 15, 2017 · Sep 15, 2017
diff --git 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
@@ -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
@@ -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
@@ -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 `<App>` 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 (
+        <span className={`install-button--icon install-button--icon--${icon}`} />
+      );
+    }
+    return null;
+  }
+
+  render() {
+    const { text } = this.props;
+    return (
+      <button className="install-button">{this.renderIcon()}{text}</button>
+    );
+  }
+}
+```
+
+### `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 <p>Hi!</p>
+  }
+}
+
+export default connect(Homepage)(mapStateToProps, mapDispatchToProps);
+```
+
+
diff --git 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/...d/src/app/components/ExperimentRowCard.js → ...app/components/ExperimentRowCard/index.js b/...d/src/app/components/ExperimentRowCard.js → ...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/...pp/components/ExperimentRowCard-story.jsx → ...p/components/ExperimentRowCard/stories.js b/...pp/components/ExperimentRowCard-story.jsx → ...p/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/.../app/components/ExperimentRowCard-test.js → ...app/components/ExperimentRowCard/tests.js b/.../app/components/ExperimentRowCard-test.js → ...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,15 +158,15 @@ 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);
   });
 
   it('should show a "soon" status message when ending in one week', () => {
     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
@@ -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<string>
+};
+
+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 (
+      <section className="incompatible-addons">
+        <header>
+          <Localized id="incompatibleHeader">
+            <h3>
+              This experiment may not be compatible with add-ons you have installed.
+            </h3>
+          </Localized>
+          <LocalizedHtml id="incompatibleSubheader">
+            <p>
+              We recommend <a href={helpUrl}>disabling these add-ons</a> before activating this experiment:
+            </p>
+          </LocalizedHtml>
+        </header>
+        <main>
+          <ul>
+            {installed.map(guid => (
+              <li key={guid}>{incompatible[guid]}</li>
+            ))}
+          </ul>
+        </main>
+      </section>
+    );
+  }
+
+  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
@@ -0,0 +1,2 @@
+// TODO: migrate <IncompatibleAddons> styles from frontend/src/styles/components/_Warning.scss
+
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		// TODO: migrate <IncompatibleAddons> styles from frontend/src/styles/components/_Warning.scss