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 args #12685

Conversation

hydrosquall
Copy link
Contributor

@hydrosquall hydrosquall commented Oct 7, 2020

Issue: #12599

What I did

  • Replaced the existing text form control with the react-json-view lib (public demo here: https://mac-s-g.github.io/react-json-view/demo/dist/ )
  • Updated ObjectConfig so that the user can customize the behavior of this control using all non-function props of the ReactJSON component
  • Updated the official-storybook story about using add-controls
  • Example GIF of the new controls in action: https://a.cl.ly/xQuAg4dy

Thanks to @yannbf for providing a guide to getting started and showing an example with using knobs here.

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 document the ObjectConfig options onthis page.

On the other hand, we could also keep this as an undocumented feature if we're not sure whether we want users to be customizing their controls to this level. Not documenting these properties officially gives us the flexibility to switch to other JSON tree components in the future, without fear that the new library may not support all of these options.

FAQ

  • Why not some other JSON tree viewer library XYZ?

  • What terminal commands did you use that you might use again next time?

# to add a dependency scoped to 1 package (seems basic, but it took me a while to figure out the right scope as a new contributor)
yarn lerna add react-json-view --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 hydrosquall force-pushed the cameron.yick/feature/ui-json-tree-knob branch from 17bbf30 to 4825594 Compare October 7, 2020 03:30
@shilman shilman changed the title feat(controls) Replace Text form with JSON Tree viewer for Object Properties Addon-controls: Replace Text form with JSON Tree viewer for Object args Oct 7, 2020
@shilman shilman changed the title Addon-controls: Replace Text form with JSON Tree viewer for Object args Addon-controls: Add JSON tree editor for Object args Oct 7, 2020
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 awesome @hydrosquall!! ❤️

Can we remove most or all of the options? That's just more stuff to maintain and we should have good defaults for users out of the box. Of the existing options, I think the "sort" and "collapsed" options are things that we might want to allow configuration for, and the rest we should just provide good defaults. WDYT?

@domyen can you take a quick look at the design?

@shilman shilman added this to the 6.1 args milestone Oct 7, 2020
@domyen
Copy link
Member

domyen commented Oct 7, 2020

Nice work @hydrosquall!

Requests:

  • Can we enable double clicking on the value reveal the editable textarea (instead of clicking the edit button)? Mousing over the 🖋 icon is pretty cumbersome when trying to edit. It's more intuitive to point and click to edit. When a Control is available for a component it implies that it's editable.

  • Normalize the typography to font-size: 13px and line-height: 18px. You may have to use !important and a generic selector (.react-json-view > *) to override the 11px fonts.

  • Display component in a box similar to how the the rest of the editable control elements are rendered.
    image

  • Support light and dark themes. You can develop both simultaneously by clicking this button in the toolbar.
    image image

Re:options
Agree with @shilman that we should not expose all the options from this component. Let's choose reasonable defaults.

  • Remove UI noise:
    • displayObjectTypes: false
    • displayDataTypes: false

@yannbf
Copy link
Member

yannbf commented Oct 7, 2020

Awesome stuff @hydrosquall !

@domyen you can check this live demo to see what's available in the plugin. I don't think double click to edit is possible, but there's a shortcut stated in the demo:

To edit a value, try ctrl + click enter edit mode

I think that won't work on mac though, because ctrl + click is equivalent to right click

@domyen
Copy link
Member

domyen commented Oct 7, 2020

@yannbf Thanks, I was referencing the demo for my earlier comments.

The goal of Controls is to make it easy to edit any component prop. The current edit interaction results in a lot of extra clicking (at least 2 extra clicks for each field you want to edit). That's pretty unwieldy.

I wonder if a simpler text editor interface makes editing json faster/easier. For example, this plugin (with fewer features) renders an editable json and validates. That way folks can paste or write the format they want.

wdyt?

@shilman
Copy link
Member

shilman commented Oct 7, 2020

Hmm that's pretty nice @domyen 🤔

@yannbf
Copy link
Member

yannbf commented Oct 7, 2020

@yannbf Thanks, I was referencing the demo for my earlier comments.

The goal of Controls is to make it easy to edit any component prop. The current edit interaction results in a lot of extra clicking (at least 2 extra clicks for each field you want to edit). That's pretty unwieldy.

I wonder if a simpler text editor interface makes editing json faster/easier. For example, this plugin (with fewer features) renders an editable json and validates. That way folks can paste or write the format they want.

wdyt?

Completely agree and we should try to make the editing experience easier, so maybe we can make a contribution to react-json-view to add the functionality we want. The dev is not actively doing new features in the repo but he said he is willing to review and accept contributions.

A simpler lib like the one you mentioned is nice, but it's pretty dev-centered (stakeholders could not understand how to tweak with these values) and it also starts to get quite messy once the object is pretty large and contains arrays etc. The nice thing about the react-json-view is that it takes complex objects and makes them simple to visualize, expan, and collapse, rather than displaying long texts like you'd get in a text editor. Controls is in a panel that is usually pretty short in height, and being able to collapse properties is a big plus.

You do mention an interesting point, though. What if the user wants to get a json and paste it directly to controls? With react-json-view that's not possible. In a way it's a good thing, because the lib enforces users to keep with the object structure coming from args and not simply be able to paste any random data. If the (niche) users want to paste a json, they could do so in the .args property of the story I guess!

@domyen
Copy link
Member

domyen commented Oct 7, 2020

I guess there are two options here:

A. Make PRs into react-json-view to make editing easier
B. Reduce scope to an editor with improved formatting (vs current object control) and some validation

B. makes sense to me after more consideration. It's lower scope and satisfies what users have asked for (better object formatting). 🤦‍♀️ Ha! I already went through and customized RJV, but now I don't think it's what we need.

A. has a lot of uncertainty about whether we can achieve the user experience we want. It's also greater maintenance surface area.

Another option: Do what Cosmos does and have a tree + editable text fields. This seems like a lot more work for

Other stakeholders
Editing object's is a developer-oriented feature more so than picking a color. RJV isn't really optimized for editing.

@hydrosquall
Copy link
Contributor Author

The goal of Controls is to make it easy to edit any component prop. The current edit interaction results in a lot of extra clicking (at least 2 extra clicks for each field you want to edit). That's pretty unwieldy.

Hi @domyen, thanks for taking a close look. I have some additional thoughts about using a nested object tree vs a text editor.

  • The nested object tree uses fewer clicks to copy paste an object property via the clipboard button. (Copy pasting an existing property in an editor will take me 4 mouse actions clicks: start selection, drag, right click, copy). I do a lot of copy pasting of sample props from docs into code that I'm writing, probably more so than editing.
  • A text editor doesn't always work well for me when I have an object property (common in data visualizations) with a long list of objects in an array - the ability to collapse nodes is very important for readability.
  • I'm not sure I completely follow the concern about 2 extra clicks in both cases (react-json-view and the cosmos component), there must be at least one click is to select the property to be modified (perhaps the issue is that the click target is the icon rather than the property name). I think that having 1 extra click to "commit" the change is a reasonable effort to make in exchange for the readability boost,

I think a collapsible tree view is preferable to a text editor (even with nicer formatting) for the components that I'm looking to use storybook with, and would like to see a tree as an option even if it's not a default.

A. Make PRs into react-json-view to make editing easier
B. Reduce scope to an editor with improved formatting (vs current object control) and some validation

What do you think about C having two possible views into objects, the same way number can be represented using a slider OR a numeric text box?

If so, would you still be interested in the react-json-view tree, even if it required clicking on the clipboard instead of the property name to trigger edit mode if I make the other design changes you requested? If we observe that users are finding the extra click too cumbersome, we can swap out the underlying implementation later on.

@hydrosquall
Copy link
Contributor Author

What if the user wants to get a json and paste it directly to controls? With react-json-view that's not possible. In a way it's a good thing, because the lib enforces users to keep with the object structure coming from args and not simply be able to paste any random data.

@yannbf I just had an idea for this - if this "import from a clipboard" is a necessary future feature request, we could have a button to read from clipboard, a popup dialog, or some open-able text field. If it passes JSON validation, then this data could be rendered and subsequently further edited in react-json-view. It doesn't strike me as required for a v1, but adding this library now still leaves the door open for us to try something like this in the future.

@hydrosquall
Copy link
Contributor Author

Can we remove most or all of the options? That's just more stuff to maintain and we should have good defaults for users out of the box. Of the existing options, I think the "sort" and "collapsed" options are things that we might want to allow configuration for, and the rest we should just provide good defaults. WDYT?

@shilman sure thing! I'll do this if we decide to move forward with react-json-view, but hold off from changing any code until getting confirmation that we are still interested in having some form of react-json-view added to storybook.

@hydrosquall
Copy link
Contributor Author

hydrosquall commented Oct 7, 2020

One alternative to waiting for react-json-view to get a new maintainer / forking the library would be to use this other lean JSON tree component from my original investigations. It has a lighter bundle weight, and fewer features (most noticeably missing the 1-click "copy to keyboard" change), but several of those features would have been disabled anyways (showing data count or data type next to each node). Do we like the UX of this component more since it lets you trigger editing with 1 click?

@domyen
Copy link
Member

domyen commented Oct 11, 2020

The target UX of Controls (and Storybook essential addons at large) is to instantly render changes when a control's value changes. That makes it faster/easier to see the results of prop changes.

My UX goals for object are:

  • Better object formatting to improve readability
  • Instant updates when object value changes
Lib Formatting Instant
react-json-view Yes No
react-editable-json-tree Yes Not sure
react-json-editor-ajrm Yes Not sure
jsoneditor Yes Not sure

For react-json-view, the UX doesn't fit and there's a lot of uncertainty around whether we can make UX changes upstream (or have the appetite to continue maintaining those changes long term). Forking a library is pretty burdensome 😅.

react-editable-json-tree might work, I haven't played with it beyond the demo. I guess it expects you to bring-your-own-styling. Do you think it satisfies the above reqs?

jsoneditor seems promising (but heavy 226kb gzipped). It has a tree and editor view.

In a nutshell, I'm trying to figure out how to satisfy the UX goals with low maintenance overhead. I suggested an "editor" because it's an evolutionary next step from what we have now. It's better than an unformatted text field (what we have now) and is straightforward to integrate in our UX.

@shilman
Copy link
Member

shilman commented Oct 11, 2020

@domyen WDYT about styling react-editable-json-tree? It appears to have a more direct UI than react-json-view, tho AFAICT it's not instant. If it looked good I think it could be a great choice.

There are actually three types of props that we care about:

  1. scalar data
  2. objects with schema (addon-controls) Support destructering object properties into individual controls #12078
  3. JSON objects without schema

We already handle 1 well. Ideally we handle 2 directly as rows in the ArgsTable, like Cosmos, but that's a bigger project and out of scope. I think for 3 we should handle that in a tree view like @hydrosquall proposes. Editing this stuff is unwieldy, even in the editor you recommended. Structuring it trumps instant keystroke-by-keystroke updates IMO.

@hydrosquall we could potentially include both object UIs as an option. Another idea would be to implement #11486 and then you could override the object control outside of Storybook. I don't have a timeline for that though.

@shilman shilman modified the milestones: 6.1 args, 6.2 args Oct 13, 2020
@hydrosquall
Copy link
Contributor Author

react-editable-json-tree might work, I haven't played with it beyond the demo. I guess it expects you to bring-your-own-styling. Do you think it satisfies the above reqs?

It is highly customizable ( many namespaced CSS class names to hook into), but I don't think it satisfies the constraint of instant updates. If that is the most important factor, then I haven't come across any custom components that do instant updates aside from the react-cosmos custom TreeView implementation.

As a point of feedback about instant updates, when typing a string name in a nested tree, there are a number of intermediary "bad" states that I don't actually want to lead to re-rendering, such as when I have a list of graph nodes and graph edges and am typing out nodes whose names have multiple characters). In these scenarios, requiring an extra click to "commit" the change is OK because I'm not distracted by viewing renders of data that technically parses correctly but isn't what I want to happen.

Another idea would be to implement #11486 and then you could override the object control outside of Storybook.

I very much like the idea of being able to bring in my own UI controls for when they fall out of the mainline Storybook design case, so whenever that's done, I'll be happy to test it out.

Next steps

  • If we move past the concern about non-instant updates and clicking the edit button instead of the property name, react-json-view has the nice UX of 1 click "copy to clipboard" feature.
  • If we only move past the concern of non-instant updates, react-editable-json-tree is heavily style-able ( you can customize the textArea, the input button, etc). I could open a new PR using that library instead we feel that having the property name as the click target is an essential feature.

If lack of non-instant updates on the default object viewer is the main issue, then I'd like to propose shifting this PR from replacing the existing Object editor (which would remain the default so users continue to get the instant experience as a default, and someone else can start the process of making a fancier JSON text editor) to adding a new Control Type for object data with the value of tree. @shilman / @domyen what do you think?

I think this solution would avoid breaking existing workflows. My question is whether the same set of design concerns (click name to edit instead of the edit button, instant updates) all apply if this tree is no longer the default. If we're able to release both of these constraints, I can make the edits to react-json-view with the original set of visual design feedback on this PR. If we instead want to try react-editable-json-tree because it lets you trigger editing by directly clicking on the property name, I'll close this PR and open a new one later in the week.

@domyen
Copy link
Member

domyen commented Oct 17, 2020

Clicking on the field you want to directly edit is a hard constraint for me because it gets us close to direct manipulation of the inputs.

I don't think react-json-view is the user experience we want because it requires precise manipulation of small click targets. Not to mention the styling constraints.

You and @shilman make good points about the tradeoffs between evolving the experience we have now and instant editing (an out-of-the-box solution doesn't seem to exist yet). It makes sense to release that constraint 👍. Thanks for getting us there!

If we instead want to try react-editable-json-tree because it lets you trigger editing by directly clicking on the property name, I'll close this PR and open a new one later in the week.

This proposal SGTM.

To confirm, the target user experience is:

  1. Click on a field
  2. Adjust it
  3. submit when user presses enter or blurs
  4. Component is updated in the Canvas tab

And it should follow our styling conventions (within reason).

@ndelangen
Copy link
Member

If we could do:

Another idea would be to implement #11486

and lazy load these components, that would be ideal.

@hydrosquall
Copy link
Contributor Author

Clicking on the field you want to directly edit is a hard constraint for me because it gets us close to direct manipulation of the inputs.

Understood! I am all for the value of direct manipulation a la Bret Victor and giving users fast feedback 👍 .

I don't think react-json-view is the user experience we want because it requires precise manipulation of small click targets.

If the library had an active maintainer I would consider opening a PR there to add "click property instead of edit button", but since that isn't the case, I agree that it'll make sense for us to switch libraries.

To confirm, the target user experience is:
Click on a field
Adjust it
submit when user presses enter or blurs (e.g. press esc, or click outside the input field)
Component is updated in the Canvas tab

From testing the demo here (https://oxyno-zeta.github.io/react-editable-json-tree/), I think all of these UX goals will be achieved. Note that unlike in react-json-viewer, this means it is harder to have prop strings that contain newlines if we're listening on the enter key (but I think this is probably a reasonable trade)

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.

5 participants