JS Objects in Knobs addon #1460

z4o4z · 2017-07-12T15:51:32Z

Issue:
#1187 #1186

What I did

Added JS Objects support to Knobs addon
Added react-ace as code editor

How to test

npm run storybook - look at 'with object' story

add support for JS Objects (#1186) add react-ace as code editor for object type (#1187)

shilman · 2017-07-12T15:57:35Z

@z4o4z looks cool! will check it out later today. in the meantime how big (KB) is the ace-editor/brace dependency?

z4o4z · 2017-07-13T08:46:07Z

@shilman Hi, all new libs(react-ace, brace, brace/mode/javascript, brace/theme/github) have size about 710kb (not gzip)

danielduan

Love the ability to read a knobs object. it's been really annoying seeing [object],[object]

I'll leave it up to others whether the editor is worth adding to our codebase.

danielduan · 2017-07-13T16:27:57Z

addons/knobs/src/components/types/Object.js

-    const { knob } = this.props;
+const getSpaces = level => Array.from({ length: level * 2 }, () => ' ').join('');
+
+function toString(obj, spaceLevel = 1) {


is there a reason why were manually converting objects? seems kind of fragile.

we can probably just do json.stringify and then pass it through some js beautifier or insert spaces ourselves.

kind of an edge case, but if there's a circular dependency somewhere, this would hang too.
i.e.: const obj = { key: this }

Hi @danielduan . I didn't use json.stringify because I tried to add support for functions. In future we can add support for other JS features(regexp, Map, Set, and etc)

makes sense. thanks @z4o4z

shilman · 2017-07-15T20:40:09Z

@tmeasday @ndelangen any thoughts on this re: code bloat with the new editor?

shilman · 2017-07-15T21:38:50Z

@z4o4z Finally had a chance to test this out. Love ❤️ the Object support and the function support is really funky and cool.

I also think:

Examples should be added to examples/cra-kitchen-sink so we can see how the different addons interact in a single spot (which we'll run automated tests against soon).
We should either get rid of the addon-specific example OR add lerna-support for them somehow (cc @ndelangen)
We should split the react-ace work into a separate PR and not merge it until we have a better use case for it. For example @wcastand was working on support for a full code editor addon. (I am very concerned about Storybook getting bigger and slower since 3.0, and am very sensitive to taking on big dependencies, and think we're probably overdue for a performance milestone soon.)

What do you think? I'm happy to do some of this work, or load-balance.

shilman · 2017-07-15T21:46:39Z

For @ndelangen

z4o4z · 2017-07-20T08:42:04Z

Hi @shilman, Excellent suggestions! I will add demos to examples/cra-kitchen-sink.
And can you help me with it?

split the react-ace work into a separate PR

ndelangen · 2017-07-28T08:21:49Z

minor linting error:

/tmp/storybook/addons/knobs/src/components/types/Object.js
  56:14  error    Arrow function should not return assignment               no-return-assign

ndelangen · 2017-08-03T10:21:33Z

@z4o4z Can you take another look at this? I think the snapshots are outdated!

z4o4z · 2017-08-03T20:23:13Z

@ndelangen yep

stale · 2017-10-31T08:19:24Z

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. We do try to do some housekeeping every once in a while so inactive issues will get closed after 90 days. Thanks!

Hypnosphi · 2017-10-31T11:30:43Z

addons/knobs/src/components/types/Object.js

  }),
  onChange: PropTypes.func,
 };

-ObjectType.serialize = object => JSON.stringify(object);
-ObjectType.deserialize = value => (value ? JSON.parse(value) : {});
+ObjectType.deserialize = value => eval(`(${value})`); // eslint-disable-line no-eval


We need some XSS protection here

Are we afraid of users scripting themselves or a knob url link with something potentially malicious inside it?

I think we can get around this by not allowing the javascript to be serialized into the URL if it's a real concern.

Yeah, mostly the latter. Storybooks are often hosted at some path of the main website, so you can make cookies leak this way

🎖 for you @Hypnosphi

Actually it shouldn't be deserialized from URL, either. And it would be a regression comparing to what we have now

Maybe we should try to do plain JSON.parse first, and just give up if it fails AND the value comes from URL

danielduan · 2017-11-15T02:56:52Z

@z4o4z do you think you can remove the js object serialization so we can move this PR forward?

Hypnosphi

See #1460 (comment)

stale · 2017-12-31T00:08:04Z

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 60 days. Thanks!

stale · 2018-01-15T00:29:08Z

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

z4o4z added 4 commits June 9, 2017 14:29

fix prop types

94e46d4

Merge branch 'master' into knobs-code-mirror

8c11961

add support for JS Objects

13768d6

add support for JS Objects (#1186) add react-ace as code editor for object type (#1187)

Merge branch 'master' into knobs-code-mirror

d2c5afd

danielduan approved these changes Jul 13, 2017

View reviewed changes

ndelangen added addon: knobs feature request labels Jul 15, 2017

ndelangen and others added 2 commits July 20, 2017 00:06

Merge branch 'master' into knobs-code-mirror

02f4034

Merge branch 'master' into knobs-code-mirror

c59beff

ndelangen and others added 3 commits July 24, 2017 00:51

Merge branch 'master' into knobs-code-mirror

52d5355

Merge branch 'master' into knobs-code-mirror

b8048e8

Merge branch 'master' into knobs-code-mirror

ce8941c

z4o4z and others added 5 commits August 1, 2017 14:35

Merge branch 'master' into knobs-code-mirror

d684f4c

add more examples

3bb7c63

fix eslint

b6900a4

Merge branch 'master' into knobs-code-mirror

0bb674a

Merge branch 'master' into knobs-code-mirror

f7d9343

Merge branch 'master' into knobs-code-mirror

83ca3bd

stale bot added the inactive label Oct 31, 2017

Hypnosphi reviewed Oct 31, 2017

View reviewed changes

stale bot removed the inactive label Oct 31, 2017

Hypnosphi mentioned this pull request Oct 31, 2017

[addon/knobs] JSX Support #1580

Closed

storybookjs deleted a comment from storybookbot Nov 14, 2017

storybookjs deleted a comment from codecov bot Nov 14, 2017

danielduan mentioned this pull request Nov 15, 2017

knob array of objects not working as expected #2310

Closed

Hypnosphi requested changes Nov 15, 2017

View reviewed changes

stale bot added the inactive label Dec 31, 2017

stale bot closed this Jan 15, 2018

Hypnosphi deleted the knobs-code-mirror branch February 17, 2018 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JS Objects in Knobs addon #1460

JS Objects in Knobs addon #1460

z4o4z commented Jul 12, 2017

shilman commented Jul 12, 2017 •

edited

Loading

z4o4z commented Jul 13, 2017

danielduan left a comment

danielduan Jul 13, 2017

z4o4z Jul 13, 2017

danielduan Jul 13, 2017

shilman commented Jul 15, 2017

shilman commented Jul 15, 2017

shilman commented Jul 15, 2017

z4o4z commented Jul 20, 2017

ndelangen commented Jul 28, 2017

ndelangen commented Aug 3, 2017

z4o4z commented Aug 3, 2017

stale bot commented Oct 31, 2017

Hypnosphi Oct 31, 2017

danielduan Nov 14, 2017

Hypnosphi Nov 14, 2017 •

edited

Loading

ndelangen Nov 15, 2017

Hypnosphi Nov 15, 2017 •

edited

Loading

danielduan commented Nov 15, 2017

Hypnosphi left a comment

stale bot commented Dec 31, 2017

stale bot commented Jan 15, 2018

JS Objects in Knobs addon #1460

JS Objects in Knobs addon #1460

Conversation

z4o4z commented Jul 12, 2017

What I did

How to test

shilman commented Jul 12, 2017 • edited Loading

z4o4z commented Jul 13, 2017

danielduan left a comment

Choose a reason for hiding this comment

danielduan Jul 13, 2017

Choose a reason for hiding this comment

z4o4z Jul 13, 2017

Choose a reason for hiding this comment

danielduan Jul 13, 2017

Choose a reason for hiding this comment

shilman commented Jul 15, 2017

shilman commented Jul 15, 2017

shilman commented Jul 15, 2017

z4o4z commented Jul 20, 2017

ndelangen commented Jul 28, 2017

ndelangen commented Aug 3, 2017

z4o4z commented Aug 3, 2017

stale bot commented Oct 31, 2017

Hypnosphi Oct 31, 2017

Choose a reason for hiding this comment

danielduan Nov 14, 2017

Choose a reason for hiding this comment

Hypnosphi Nov 14, 2017 • edited Loading

Choose a reason for hiding this comment

ndelangen Nov 15, 2017

Choose a reason for hiding this comment

Hypnosphi Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

danielduan commented Nov 15, 2017

Hypnosphi left a comment

Choose a reason for hiding this comment

stale bot commented Dec 31, 2017

stale bot commented Jan 15, 2018

shilman commented Jul 12, 2017 •

edited

Loading

Hypnosphi Nov 14, 2017 •

edited

Loading

Hypnosphi Nov 15, 2017 •

edited

Loading