Add URL query param feature #6601

leoyli · 2019-04-24T01:31:06Z

Issue: #6602

What I did

[feature] query param feature.
[type] types to match with its typed function name.
[doc] quickly document the new feature in README.md.
[test] add tests to cover the newly added serializers.
[chore] clean up: rename file structure.

How to test

Story running under default contexts

Story running under preselected contexts set by URL query params

Known bugs

The iframe URL is not get updated unless you switch from tabs (i.e. the iframe opening url is out of sync), which is the same problem as in the addon-knob.
Perform api.setQueryParams({ contexts: null }): nothing happened, but based on the doc this query param should be removed.

Will file issues later to track it.

vercel · 2019-04-24T01:31:13Z

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-add-addon-contextsurl-q.storybook.now.sh

shilman · 2019-04-24T02:21:10Z

Possibly related to the known bugs #6075

@tmeasday is looking into that issue

leoyli · 2019-04-24T02:37:51Z

Sorry for some drastic files movement, but I think this can make the source code easier to follow... And I quickly add 2 tests to cover the new pure functions. I think the feature is good to go. Let me see if we can push this addon to Angular community as the next. 💪

shilman

Looking great @leoyli 👏

I wonder if there are some types & patterns of this that would be useful for other addons as well (e.g. the createAddonDecorator) @ndelangen @tmeasday

leoyli · 2019-04-24T02:43:54Z

@shilman, I believe there are some pattern that potential can be further extracted to make cross framework addon development a breeze. I would be happy to share or draft a post to discuss on it. So far it seems each addon came up its unique way (and I came up with my own... but hope people think this could be easy to understand and follow).

leoyli · 2019-04-24T03:00:20Z

I guess many people starts to get no-cycle linter errors due to eslint-plugin-import v1.7.2. The feature was added last year but get finally fixed and running out-of-box. It's a nice linting though. Here is the error when I run it on my local machine:

/Volumes/Data/GitHub/storybook/addons/info/src/components/types/ArrayOf.js
  1:1  error  Unused eslint-disable directive (no problems were reported from 'import/no-cycle')

/Volumes/Data/GitHub/storybook/addons/info/src/components/types/ObjectOf.js
  1:1  error  Unused eslint-disable directive (no problems were reported from 'import/no-cycle')

/Volumes/Data/GitHub/storybook/addons/info/src/components/types/OneOfType.js
  1:1  error  Unused eslint-disable directive (no problems were reported from 'import/no-cycle')

/Volumes/Data/GitHub/storybook/addons/info/src/components/types/PrettyPropType.js
  1:1  error  Unused eslint-disable directive (no problems were reported from 'import/no-cycle')

/Volumes/Data/GitHub/storybook/addons/info/src/components/types/Shape.js
  1:1  error  Unused eslint-disable directive (no problems were reported from 'import/no-cycle')

shilman · 2019-04-24T03:03:30Z

@leoyli I just pushed a fix for the linting error. Hopefully it passes CI. 🙏

shilman · 2019-04-24T03:05:00Z

@leoyli BTW can you create an issue in the storybook repo and reference that instead using the Issue: #xxx format? that way the issue will be closed & commented on when i release this improvement

leoyli · 2019-04-24T03:09:24Z

@shilman, do you mean open an issue for the linting error or copy the original issue into this repo?!

shilman · 2019-04-24T03:12:23Z

@leoyli copy the original issue into this repo

…g, and package.json

codecov · 2019-04-24T05:12:14Z

Codecov Report

Merging #6601 into next will increase coverage by 0.02%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##             next    #6601      +/-   ##
==========================================
+ Coverage   40.67%   40.69%   +0.02%     
==========================================
  Files         633      633              
  Lines        8674     8686      +12     
  Branches      620      624       +4     
==========================================
+ Hits         3528     3535       +7     
- Misses       5057     5061       +4     
- Partials       89       90       +1

Impacted Files	Coverage Δ
...contexts/src/manager/components/ToolbarControl.tsx	`0% <ø> (ø)`
addons/contexts/src/manager/components/ToolBar.tsx	`0% <ø> (ø)`
addons/contexts/src/shared/constants.ts	`100% <ø> (ø)`
...ns/contexts/src/manager/components/ToolBarMenu.tsx	`0% <ø> (ø)`
...exts/src/manager/components/ToolBarMenuOptions.tsx	`0% <ø> (ø)`
addons/contexts/src/manager/libs/useChannel.ts	`0% <ø> (ø)`	⬆️
addons/contexts/src/preview/frameworks/react.ts	`0% <0%> (ø)`	⬆️
addons/contexts/src/register.ts	`0% <0%> (ø)`	⬆️
addons/contexts/src/preview/ContextsPreviewAPI.ts	`0% <0%> (ø)`
addons/contexts/src/manager/ContextsManager.tsx	`0% <0%> (ø)`
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a71a0b...d66aa30. Read the comment docs.

codecov · 2019-04-24T05:12:14Z

Codecov Report

Merging #6601 into next will increase coverage by 0.03%.
The diff coverage is 30.3%.

@@            Coverage Diff            @@
##             next   #6601      +/-   ##
=========================================
+ Coverage   40.67%   40.7%   +0.03%     
=========================================
  Files         633     633              
  Lines        8674    8687      +13     
  Branches      620     625       +5     
=========================================
+ Hits         3528    3536       +8     
- Misses       5057    5062       +5     
  Partials       89      89

Impacted Files	Coverage Δ
addons/contexts/src/manager/components/ToolBar.tsx	`0% <ø> (ø)`
addons/contexts/src/shared/constants.ts	`100% <ø> (ø)`
...ns/contexts/src/manager/components/ToolBarMenu.tsx	`0% <ø> (ø)`
...exts/src/manager/components/ToolBarMenuOptions.tsx	`0% <ø> (ø)`
addons/contexts/src/manager/libs/useChannel.ts	`0% <ø> (ø)`	⬆️
...contexts/src/manager/components/ToolbarControl.tsx	`0% <0%> (ø)`
addons/contexts/src/preview/frameworks/react.ts	`0% <0%> (ø)`	⬆️
addons/contexts/src/register.ts	`0% <0%> (ø)`	⬆️
addons/contexts/src/preview/ContextsPreviewAPI.ts	`0% <0%> (ø)`
addons/contexts/src/manager/ContextsManager.tsx	`0% <0%> (ø)`
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a71a0b...d2704db. Read the comment docs.

leoyli · 2019-04-24T06:39:55Z

I have rebased the branch, and patch d66aa30, b178d68 for fixing the potential bugs due to exposing the state via URL query param. This avoid any inconsistency (as far as I can catch).

shilman · 2019-04-24T06:50:17Z

@leoyli build failed in addons/contexts:

$ node ../../scripts/prepare.js
src/shared/serializers.ts(21,3): error TS2322: Type 'string[]' is not assignable to type 'SelectionState'.
  Index signature is missing in type 'string[]'.

leoyli · 2019-04-24T07:04:38Z

@shilman, learned a lesson on .reduce type inference... sometimes it failed in inferring the type, especially when its initial value is null or undefined; then I have to manually specify it 👀.

leoyli · 2019-04-24T13:26:30Z

Going to merge it since CD/CI are happy and all my paranoid checks and bug hunting have been done! 💪

leoyli · 2019-04-24T16:14:18Z

Related to #6602.

leoyli added feature request addon: contexts labels Apr 24, 2019

leoyli added this to the 5.1.0 milestone Apr 24, 2019

leoyli requested review from shilman and ndelangen April 24, 2019 01:31

leoyli self-assigned this Apr 24, 2019

vercel bot deployed to staging April 24, 2019 01:35 View deployment

vercel bot deployed to staging April 24, 2019 02:04 View deployment

storybookjs deleted a comment from codecov bot Apr 24, 2019

vercel bot deployed to staging April 24, 2019 02:34 View deployment

storybookjs deleted a comment from codecov bot Apr 24, 2019

shilman approved these changes Apr 24, 2019

View reviewed changes

leoyli mentioned this pull request Apr 24, 2019

Add URL queries for controlling contexts in the SB full iFrame mode #6602

Closed

Leo Y. Li added 7 commits April 24, 2019 01:11

UPDATE types to match with its typed function name

e9ba4a3

CLEANUP

efbc1ab

UPDATE addon-contexts: file structure, unnecessary deprecation warnin…

98877a1

…g, and package.json

ADD addon-contexts: query param feature

e512c9f

REVERT wrong IDE refactoring

3989801

CLEANUP addon-contexts: match up file structure

90ed045

UPDATE addon-contexts: README.md

2df5ff6

Leo Y. Li added 2 commits April 24, 2019 01:11

ADD addon-contexts: tests on serializers

06e057e

FIX addon-contexts: edge case bugs

d66aa30

leoyli force-pushed the add/addon-contexts/url-query-param branch from d8ca8e0 to d66aa30 Compare April 24, 2019 05:12

vercel bot deployed to staging April 24, 2019 05:12 View deployment

FIX addon-contexts: edge case bugs part.2

b178d68

vercel bot had a problem deploying to staging April 24, 2019 06:39 Failure

FIX addon-contexts: fix build fail due to the failed type inference

d2704db

vercel bot deployed to staging April 24, 2019 07:00 View deployment

leoyli merged commit 97fbbc0 into next Apr 24, 2019

leoyli deleted the add/addon-contexts/url-query-param branch April 24, 2019 13:26

leoyli restored the add/addon-contexts/url-query-param branch April 24, 2019 13:29

leoyli deleted the add/addon-contexts/url-query-param branch April 24, 2019 13:29

leoyli restored the add/addon-contexts/url-query-param branch April 24, 2019 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add URL query param feature #6601

Add URL query param feature #6601

leoyli commented Apr 24, 2019 •

edited by shilman

Loading

vercel bot commented Apr 24, 2019 •

edited

Loading

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman left a comment

leoyli commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

codecov bot commented Apr 24, 2019

codecov bot commented Apr 24, 2019 •

edited

Loading

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019 •

edited

Loading

leoyli commented Apr 24, 2019

leoyli commented Apr 24, 2019

Add URL query param feature #6601

Add URL query param feature #6601

Conversation

leoyli commented Apr 24, 2019 • edited by shilman Loading

What I did

How to test

Story running under default contexts

Story running under preselected contexts set by URL query params

Known bugs

vercel bot commented Apr 24, 2019 • edited Loading

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman left a comment

Choose a reason for hiding this comment

leoyli commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

codecov bot commented Apr 24, 2019

Codecov Report

codecov bot commented Apr 24, 2019 • edited Loading

Codecov Report

leoyli commented Apr 24, 2019

shilman commented Apr 24, 2019

leoyli commented Apr 24, 2019 • edited Loading

leoyli commented Apr 24, 2019

leoyli commented Apr 24, 2019

leoyli commented Apr 24, 2019 •

edited by shilman

Loading

vercel bot commented Apr 24, 2019 •

edited

Loading

codecov bot commented Apr 24, 2019 •

edited

Loading

leoyli commented Apr 24, 2019 •

edited

Loading