Skip to content

Nested arrays not finding data to render #994

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

Closed
clayroach opened this issue Jun 1, 2018 · 11 comments
Closed

Nested arrays not finding data to render #994

clayroach opened this issue Jun 1, 2018 · 11 comments

Comments

@clayroach
Copy link
Contributor

I have replicated this on my own fork

First one is broken where "firstarray" is an array. 2nd one works where "firstarray" is an object.

//JSON
const broken = {
  "firstarray": [
    {
      "objectarrayofstrings": {
        "choices": [
          "CHOICE_STRING_1",
          "CHOICE_STRING_2",
          "CHOICE_STRING_3"
        ]
      }
    }
  ]
}

// Schema
{
  "type": "object",
  "properties": {
    "firstarray": {
      "type": "array",
      "items": {
        "type": "object",
        "properties": {
          "objectarrayofstrings": {
            "type": "object",
            "properties": {
              "choices": {
                "type": "array",
                "items": {
                  "type": "string"
                }
              }
            }
          }
        }
      }
    }
  }
}

// UI Schema
{
  "type": "HorizontalLayout",
  "elements": [
    {
      "type": "Control",
      "label": "1",
      "scope": "#/properties/firstarray/items/properties/objectarrayofstrings/properties/choices"
    }
  ]
}


screen shot 2018-06-01 at 4 25 28 pm

// JSON
const works = {
  "firstarray": {
    "objectarrayofstrings": {
      "choices": [
        "CHOICE_STRING_1",
        "CHOICE_STRING_2",
        "CHOICE_STRING_3"
      ]
    }
  }
}

// Schema
{
  "type": "object",
  "properties": {
    "firstarray": {
      "type": "object",
      "properties": {
        "objectarrayofstrings": {
          "type": "object",
          "properties": {
            "choices": {
              "type": "array",
              "items": {
                "type": "string"
              }
            }
          }
        }
      }
    }
  }
}

//UI Schema
{
  "type": "HorizontalLayout",
  "elements": [
    {
      "type": "Control",
      "label": "1",
      "scope": "#/properties/firstarray/properties/objectarrayofstrings/properties/choices"
    }
  ]
}

screen shot 2018-06-01 at 4 26 53 pm

@edgarmueller
Copy link
Contributor

Thanks for the repro. I think part of the problem here is that a pointer to a nested array within the schema needs to be bound to the data instance at runtime which is ambiguous in this case since we can't specify indices in the schema, so it's not clear to what this should be bound to; to resolve this ambiguity, we would also need to render the outer array or somehow bind it implicitly to an index, I think. So what would be your expectation?

@clayroach
Copy link
Contributor Author

I agree... binding or otherwise explicitly referencing an index isn't desirable in this case. (It may be in other cases, but would be a departure from your current more declarative design). If you consider an array as just a grouping mechanism, then it corresponds more closely to one of the layout elements (Horizontal or Vertical) rather than an explicit control. Your approach to arrays binds the view and control together explicitly in the MaterialArrayControlRenderer which dictates a vertical layout and explicit controls.

I think the right approach is to treat an array as just a grouping mechanism (similar to a MUI FormGroup, RadioGroup, List, or Select (with multiple set to true). These are really just additional Horizontal or Vertical layout elements plus some controls for selecting/deselecting, adding/removing, etc. The simplest could even be just a couple of

's for wrapping the contents below into some HTML.

My initial thought is 2 controls - an ArrayLayout (either horizontal or vertical) which is just used to render the items below (horizontally or vertically), then an ArrayControl that can be subclassed into one of the MUI components listed above (and contains the appropriate controls).

I'm happy to help implement this in a PR today if we can decide on a direction here.

My immediate goal is to get the lower array into a Select control with multiple checkboxes - similar to the one on the MUI site, due to the need to compress a long list down to a compact control.

@edgarmueller
Copy link
Contributor

Yes, I think we did something similar in the past, e.g. the vanilla ArrayControl renderer does not render a table (which is called TableArrayControl), but renders each item with its own, generated UI schema. I think this is pretty similar to your suggestion, although I agree that we should probably rename ArrayControl to ArrayLayout. Also, this issue is a bit related to #992 which aims at being able to specify a separate UI schema that will be used to render each item of the array (e.g. via an additional option). Having that said, none of this is yet available for the material-renderer set, but does it cover what you have in mind?

@clayroach
Copy link
Contributor Author

Yes, this is related to #992, looks like. However, I don't see this as just a problem of what to render for each element of the array but also of the grouping control for the array itself - List, RadioGroup, etc

Thinking aloud... I'd argue that you have 2 types developer scenarios of jsonforms to consider:

  1. You just need a quick and dirty solution to get a form to display by setting up a basic schema and UI schema and having the framework automatically generate a "reasonable set of defaults" for interpreting how to display a simple 1-2 level JSON schema.
  2. You have a complex JSON object and need to pull out some of the pieces to display in a UI and ignore some of the other intermediate levels of nesting of components. Effectively being able to reach deeply into a JSON object and pull out something and present as a form element.

Maybe I'm oversimplifying, but my use cases fall squarely into the latter bucket. I have some complex JSON that I may have little to no control over how it is generated but need to get it into something that can be displayed easily. The alternative to this is to use something like jsonpath to transform something complex into and out of a simplified format that is more easily consumable by a UI framework. But I'd rather avoid additional mangling of the source JSON if I can avoid it.

This is a long-winded answer, but I think the approach of keeping the schema and uischema relatively simple and approachable allows more adoption (and contributors!), but for more complex scenarios or custom components, adding the ui schema registry seems like the right approach. In my case, I would register the #/properties/firstarray as just a simple ArrayLayout (vertical by default) and then the #/properties/firstarray/properties/objectarrayofstrings gets registered as a new or existing Select control, along with any custom renderers for rendering the Select child elements.

IMHO... In terms of default testers for this (scenario #1), I think you end up with the majority of the time, LEAF arrays get rendered using an ArrayControl, but any intermediate/non-leaf arrays get treated as an ArrayLayout.

@edgarmueller
Copy link
Contributor

I agree with everything you say, so I think we'll need to add the UI schema registry first step to support your use case properly. If you want to start working on this, we'd welcome any contribution, of course. I'm off for today and probably also tomorrow, but I'll get back to you as soon as I can if you should have any questions.

@clayroach
Copy link
Contributor Author

Ok, good that we're in alignment on this. The schema ui registry may be a bit more than I'm able to bite off in the next few days but I'll see what I can do to help. Might be that I take the more imperative approach in the short term and see how that could expand into the ui schema registry.

@edgarmueller
Copy link
Contributor

According to Eugen's comment it seems like we already have a UI schema registry, but in the editor package, so we might want to move it to core package.

edgarmueller added a commit to edgarmueller/jsonforms that referenced this issue Jun 6, 2018
* Add UI schema registry reducer
* Add material array layout
@edgarmueller
Copy link
Contributor

edgarmueller commented Jun 6, 2018

I've had a look at this yesterday and created PR #995 that demonstrates how this could look like. The actual layout renderer is ugly and tests are missing, but I think it contains everything what's necessary. Would you mind having a look if you find the time? Does this cover your use case properly?

edgarmueller added a commit to edgarmueller/jsonforms that referenced this issue Jun 9, 2018
* Add UI schema registry reducer
* Add material array layout
eneufeld pushed a commit to eneufeld/jsonforms that referenced this issue Jun 21, 2018
* Add UI schema registry reducer
This allows to register, unregister and retrieve uischemas.
The user can now configure how the detail in an ArrayLayout renderer should look like.
eneufeld pushed a commit to eneufeld/jsonforms that referenced this issue Jun 21, 2018
* Add UI schema registry reducer
This allows to register, unregister and retrieve uischemas.
The user can now configure how the detail in an ArrayLayout renderer should look like.
eneufeld pushed a commit to eneufeld/jsonforms that referenced this issue Jun 21, 2018
* Add UI schema registry reducer
This allows to register, unregister and retrieve uischemas.
The user can now configure how the detail in an ArrayLayout renderer should look like.
eneufeld pushed a commit to eneufeld/jsonforms that referenced this issue Jun 21, 2018
Add a new array renderer to material which renders all its children below each other.
This renderer is only used if the array being rendered contains another array in its path.
edgarmueller added a commit that referenced this issue Jun 25, 2018
* Add UI schema registry reducer
This allows to register, unregister and retrieve uischemas.
The user can now configure how the detail in an ArrayLayout renderer should look like.
edgarmueller added a commit that referenced this issue Jun 25, 2018
Add a new array renderer to material which renders all its children below each other.
This renderer is only used if the array being rendered contains another array in its path.
@edgarmueller
Copy link
Contributor

This was fixed with #1013 and has been released with 2.0.2. We've also added some documentation about the usage in the Providing UI schemas section. If there's anythings unclear, let us know.

@guenthermahr
Copy link

Hi, from your description in Providing UI schemas I can see how to register and retrieve an ui schema, but my problem is to modify the length of the different fields in an array. Can you please give a hint how registering an ui schema helps with that? Thanks, Günther

@edgarmueller
Copy link
Contributor

Hi @guenthermahr, what exactly do mean by length? Is it just CSS styling? If so, Custom Renderers might be the way a go, but it depends on what exactly you are trying to achieve (and which renderer set you are using). Or did you mean something different? If you could elaborate a bit more, I think we are able to help.

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

No branches or pull requests

3 participants