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

Addon knobs values aren't loaded from URL generated by "Copy" button in the knob panel. #6089

Closed
andyindahouse opened this issue Mar 14, 2019 · 9 comments

Comments

@andyindahouse
Copy link

Describe the bug
When you access to storybook from a URL generated with "Copy" button from knob panel, it doesn't set knobs like they were in the URL.

To Reproduce
Steps to reproduce the behaviour:

  1. Open storybooks-official.netlify 5.0.1
  2. Go to knobs panel and change some knobs like a "name" and "age", then tap "Copy" button.
  3. Open a new tab in your browser and paste the URL with knobs, enter.

Expected behavior:

The storybooks should load the knobs values that appear in the URL generated by the "Copy" button in the knob panel.

@storybook/addon-knobs 5.0.1

@andyindahouse andyindahouse changed the title Addon knobs values aren't loaded from URL generated by "Copy" button in the Knob panel. Addon knobs values aren't loaded from URL generated by "Copy" button in the knob panel. Mar 14, 2019
@shilman shilman added this to the 5.0.x milestone Mar 14, 2019
@shilman
Copy link
Member

shilman commented Mar 14, 2019

I fixed this during the beta but looks like this regressed during the course of the release. I'll dig in soon. Possibly a duplicate of #6075

@stale stale bot added the inactive label Apr 4, 2019
@shilman shilman removed the inactive label Apr 5, 2019
@stale stale bot added the inactive label Apr 26, 2019
@shilman
Copy link
Member

shilman commented Apr 26, 2019

This appears to be working in both the latest next and master

@stale stale bot removed the inactive label Apr 26, 2019
@willryan
Copy link

willryan commented May 8, 2019

@shilman I'm seeing Copy not work when the knob's name has a space in it (or any other character that needs to be URL-encoded). If you look in addons/knobs/src/components/Panel.js, you can see that the query param is set to knob-${name} in several places, without first escaping the name. Since the qs.stringify() call deliberately does not encode the query object, this leaves spaces and other invalid characters in the final URL that gets copied.

@storybookjs storybookjs deleted a comment from github-actions bot May 8, 2019
@storybookjs storybookjs deleted a comment from stale bot May 8, 2019
@storybookjs storybookjs deleted a comment from stale bot May 8, 2019
@leoyli
Copy link
Contributor

leoyli commented May 8, 2019

qs can do encoding for us just need an option flag encodeValuesOnly: true, but shareable URL is really tricky IMHO, likely to have edge cases to think about. And I think a better way it get namespaced like I did in addon-contexts since URL have max size (we can at least prevent it grows too fast).

@stale
Copy link

stale bot commented May 29, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label May 29, 2019
@shilman shilman modified the milestones: 5.0.x, 5.1.x Jun 5, 2019
@stale stale bot removed the inactive label Jun 5, 2019
@stale
Copy link

stale bot commented Jun 26, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@andyindahouse
Copy link
Author

This works, thanks for all.

@johnhunter
Copy link
Contributor

@willryan's comment is still true with v5.3.18

I'm seeing Copy not work when the knob's name has a space in it (or any other character that needs to be URL-encoded).

I have stories where the knob names have spaces. The url from the copy button fails as the spaces are not encoded. Replacing the spaces in the url with %20 and the url loads the expected state so its definitely a space encoding issue.

@johnhunter
Copy link
Contributor

Ah, so it works. The issue is pasting the url into other applications that expect urls to have spaces escaped (Slack, I'm looking at you). In all fairness there is no way handle links with spaces correctly when pasted as text.

Pasting links into other apps is a pretty common use case - it would be nice to resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants