Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Canvas] Storybook Redux Addon #73227

Merged

Conversation

clintandrewhall
Copy link
Contributor

Summary

This PR is something I've been excited about for weeks, and now that its dependencies are committed, I can finally create it: a Redux store and inspector for Canvas Storybook!

Screen Shot 2020-07-26 at 1 03 40 AM

Screen Shot 2020-07-26 at 1 03 47 AM

I'll be meeting with @streamich and others on Monday to discuss how to enable these changes in kbn-storybook so we can effectively migrate ASAP.

What is this?

This addon provides a decorator that will surround a component with a custom Redux provider, one that logs the store changes to an Addon Panel in Storybook. This means:

  1. We can create stories that test Redux components.
  2. Dependency components will work out of the box.
  3. We can create custom stores for each test.
  4. We can directly inspect the effect of a component action in the addon panel.

How do I use it?

In your story definition:

import { reduxDecorator, getAddonPanelParameters } from '../../../../storybook';

storiesOf('my/redux/component', module)
  .addDecorator(reduxDecorator())            // Adds the redux store
  .addParameters(getAddonPanelParameters())  // Optional: sets Storybook to open 
                                             // the addon by default

The reduxDecorator can (currently) accept three state settings:

  1. workpad: an entire workpad
  2. elements: custom elements for the workpad
  3. assets: compatible assets

Feel free to contribute more as necessary...!

Addon Development

Want to work on or improve the addon? Add the --addon flag to the script to launch the addon in watch mode. Refreshing the page should enable any changes:

canvas: node scripts/storybook --addon

What's in the PR?

  1. Upgrades to Storybook dependencies (v5.3.19)
  2. Upgrades and refactors to our Storybook config to be cleaner and clearer.
  3. The new addon :-)
  4. Updates to the Asset/AssetManager test cases.

What did I need to ship in order to ship this?

This takes advantage of a number of changes from my Time to Present PoC. It also needed to use the improvements @crob611 and I made to how we use Kibana Services-- we can now stub out and run Canvas without any Kibana Services present.

@clintandrewhall clintandrewhall added review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.10.0 labels Jul 26, 2020
@clintandrewhall clintandrewhall requested review from spalger, streamich and a team July 26, 2020 05:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas (Team:Canvas)

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The move to decorators for storybooks LGTM. I'm struggling to compare the rewritten webpack config but I'm assuming that what you have works which means it's not a problem. That said, I do recommend updating it to use webpack-merge vs a bunch of single-word variables.

x-pack/plugins/canvas/storybook/webpack.config.js Outdated Show resolved Hide resolved
@clintandrewhall
Copy link
Contributor Author

@spalger I'll use webpack-merge... I'm building a set of improvements to kbn-storybook and want it to be flexible.

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@clintandrewhall clintandrewhall changed the title [Canvas] Storybook Redux Addon and Upgrade [Canvas] Storybook Redux Addon Jul 30, 2020
Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
canvas 1.4MB -11.0B 1.4MB

page load bundle size

id value diff baseline
canvas 1.4MB +89.0B 1.4MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@clintandrewhall clintandrewhall merged commit 4f18f1d into elastic:master Aug 1, 2020
@clintandrewhall clintandrewhall deleted the canvas-storybook-addon branch August 1, 2020 11:28
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Aug 1, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx
clintandrewhall added a commit that referenced this pull request Aug 1, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 2, 2020
* master: (39 commits)
  [Canvas][tech-debt] Rename __examples__ to __stories__ (elastic#73853)
  [Canvas] Storybook Redux Addon (elastic#73227)
  Use "Apply_filter_trigger" in "explore underlying data" action (elastic#71445)
  [maps] convert top nav config to TS (elastic#73851)
  [maps] fix fit to bounds for ES document layers with joins (elastic#73985)
  [Canvas][tech-debt] Refactor Toolbar (completes Kill Recompose.pure) (elastic#73309)
  [CI] In-progress Slack notifications (elastic#74012)
  [SIEM][Detection Engine] Fixes tags to accept characters such as AND, OR, (, ), ", * (elastic#74003)
  [SECURITY_SOLUTION][ENDPOINT] Fix host list Configuration Status cell link loosing list page/size state (elastic#73989)
  Tweak injected metadata (elastic#73990)
  Closes elastic#73998 by using `canAccessML` in the ML capabilities API to (elastic#73999)
  [SIEM] Fixes toaster errors when siemDefault index is an empty or empty spaces (elastic#73991)
  [Security Solution] Fix timeline pin event callback (elastic#73981)
  [Security Solution] Fix unexpected redirect (elastic#73969)
  [Metrics UI] Fix Metrics Explorer TSVB link to use workaround pattern (elastic#73986)
  [APM] docs: Update machine learning integration (elastic#73597)
  [Ingest Manager] Fix limited concurrency helper (elastic#73976)
  [build/sysv] fix missing env variable rename (elastic#73977)
  Fix a typo. (elastic#73948)
  [Ingest Manager] Revert fleet config concurrency rollout to rate limit (elastic#73940)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants