writer / writer-framework

No-code in the front, Python in the back. An open-source framework for creating data apps.
https://dev.writer.com/framework/introduction
Apache License 2.0
1.3k stars 73 forks source link

feat: implement `JsonViewer` widget #479

Closed madeindjs closed 1 month ago

madeindjs commented 2 months ago

Implement a brand new CoreJsonViewer component

Screencast from 2024-07-09 21-28-15.webm

This component uses new base components as:

FabienArcellier commented 2 months ago

I feel like I would want to open level 2 or level 3 easily without giving complex jsonpath expression.

@ramedina86 @madeindjs Would it be interesting to have a complementary Opened level field with Paths open ?

Opened level: 0 # none (default)
Opened level: -1 # all
Opened level: 1 # open all level one
name: Opened level
desc: Open a specific level of json object (-1 to open all)
ramedina86 commented 2 months ago

I feel like I would want to open level 2 or level 3 easily without giving complex jsonpath expression.

@ramedina86 @madeindjs Would it be interesting to have a complementary Opened level field with Paths open ?

Opened level: 0 # none (default)
Opened level: -1 # all
Opened level: 1 # open all level one
name: Opened level
desc: Open a specific level of json object (-1 to open all)

@FabienArcellier Yes I wasn't convinced that we needed paths in the first place but @madeindjs thought we should keep it

I'm not in favor of keeping both but I'd prefer Fabien's approach one over the current path mechanism, unless @madeindjs can convince me with a solid use case.

FabienArcellier commented 2 months ago

I think a copy action would be really helpful for json. Does it make sense ?

image

FabienArcellier commented 2 months ago

I'd prefer Fabien's approach one over the current path mechanism, unless @madeindjs can convince me with a solid use case.

I am convinced it's interesting to keep it. I see a usecase for implementing search in huge json file. It allow advanced scenario for people.

ramedina86 commented 2 months ago

Ok let's keep the paths.

I think a copy action would be really helpful for json. Does it make sense ?

That'd be great, but let's do that at a different stage. @polymorpheuz is working on a bar to enable "Copy" in Annotated Text component, so once we merge that we can reuse and apply to this one. I'll create a ticket.

madeindjs commented 2 months ago

Thanks for your feedback @ramedina86 & @FabienArcellier !

About the pathsOpen, yes, my argument was actually to "highlights" a part of a JSON without expanding everything (as @FabienArcellier pointed it). For instance, in the example below, user might want to display foo.bar without showing others

{
  "foo": { "bar": "value" },
  "others": [/* 1K items */]
}

But,I don't know well the product to know if it's a solid use case.

Also, little disclaimer, my implementation of the JSON path is a bit naive. It doesn't work for keys containing . like { "foo.bar.1": "value" } https://github.com/writer/writer-framework/blob/28db80f758545bb3e1221aa29049229666a7e149/src/ui/src/core_components/content/CoreJsonViewer.vue#L81

I'm fine to implement @FabienArcellier 's idea or to not expose this setting for now.

ramedina86 commented 2 months ago

Good point about the dots, what we could do is use arrays

["foo", "bar", "1"]

@FabienArcellier what do you think?

FabienArcellier commented 2 months ago

If we don't follow Jsonpath or JMESPath (two json query standard), I would advocate we don't propose the feature yet. It would be too hard to explain and document for the user.

If we want to propose this feature, I would advocate to use external library as jsonpath

ramedina86 commented 2 months ago

@madeindjs ok let's drop the open specific paths for now and go for @FabienArcellier's idea (as list of options)

Opened level: 0 # none (default) Opened level: -1 # all Opened level: 1 # open all level one

FabienArcellier commented 2 months ago

It's almost looks perfect. I enjoy the component. I was surprised when I set 0, the level 1 is opened. Is it voluntary ?

image

FabienArcellier commented 1 month ago

Decision :

madeindjs commented 1 month ago

@ramedina86 & @FabienArcellier , I implemented the final modifications

Screencast from 2024-07-09 21-28-15.webm