[WIP] Handle nested array

[edgarmueller]

This PR aims at coming up with a solution for #994 and adds a UI schema registry which allows registering a custom UI schema that might be used by any renderer. It is only used by the MaterialArrayLayoutRenderer, which has also been added with this PR.
A basic react application which democases the proposed solution and serves as a playground in the future also has been added.

[edgarmueller]

@eneufeld Requesting feedback

[coveralls]

Coverage decreased (-0.5%) to 87.3% when pulling 9c9aba4 on edgarmueller:topic/uischema-registry into 587e744 on eclipsesource:master.

[eneufeld]

looks good
why do we need the app stuff though?

[eneufeld]

shouldn't the tester be unique? or the uischema?

[eneufeld]

maybe pass in the data?

[edgarmueller]

Return generated UI schema instead of registered one.

[edgarmueller]

Use same logic as in JsonForms renderer (which is maxBy)

[clayroach]

Awesome! I'm going to pull down and validate some use cases today.

[clayroach]

I'm midway through debugging root cause but this doesn't yet support the issue identified in #994. I understand this is a WIP.

First thing I encountered was just compile issue. I had to add the following to MaterialArrayLayout line 47:

findUISchema(schema: JsonSchema, schemaPath: string, path: string);

Anything I can do to help? I can continue to debug unless there are known issues here that you're already aware of.

[edgarmueller]

Not sure which line you are referring, did you mean this one?
Also, there was indeed a typing error (which I've fixed), since findUISchema was missing in the MaterialArrayLayoutRendererProps, but, as the name implies, this was an error in MaterialArrayLayoutRenderer, not MaterialArrayLayout. The findUISchema function in MaterialArrayLayout, which you possibly referred to, is coming from the props, so there should be no need to import it.

[clayroach]

Ah yes.... was MaterialArrayLayoutRenderer I was referring to.

[edgarmueller]

Ok, what other issues are you experiencing, can I help?

[clayroach]

The test case for #994 I created still seems broken.

Few minor ones...

warning.js:33 Warning: Unsupported style property white-space. Did you mean whiteSpace?
    in label
    in FormLabel
    in WithStyles(FormLabel)

Warning: Unsupported style property text-overflow. Did you mean textOverflow?
    in label
    in FormLabel
    in WithStyles(FormLabel)

[edgarmueller]

How do you test this? Do you use the example package (which is just a copy of the seed) that has been included in this PR that contains the configuration from your fork? I've set it up in order to being able to test your use case. When starting it, it should look as follows:

Thanks for pointing out the minor issues, we've fixed those already in #999

[clayroach]

Must be my set up then.... I was using my own fork of the example, not the one that was part of the PR.

[clayroach]

I tried starting from a fresh clone and pull from the pr/995 and still seeing the same issue...

Also validated I had the updated versions in my node_modules.

Looks like I need to dive in a bit deeper with debug tools.

This is what I get with the uischema-broken.json from my previous fork.

UI SCHEMA

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

[edgarmueller]

Ok, that looks a bit better, but it seems as if the custom UI schema hasn't been registered yet.
Before making any assumptions why this is the reason, let's just step through the entire process, so that you can find the spot where it differs from your approach. I'll also explain any relevant changes that have been introduced with this PR (I realize that this will become a somewhat longer answer so I apologize upfront):

Let's start by cloning & building:

git clone https://github.com/edgarmueller/jsonforms.git jsonforms-995
cd jsonforms-995
git checkout topic/uischema-registry
lerna bootstrap --hoist
lerna run build

If you run into an error similar to (I don't know why it pops up, we have to check):

Failed to minify the code from this file:
       $CLONE_LOCATION/node_modules/ajv/node_modules/uri-js/dist/esnext/uri.js:42

edit node_modules/ajv/node_modules/uri-js/package.json and remove the module field and run lerna run build again. Once the build is done, let's start the example:

cd packages/example
npm run start

Now let's go through the actual changes in this PR and how they are supposed to work:

• there's a new reducer/substate called uischemas which serves as the mentioned UI registry, it basically is just a place where to put UI schemas. UI schemas can be registered there with a newly created action creator called registerUISchema (and also unregistered via unregisterUISchema). The registerUIschema actions expects two parameters: the first one is a tester, which determines when a specific UI schema should be considered for applicability, the 2nd one is the actual UI schema to be used.
• The interface of the tester is called UISchemaTester and currently expects three parameters (this might change): the currently scoped down JSON schema, an optional schemaPath and the current data scope via path. The schemaPath will have the value of the scope property if the current UI schema element is a control. For your use case this will become "#/properties/firstarray" (also see description of MaterialArrayLayout renderer further down).
  The UISchemaTester is expected to return a number in order allow overriding any already registered UI schemas.
• The findUISchema function, given the arguments for a tester, will try to find any registered UI schema.

In packages/example/index.js you can see how we register a UI schema (I've added a 'name' property to the schema here):

store.dispatch(Actions.registerUISchema(
  (jsonSchema, schemaPath) => {
    return schemaPath === '#/properties/firstarray' ? 2 : NOT_APPLICABLE;
  },
  {
    type: 'VerticalLayout',
    elements: [
      {
        type: 'Control',
        scope: '#/properties/name'
      },
      {
        type: 'Control',
        scope: '#/properties/objectarrayofstrings/properties/choices'
      }
    ]
  }
));

• The new MaterialArrayLayout renderer will be triggered if we encounter any intermediate array (see the materialArrayLayoutTester). It will not be used for 'leaf' arrays. Additionally, it queries the UI schema registry (using the schema, the schemaPath obtained by the current UI schema element as mentioned before and the path) and checks whether any UI schemas have been registered that might be used to render the elements of the array. If none are found, findUISchema will generate one. This UI schema will then be used to render each child of the array.

This is how we basically end up with the UI schema we have registered before. I hope this all make sense and it's a little bit more clear how all of this works. If this doesn't work for you or you keep having problems, please update your fork and I'll have a look.

[clayroach]

@edgarmueller - I just updated with the additional changes in the last commit and is working now for me!

The only couple of very minor things I can see at this point are:

1. The 2 warnings to change white-space to whiteSpace and text-overflow to textOverflow.
2. The <input> elements of the array don't have unique ids generated so also is a console-level warning.

Otherwise this all looks really great! I am going to experiment a bit with additional control options and playing around with the ui schema registry. I'll provide PR's for any that seem worthwhile adding back to the framework.

Awesome work on this and thanks for the support getting through the testing/validation.

[edgarmueller]

Great, happy to hear it works now! I've re-based the branch with master to include #999 so at the least the first warning should be gone now.
We are of course happy about any feedback/PRs if you find the time.

[eneufeld]

closing in favor of #1013