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

Use Storybook Controls instead of Knobs #80705

Merged
merged 5 commits into from Oct 21, 2020
Merged

Conversation

smith
Copy link
Contributor

@smith smith commented Oct 15, 2020

  • Change an example in embeddable to use controls instead of knobs
  • Add controls to SyncBadge APM story
  • Convert both to CSF
  • Remove the Knobs addon from the default Storybook configuration

Do not remove the Knobs addon package, since Canvas is still using it and this does not change anything in Canvas.

* Change an example in embeddable to use controls instead of knobs
* Add controls to SyncBadge APM story
* Convert both to [CSF](https://storybook.js.org/docs/react/api/csf)
* Remove the Knobs addon from the default Storybook configuration

Do not remove the Knobs addon package, since Canvas is still using it and this does not change anything in Canvas.
@smith smith added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Oct 15, 2020
@smith smith requested review from a team as code owners October 15, 2020 16:42
@botelastic botelastic bot added Feature:Embedding Embedding content via iFrame Team:APM All issues that need APM UI Team support labels Oct 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

import { action } from '@storybook/addon-actions';
import { withKnobs, boolean } from '@storybook/addon-knobs';
import * as React from 'react';
Copy link
Contributor

@ogupte ogupte Oct 15, 2020

Choose a reason for hiding this comment

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

we can use the default import hereimport React from 'react';. It shouldn't change anything else in this file. or we can copy what you did in SyncBadge.stories.tsx (e.g. import React, { ComponentProps } from 'react';)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VSCode auto-formatted this for me so I thought maybe this plugin had some lint rules around this. @elastic/kibana-app-arch let me know if there's a preference for react imports here.

@smith
Copy link
Contributor Author

smith commented Oct 21, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@smith smith merged commit 99e90aa into elastic:master Oct 21, 2020
@smith smith deleted the nls/sb-controls branch October 21, 2020 20:44
smith added a commit to smith/kibana that referenced this pull request Oct 21, 2020
* Change an example in embeddable to use controls instead of knobs
* Add controls to SyncBadge APM story
* Convert both to [CSF](https://storybook.js.org/docs/react/api/csf)
* Remove the Knobs addon from the default Storybook configuration

Do not remove the Knobs addon package, since Canvas is still using it and this does not change anything in Canvas.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 22, 2020
* master: (63 commits)
  [KP] Fix Headers timeout issue (elastic#81140)
  [ML] Functional tests - stabilize typing with checks service method (elastic#81338)
  tabify - support docs (elastic#80351)
  [Security Solution][Detections] Look-back time logic fix (elastic#81383)
  [Workplace Search] Add top-level tests for Groups (elastic#81215)
  [Fleet] Fix agent action observable for long polling (elastic#81376)
  [Maps] fix feature tooltip remains open when zoom level change hides layer (elastic#81373)
  skip flaky suite (elastic#78689)
  chore(NA): add spec-to-console and plugin-helpers as devOnly dependencies (elastic#81357)
  Ensure some data is returned (elastic#81375)
  Change dumb-init to tini (elastic#81126)
  [Reporting/Tech Debt] Convert PdfMaker class to TypeScript (elastic#81242)
  Use Storybook Controls instead of Knobs (elastic#80705)
  [junit] make sure that report paths are unique (elastic#81255)
  bump elastic/elasticsearch-js version to 7.10.0-rc1 (elastic#81288)
  run ssl tests on CI (elastic#81320)
  Fix alert defaults (elastic#81207)
  [ML] DF Analytics wizard: ensure user can set mml manually or select to use given estimate (elastic#81078)
  Add UI notifier to indicate secret fields and to remember / reenter values (elastic#80657)
  [Monitoring] Use async/await (elastic#81200)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 23, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 27, 2020
smith added a commit that referenced this pull request Oct 27, 2020
* Change an example in embeddable to use controls instead of knobs
* Add controls to SyncBadge APM story
* Convert both to [CSF](https://storybook.js.org/docs/react/api/csf)
* Remove the Knobs addon from the default Storybook configuration

Do not remove the Knobs addon package, since Canvas is still using it and this does not change anything in Canvas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants