Skip to content

Commit

Permalink
APM Storybook fixes (#68671) (#68713)
Browse files Browse the repository at this point in the history
* Resolve core legacy assets in @kbn/storybook webpack configuration
* Ignore stories in Jest coverage
* Combine effects in Cytoscape component so handlers are always added before events are triggered
* Add mock context to ErrorRateAlertTrigger stories
* Disable TransactionDurationAlertTrigger stories

Changing the Cytoscape effect behavior is necessary because the layout was not being triggered when the final set of elements is provided as props to the component. When this is used in Kibana we're always starting with empty elements and fetching them, but in the Storybook we're starting out with the full elements.
  • Loading branch information
smith authored Jun 9, 2020
1 parent 9a695e5 commit f78e790
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 24 deletions.
23 changes: 22 additions & 1 deletion packages/kbn-storybook/storybook_config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

const { resolve } = require('path');
const { parse, resolve } = require('path');
const webpack = require('webpack');
const { stringifyRequest } = require('loader-utils');
const CopyWebpackPlugin = require('copy-webpack-plugin');
Expand Down Expand Up @@ -95,6 +95,27 @@ module.exports = async ({ config }) => {
},
},
},
{
loader: 'resolve-url-loader',
options: {
// If you don't have arguments (_, __) to the join function, the
// resolve-url-loader fails with a loader misconfiguration error.
//
// eslint-disable-next-line no-unused-vars
join: (_, __) => (uri, base) => {
if (!base || !parse(base).dir.includes('legacy')) {
return null;
}

// URIs on mixins in src/legacy/public/styles need to be resolved.
if (uri.startsWith('ui/assets')) {
return resolve(REPO_ROOT, 'src/core/server/core_app/', uri.replace('ui/', ''));
}

return null;
},
},
},
{
loader: 'sass-loader',
options: {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/apm/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module.exports = {
collectCoverageFrom: [
'**/*.{js,jsx,ts,tsx}',
'!**/{__test__,__snapshots__,__examples__,integration_tests,tests}/**',
'!**/*.stories.{js,ts,tsx}',
'!**/*.test.{js,ts,tsx}',
'!**/dev_docs/**',
'!**/e2e/**',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ storiesOf('app/ServiceMap/Cytoscape', module).add(
},
},
];
return <Cytoscape elements={elements} height={300} width={1340} />;
return <Cytoscape elements={elements} height={600} width={1340} />;
},
{
info: { propTables: false, source: false },
Expand Down
15 changes: 5 additions & 10 deletions x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,6 @@ export function Cytoscape({

const trackApmEvent = useUiTracker({ app: 'apm' });

// Trigger a custom "data" event when data changes
useEffect(() => {
if (cy) {
cy.remove(cy.elements());
cy.add(elements);
cy.trigger('data');
}
}, [cy, elements]);

// Set up cytoscape event handlers
useEffect(() => {
const resetConnectedEdgeStyle = (node?: cytoscape.NodeSingular) => {
Expand Down Expand Up @@ -223,6 +214,10 @@ export function Cytoscape({
cy.on('mouseout', 'edge, node', mouseoutHandler);
cy.on('select', 'node', selectHandler);
cy.on('unselect', 'node', unselectHandler);

cy.remove(cy.elements());
cy.add(elements);
cy.trigger('data');
}

return () => {
Expand All @@ -241,7 +236,7 @@ export function Cytoscape({
}
clearTimeout(layoutstopDelayTimeout);
};
}, [cy, height, serviceName, trackApmEvent, width]);
}, [cy, elements, height, serviceName, trackApmEvent, width]);

return (
<CytoscapeContext.Provider value={cy}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
import { storiesOf } from '@storybook/react';
import React from 'react';
import { ErrorRateAlertTrigger } from '.';
import { ApmPluginContextValue } from '../../../context/ApmPluginContext';
import {
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../context/ApmPluginContext/MockApmPluginContext';

storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => {
const params = {
Expand All @@ -15,12 +20,16 @@ storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => {
};

return (
<div style={{ width: 400 }}>
<ErrorRateAlertTrigger
alertParams={params as any}
setAlertParams={() => undefined}
setAlertProperty={() => undefined}
/>
</div>
<MockApmPluginContextWrapper
value={(mockApmPluginContextValue as unknown) as ApmPluginContextValue}
>
<div style={{ width: 400 }}>
<ErrorRateAlertTrigger
alertParams={params as any}
setAlertParams={() => undefined}
setAlertProperty={() => undefined}
/>
</div>
</MockApmPluginContextWrapper>
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,24 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
// import { storiesOf } from '@storybook/react';
import { cloneDeep, merge } from 'lodash';
import { storiesOf } from '@storybook/react';
import React from 'react';
import { TransactionDurationAlertTrigger } from '.';
import { ApmPluginContextValue } from '../../../context/ApmPluginContext';
import {
MockApmPluginContextWrapper,
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../context/ApmPluginContext/MockApmPluginContext';
import { MockUrlParamsContextProvider } from '../../../context/UrlParamsContext/MockUrlParamsContextProvider';
import { ApmPluginContextValue } from '../../../context/ApmPluginContext';

storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => {
// Disabling this because we currently don't have a way to mock `useEnvironments`
// which is used by this component. Using the fetch-mock module should work, but
// our current storybook setup has core-js-related problems when trying to import
// it.
// storiesOf('app/TransactionDurationAlertTrigger', module).add('example',
// eslint-disable-next-line no-unused-expressions
() => {
const params = {
threshold: 1500,
aggregationType: 'avg' as const,
Expand Down Expand Up @@ -44,4 +50,4 @@ storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => {
</MockApmPluginContextWrapper>
</div>
);
});
};

0 comments on commit f78e790

Please sign in to comment.