Skip to content

Commit

Permalink
[7.x] [Canvas] Storybook for testing and development (#29072) (#34329)
Browse files Browse the repository at this point in the history
* [Canvas] Storybook for testing and development (#29072)

This PR adds [Storybook](https://storybook.js.org/) to our testing and development suite.

![screen shot 2019-01-21 at 4 35 32 pm](https://user-images.githubusercontent.com/297604/51502196-9f856780-1d9a-11e9-97bf-07c99c3f279b.png)

This will allow us to:

1. create a site outlining all components within Canvas, including their TS type information;
2. demonstrate usage of all components by example;
3. allow for individual component testing, both manually and by Jest;
4. iterate and fix bugs on individual components *without* having to start up Kibana, in a [HMR](https://webpack.js.org/concepts/hot-module-replacement/) environment;
5. automatically generate [snapshots](https://jestjs.io/docs/en/snapshot-testing) based on any examples written;

This PR also converts a few components to Typescript and adds examples.

I was inspired to add this when I was fixing #25342.  In order to fix my changes, I had to run elasticsearch and kibana, as well as refresh my page whenever I needed to test a change.  Had I had a Storybook instance, I would have been done much faster.

In this PR, you'll see I converted `AdvancedFilter` from `renderers` and `FontPicker` and `ImageUpload` from `public/components`.  Would you believe I discovered and fixed bugs just by converting to Typescript and writing examples?

- `onChange` and `commit` are not marked as required in `propTypes`, but the component will error out if they're not supplied.
- `commit` was actually being called twice when 'Apply' was clicked.  This was shown in the 'Actions' panel when I was testing it.

- The `fonts` collection was not strongly-typed, therefore any string could be passed to the `value` parameter without error.
- While the code allows for any font string to be given to the component, there is no way to currently select that value, nor type it in within the control.  This is likely a bug in design.
- The `aria-labeledby` attribute in the drop down includes `undefined`.  This is likely a bug in EUI:

![screen shot 2019-01-21 at 4 25 58 pm](https://user-images.githubusercontent.com/297604/51501908-5ed91e80-1d99-11e9-913a-ce1bb5f4e352.png)

- `cd x-pack/plugins/canvas/`
- Run `node scripts/storybook` to start up a local development version, with HMR.
- Run `node scripts/storybook_build` to build a complete static version of the book.
- Run `node scripts/jest` which will run the Storyshots test; run `node scripts/jest --updateSnapshot` if source code has changed as expected.

- Adding Jest coverage and output to the info panels, ([this](https://www.npmjs.com/package/@storybook/addon-jest) is *sick* functionality).
- Adding automatic [a11y testing](https://www.npmjs.com/package/@storybook/addon-a11y), (currently [blocked](storybookjs/storybook#4889)).
- Adding generic knobs for stories
- Adding more example info, (e.g. who edited last, descriptions, etc).

* Update based on errors in CI

* Fix mismerge of dependencies

* Fix yarn.lock as well
  • Loading branch information
clintandrewhall authored Apr 2, 2019
1 parent d577e6f commit aecaffc
Show file tree
Hide file tree
Showing 24 changed files with 4,183 additions and 2,361 deletions.
3,764 changes: 1,609 additions & 2,155 deletions packages/kbn-pm/dist/index.js

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions x-pack/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
/.env
/.kibana-plugin-helpers.dev.*
!/plugins/infra/**/target
.cache

# We don't want any yarn.lock files in here
/yarn.lock
12 changes: 12 additions & 0 deletions x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@
"@kbn/expect": "1.0.0",
"@kbn/plugin-helpers": "9.0.2",
"@kbn/test": "1.0.0",
"@storybook/addon-actions": "^4.1.7",
"@storybook/addon-console": "^1.1.0",
"@storybook/addon-info": "^4.0.7",
"@storybook/addon-knobs": "^4.0.7",
"@storybook/addon-options": "^4.1.7",
"@storybook/addon-storyshots": "^4.1.7",
"@storybook/react": "^4.0.7",
"@types/angular": "1.6.50",
"@types/cheerio": "^0.22.10",
"@types/d3-array": "^1.2.1",
Expand All @@ -56,6 +63,9 @@
"@types/recompose": "^0.30.2",
"@types/reduce-reducers": "^0.1.3",
"@types/sinon": "^5.0.1",
"@types/storybook__addon-actions": "^3.4.1",
"@types/storybook__addon-info": "^3.4.2",
"@types/storybook__react": "^4.0.0",
"@types/supertest": "^2.0.5",
"@types/uuid": "^3.4.4",
"abab": "^1.0.4",
Expand All @@ -78,6 +88,7 @@
"enzyme-adapter-react-16": "^1.9.0",
"enzyme-adapter-utils": "^1.10.0",
"enzyme-to-json": "^3.3.4",
"execa": "^1.0.0",
"fancy-log": "^1.3.2",
"fetch-mock": "7.3.0",
"graphql-code-generator": "^0.13.0",
Expand Down Expand Up @@ -111,6 +122,7 @@
"sass-loader": "^7.1.0",
"simple-git": "1.37.0",
"sinon": "^5.0.7",
"string-replace-loader": "^2.1.1",
"supertest": "^3.1.0",
"supertest-as-promised": "^4.0.2",
"tmp": "0.0.31",
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/canvas/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,7 @@ public/style/index.css
canvas_plugin/*

# Don't commit the Webpack statistics
webpack_stats.json
webpack_stats.json

# Don't commit storybook builds
storybook
9 changes: 9 additions & 0 deletions x-pack/plugins/canvas/.storybook/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"env": {
"test": {
"plugins": [
"require-context-hook"
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { compose, withState } from 'recompose';
import { AdvancedFilter as Component } from './advanced_filter';

export const AdvancedFilter = compose(withState('value', 'onChange', ({ filter }) => filter || ''))(
Component
);
import '@storybook/addon-options/register';
import '@storybook/addon-actions/register';
import '@storybook/addon-knobs/register';
import '@storybook/addon-console';
65 changes: 65 additions & 0 deletions x-pack/plugins/canvas/.storybook/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { configure, addDecorator } from '@storybook/react';
import { withKnobs } from '@storybook/addon-knobs/react';
import { withInfo } from '@storybook/addon-info';
import { withOptions } from '@storybook/addon-options';

// Import dependent CSS
require('@elastic/eui/dist/eui_theme_light.css');
require('@kbn/ui-framework/dist/kui_light.css');
require('../../../../src/legacy/ui/public/styles/bootstrap_light.less');

// If we're running Storyshots, be sure to register the require context hook.
// Otherwise, add the other decorators.
if (process.env.NODE_ENV === 'test') {
require('babel-plugin-require-context-hook/register')();
} else {
// Customize the info for each story.
addDecorator(
withInfo({
inline: true,
styles: {
infoBody: {
margin: 20
},
infoStory: {
margin: '40px 60px'
}
}
})
);

// Add optional knobs to customize each story.
addDecorator(withKnobs);
}

function loadStories() {
// Pull in the built CSS produced by the Kibana server
const css = require.context('../../../../built_assets/css', true, /light.css$/);
css.keys().forEach(filename => css(filename));

// Include the legacy styles
const uiStyles = require.context('../../../../src/legacy/ui/public/styles', false, /[\/\\](?!mixins|variables|_|\.|bootstrap_(light|dark))[^\/\\]+\.less/);
uiStyles.keys().forEach(key => uiStyles(key));

// Find all files ending in *.examples.ts
const req = require.context('./..', true, /.examples.tsx$/);
req.keys().forEach(filename => req(filename));
}

// Set up the Storybook environment with custom settings.
addDecorator(
withOptions({
goFullScreen: false,
name: 'Canvas Storybook',
showAddonsPanel: true,
url: 'https://github.com/elastic/kibana/tree/master/x-pack/plugins/canvas'
})
);

configure(loadStories, module);
13 changes: 13 additions & 0 deletions x-pack/plugins/canvas/.storybook/middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

const serve = require('serve-static');
const path = require('path');

// Extend the Storybook Middleware to include a route to access ui assets
module.exports = function (router) {
router.get('/ui', serve(path.resolve(__dirname, '../../../../src/legacy/ui/public/assets')));
}
18 changes: 18 additions & 0 deletions x-pack/plugins/canvas/.storybook/storyshots.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import path from 'path';
import initStoryshots, { multiSnapshotWithOptions } from '@storybook/addon-storyshots';
import styleSheetSerializer from 'jest-styled-components/src/styleSheetSerializer';
import { addSerializer } from 'jest-specific-snapshot'

jest.mock(`@elastic/eui/lib/components/form/form_row/make_id`, () => () => `generated-id`);

addSerializer(styleSheetSerializer);
initStoryshots({
configPath: path.resolve(__dirname, './../.storybook'),
test: multiSnapshotWithOptions({}),
});
74 changes: 74 additions & 0 deletions x-pack/plugins/canvas/.storybook/webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

const path = require('path');
const TSDocgenPlugin = require('react-docgen-typescript-webpack-plugin');

// Extend the Storybook Webpack config with some customizations;
module.exports = (_baseConfig, _env, config) => {
// Include the React preset for Storybook JS files.
config.module.rules.push({
test: /\.js$/,
include: /\.storybook/,
loaders: 'babel-loader',
options: {
presets: [require.resolve('@kbn/babel-preset/webpack_preset')],
},
});

// Find and alter the CSS rule to replace the Kibana public path string with a path
// to the route we've added in middleware.js
const cssRule = config.module.rules.find(rule => rule.test.source.includes('.css$'));
cssRule.use.push({
loader: 'string-replace-loader',
options: {
search: '__REPLACE_WITH_PUBLIC_PATH__',
replace: '/',
flags: 'g',
},
});

// Configure loading LESS files from Kibana
config.module.rules.push({
test: /\.less$/,
use: [
{ loader: 'style-loader' },
{ loader: 'css-loader', options: { importLoaders: 2 } },
{
loader: 'postcss-loader',
options: {
config: { path: path.resolve(__dirname, './../../../../src/optimize/postcss.config.js') },
},
},
{ loader: 'less-loader' },
],
});

// Support .ts/x files using the tsconfig from Kibana
config.module.rules.push({
test: /\.tsx?$/,
use: [
{
loader: 'babel-loader',
options: {
presets: [require.resolve('@kbn/babel-preset/webpack_preset')],
},
},
require.resolve('react-docgen-typescript-loader'),
],
});

// Include the TSDocgen plugin to display Typescript param comments in the stories.
config.plugins.push(new TSDocgenPlugin());

// Tell Webpack about the ts/x extensions
config.resolve.extensions.push('.ts', '.tsx');

// Alias the any imports from ui/ to the proper directory.
config.resolve.alias.ui = path.resolve(__dirname, './../../../../src/legacy/ui/public');

return config;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Storyshots renderers/AdvancedFilter default 1`] = `
<form
className="canvasAdvancedFilter"
onSubmit={[Function]}
>
<div
className="euiFlexGroup euiFlexGroup--gutterExtraSmall euiFlexGroup--directionRow euiFlexGroup--responsive"
>
<div
className="euiFlexItem"
>
<input
className="canvasAdvancedFilter__input"
onChange={[Function]}
placeholder="Enter filter expression"
type="text"
value=""
/>
</div>
<div
className="euiFlexItem euiFlexItem--flexGrowZero"
>
<button
className="canvasAdvancedFilter__button"
type="submit"
>
Apply
</button>
</div>
</div>
</form>
`;

exports[`Storyshots renderers/AdvancedFilter with value 1`] = `
<form
className="canvasAdvancedFilter"
onSubmit={[Function]}
>
<div
className="euiFlexGroup euiFlexGroup--gutterExtraSmall euiFlexGroup--directionRow euiFlexGroup--responsive"
>
<div
className="euiFlexItem"
>
<input
className="canvasAdvancedFilter__input"
onChange={[Function]}
placeholder="Enter filter expression"
type="text"
value="expression"
/>
</div>
<div
className="euiFlexItem euiFlexItem--flexGrowZero"
>
<button
className="canvasAdvancedFilter__button"
type="submit"
>
Apply
</button>
</div>
</div>
</form>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { action } from '@storybook/addon-actions';
import { storiesOf } from '@storybook/react';
import React from 'react';
import { AdvancedFilter } from './advanced_filter';

storiesOf('renderers/AdvancedFilter', module)
.add('default', () => <AdvancedFilter onChange={action('onChange')} commit={action('commit')} />)
.add('with value', () => (
<AdvancedFilter onChange={action('onChange')} commit={action('commit')} value="expression" />
));
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,20 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import PropTypes from 'prop-types';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import PropTypes from 'prop-types';
import React, { SFC } from 'react';

export interface Props {
/** Optional value for the component */
value?: string;
/** Function to invoke when the filter value is changed */
onChange: (value: string) => void;
/** Function to invoke when the filter value is committed */
commit: (value: string) => void;
}

export const AdvancedFilter = ({ value, onChange, commit }) => (
export const AdvancedFilter: SFC<Props> = ({ value = '', onChange, commit }) => (
<form
onSubmit={e => {
e.preventDefault();
Expand All @@ -27,20 +36,20 @@ export const AdvancedFilter = ({ value, onChange, commit }) => (
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<button
className="canvasAdvancedFilter__button"
type="submit"
onClick={() => commit(value)}
>
<button className="canvasAdvancedFilter__button" type="submit">
Apply
</button>
</EuiFlexItem>
</EuiFlexGroup>
</form>
);

AdvancedFilter.defaultProps = {
value: '',
};

AdvancedFilter.propTypes = {
onChange: PropTypes.func,
onChange: PropTypes.func.isRequired,
value: PropTypes.string,
commit: PropTypes.func,
commit: PropTypes.func.isRequired,
};
Loading

0 comments on commit aecaffc

Please sign in to comment.