jsonforms icon indicating copy to clipboard operation
jsonforms copied to clipboard

ability to define uischema scope by $id

Open gilbdev opened this issue 5 years ago • 18 comments

Is your feature request related to a problem? Please describe.

currently i have to reference : { "type": "Control", "scope": "#/definitions/AnyElement/anyOf/0/allOf/1" } this is not really safe in case i add more sections in the anyOf or allOf, i will ahve to verify my uischema each time.

Describe the solution you'd like i would like to be able to define this way: { "type": "Control", "scope": "#myUniqueId" }

where #myUniqueId is the "$id" json field of the element i want.

bonus: being able to go further down the scope: "scope": "#myUniqueId/properties/myproperty"

gilbdev avatar Jul 24 '19 08:07 gilbdev

Hi, are you able to update the JSON Schema you are using? An alternative to this deeply nested reference would be to use $ref this way your UISchema can reference something that's used in any place in your schema.

There's some documentation about the use of $ref

Lily418 avatar Jul 24 '19 09:07 Lily418

@Lily418 @sdirix Are there any ways to avoid using such scopes in uischema now? (like "#/definitions/AnyElement/anyOf/0/allOf/1")

buryndin avatar Apr 24 '20 10:04 buryndin

In general yes as anything in JSON Forms is customizable, however there is no specific support.

We use the scope in two ways:

  • Resolving the corresponding part in the JSON Schema, currently using the JSON Schema ref parser. So for this part you can use any functionality this library supports.
  • Resolving the path within the data object to retrieve / modify. Currently this is manually implemented, so whatever scope you give will be transformed and used (e.g. removing properties out of the path etc.)

So you can just specify different scopes when they work with these two use cases.

The other approach would be to handle these two use cases in your own code instead of relying on JSON Forms. For this you need to register a whole renderer set (or at least all renderers which shall handle the custom scopes) and do the resolving on your own. The code in JSON Forms lives in the mapStateToControlProps function which is called by withJsonFormsControlProps. So you can "just" wrap all your or existing renderers with a customized withJsonFormsControlProps which can handle your custom scopes. This way any scope can be supported.

sdirix avatar Apr 24 '20 11:04 sdirix

Stefan,

thank you for the fast response. But unfortunately I don't understand how can I achieve this without writing custom renderers. Let me explain problem a little more. Suppose we have schema:

{
  "definitions": {
    "named_element": {
      "type": "object",
      "properties": {
        "name": { 
          "id": "/name",
          "type": "string" 
        }
      }
    }
  },
  "my_element": {
    "type": "object",
    "properties": {
      "allOf": [
        {"$ref": "#/definitions/named_element" },
        {
          "properties": {
            "active": { 
              "id": "/active",
              "boolean": "string" 
            }
          }
        }
      ]
    }
  }
}

and uischema

{
    'type': 'HorizontalLayout',
    'elements': [
        {
            'type': 'Control',
            'scope': '#/my_element/properties/allOf/0/properties/name'
        },
        {
            'type': 'Control',
            'scope': '#/my_element/properties/allOf/1/properties/active'
        }
    ]
}

It's work fine until somebody changes shema like this

...
 {"$ref": "#/definitions/named_element" },
 {"$ref": "#/definitions/named_element2" },

in this case uischema will become invalid, because we have to rewrite second scope like this 'scope': '#/my_element/properties/allOf/2/properties/active'

buryndin avatar Apr 24 '20 12:04 buryndin

I've created POC how it can be implemented.

It can be used like this: "scope": "$ref/definitions/named_element"

buryndin avatar Apr 29 '20 02:04 buryndin

Hi @buryndin, thank you very much for the POC! Can you give a concrete example on how the POC is intended to be used? Just by quickly looking at it I don't see the difference between using $ref/definitions/named_element and #/definitions/named_element but I'm probably overlooking something.

Even better if you could add the example as a testcase as we can then immediately step through it too and we need the testcases anyway before merging ;)

sdirix avatar Apr 29 '20 07:04 sdirix

Hi Stefan, my idea with '$ref' prefix in the scope is not so good, because we also need a data path. POC is completele wrong. Now I see 2 solution of described problem(I don't like scope like this 'scope': '#/my_element/properties/allOf/2/properties/active')

  1. Support object in scope: "scope": { "$ref": "/my_schema/active", "dataPath": "active" } where $ref - reference to schema and dataPath - path in data This is explicit way, but it increases uischema.

  2. Support abilitity to skip allOf/oneOf/anyOf in scope: 'scope': '#/my_element/properties/active' This is implicit way. Because in 'allOf' schema can be exist several occurrences of property and we can return first or last, but not exactly what the user wants.

Personally I prefer the second way because I don't want to increase our uischema. What do you think is the best way?

buryndin avatar Apr 29 '20 14:04 buryndin

@sdirix We have implemented approach 2. We have been using it successfully for 6 months. Could you please look into it?

buryndin avatar Feb 24 '21 08:02 buryndin

@buryndin So what do you do when the scope is no longer uniquely identifying the property because the allOf, oneOf or anyOf is missing? Do you just return the first found element?

sdirix avatar Feb 24 '21 11:02 sdirix

You are right. We just return the first found. If user does not agree with this behavior, he can use current approach(with allOf/oneOf/anyOf in the scope). We keep backward compatibility in our PR

buryndin avatar Feb 24 '21 12:02 buryndin

@sdirix Are there any thoughts about suggested PR?

buryndin avatar Mar 02 '21 07:03 buryndin

Hi @buryndin, I actually discussed the PR with my team mates yesterday. It's kind of a special PR as it only improves on the current behavior, but the added functionality is kind of magic, needs to be documented and increases the complexity of the implementation just for this edge case. This change does not even enable something new conceptually, it's more a convenience feature to not require any ui schema changes for minor JSON schema edits.

We're still considering merging, as it's a rather small change and shouldn't lead to any problems. However before we do that we would like to know more about your use case.

We were wondering why you are actually creating controls with such nested paths, e.g. #/definitions/AnyElement/anyOf/0/allOf/1? Is the anyOf control not good enough, e.g. { "type": "Control", "scope": "#/definitions/AnyElement" }? Are you aware of the uischemas feature in which you can register sub-uischemas for certain paths, so you can influence the uischemas of the displayed oneOf, anyOf tab?

sdirix avatar Mar 02 '21 07:03 sdirix

@sdirix We have a huge class hierarchy and we describe class properties using JSON schema. We use exteding for this. But when we describe uischema we want to have a simple path like { "type": "Control", "scope": "#/properties/PropFromBaseClass" } and we don't want to create sub-uischemas, because every class has its own layout for ui elements.

buryndin avatar Mar 02 '21 07:03 buryndin

Can you post a smallish JSON Schema and UI Schema example that you are using? I'm also wondering why you need to point through an anyOf. So why not stop there and use the anyOf renderer?

sdirix avatar Mar 02 '21 08:03 sdirix

Base class

{
  "id": "/Base",
  "type": "object",
  "properties": {
    "width": {
      "type": "integer"
    }
  }
}

Child

{
  "id": "/Child",
  "type": "object",
  "allOf": [
    { "$ref": "/Base" },
    {
      "properties": {
        "geometry": {
          "type": "string"
        }
      }
    }
  ]
}

Data:

{
  width: 1,
  geometry: 'rect'
}

and we want to have schema like this:

'elements': [
    {
        'type': 'Control',
        'scope': '#/properties/width'
    },
    {
        'type': 'Control',
        'scope': '#/properties/geometry'
    }
]

In our case, uischema does not need to know about the hierarchy.

buryndin avatar Mar 02 '21 08:03 buryndin

@sdirix Are there any thoughts about our use case?

the added functionality is kind of magic, needs to be documented

Should we add some documentation to our PR?

buryndin avatar Mar 09 '21 03:03 buryndin

Hi @buryndin, I added an example with your use case to the examples bundle. Note that at least the allOf renderer works perfectly fine without needing the special resolving suggested here. However for specifying controls independent of the allOf control it's really convenient. Therefore we merged your PR. I modified the code a bit and documented the intention.

Would be great if you could test the current prerelease, i.e. version 2.5.1-alpha.1.

Note that we'll maybe rework resolving in general in the future to conveniently allow user defined resolving behavior. In that case we might remove this "special" code but then you can easily customize it again. However this will happen with 3.X at earliest.

sdirix avatar Mar 11 '21 17:03 sdirix

@sdirix

I've tested and it works properly. Thanks a lot! As for plans about next major version: it is a good idea to allow user defined resolving behavior.

buryndin avatar Mar 12 '21 02:03 buryndin

I'll close this issue for now as the problems discussed within were solved. Feel free to reopen if you disagree.

sdirix avatar Nov 08 '22 22:11 sdirix