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-controls: Add JSON tree editor for Object/Array Type args #12824

Conversation

hydrosquall
Copy link
Contributor

@hydrosquall hydrosquall commented Oct 20, 2020

Issue: #12599 , reimplements #12685

What I did

  • Replaced the existing text form control with the react-editable-json lib (public demo here: https://oxyno-zeta.github.io/react-editable-json-tree/ ). I also added typings for the library to the closest typings file so we can safely use it in typescript.

  • Made a first pass at customizing the styles (spacing and colors) based on @domyen's feedback here and looking at the colors available in Storybook here . The "dark/light mode toggles" aren't toggling the styles in the control pane (just inside the main component body area), so I'm not exactly how this will look in Dark Mode yet, but I set up the style customization function such that it should be easy to replace things

  • Updated the official-storybook story about using addon-controls to make use of the fact that we now handle nested objects.

  • Demo GIF: https://a.cl.ly/geuqpZrq

How to test

  • Is this testable with Jest or Chromatic screenshots?

It's possible, but I'm not sure that it's necessary.

  • Does this need a new example in the kitchen sink apps?

We could, but I think updating the main storybook story may be sufficient, as this change should benefit most users without needing them to customize anything.

  • Does this need an update to the documentation?

We could update this page to indicate that tree view is used instead of a text view.

FAQ

  • Why not some other JSON tree viewer library XYZ?

    • We explored several different alternatives, tradeoffs are documented here, here, and here
  • What terminal commands did you use that you might use again next time?

# to add a dependency scoped to 1 package 
yarn lerna add react-editable-json-tree --scope=@storybook/components

# to test locally
yarn build components --watch
yarn start
  • Any modifications useful in dev that you didn't commit?

1 change in examples/official-storybook/main.ts to speed up build time:

{
stories: [
    './stories/**/addon-controls.stories.@(js|ts|tsx|mdx)',
  ]
 // ... rest
}

@hydrosquall
Copy link
Contributor Author

I'm not sure how to debug the CI failures because I don't have access to any of the TeamCity URLs in the Github status checks. I can look into any issues, but I wasn't able to pick up any glaring issues in my diff, since this runs without error when I build locally. Is there anything else I should do before this can be reviewed?

cc @shilman

@shilman
Copy link
Member

shilman commented Oct 23, 2020

Don't worry about it. Sorry i missed this PR first time around. Looking amazing! I'll test and review later today 🙏

@ghengeveld
Copy link
Member

Alright so I had to pull in the code from react-editable-json-tree into the Controls addon directly, because doing so was infinitely easier than trying to get that library up to date with everything that's changed since it was last released 3 years ago.

That also means that we now have full control over how it's rendered, so I was able to lay it out as @domyen requested.

Screenshot.2021-02-22.at.17.41.47.mov

@ghengeveld ghengeveld removed their request for review February 22, 2021 22:36
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looking great @ghengeveld @hydrosquall 💯

@shilman
Copy link
Member

shilman commented Feb 23, 2021

Hmm, the UI is kind of strange for undefined values. @ghengeveld WDYT?

Reviewing_Build_21108_•_storybookjs_storybook

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

Successfully merging this pull request may close these issues.

6 participants