stoplightio / json-schema-viewer

A JSON Schema viewer React component
Apache License 2.0
175 stars 37 forks source link

feat: oneOf/anyOf as a dropdown, attempt no 2 #110

Closed marcelltoth closed 3 years ago

marcelltoth commented 3 years ago

Implements most of stoplightio/elements#667

The only missing piece is figuring out titles based on $ref names (see the issue), that will be handled separately. That will need to be a JST feature, as we simply don't have the requested info right now (beyond looking at fragment).

@P0lip Do you think I can simply default the title property (meaning it will be used everywhere), or should I keep it separately in another property and only use it here?

P0lip commented 3 years ago

@P0lip Do you think I can simply default the title property (meaning it will be used everywhere), or should I keep it separately in another property and only use it here?

Do you mean the potential JST change?

Based on the following sentence

The only missing piece is figuring out titles based on $ref names (see the issue), that will be handled separately

and the actual requirement the above adhered to

the last segment of the $ref. If last segment is a file, chop off the extension

I'm not sure we need any change.

What you have implemented here seems to be covering everything. Am I missing anything?

philsturgeon commented 3 years ago

Can you give a before and after screenshot so I can approve? Thanks!

marcelltoth commented 3 years ago

@P0lip The scenario that you're missing:

Take the following schema:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "definitions": {
    "address": {
      "type": "object",
      "properties": {
        "street_address": {
          "type": "string"
        }
      },
      "required": [
        "street_address"
      ]
    },
    "person": {
      "type": "object",
      "properties": {
        "first_name": {
          "type": "string"
        },
        "last_name": {
          "type": "string"
        }
      }
    }
  },
  "type": "object",
  "properties": {
    "combiner": {
      "oneOf": [{"$ref": "#/definitions/address"}, {"$ref": "#/definitions/person"}]
    },
    "billing_address": {
      "$ref": "#/definitions/address"
    },
    "shipping_address": {
      "$ref": "#/definitions/address"
    }
  }
}

Now if I render this, this currently lets me chose between "object" and well... "object". image

What @philsturgeon was looking for in the issue description is that I realize that these objects are referenced as #/definitions/person and automatically name it "person".

My question to you both is:

Should this be a feature for this specific dropdown, or can it be a general feature?

To give you an example based on the above, if this can be made a generic JSV feature (which is the simpler one to implement actually I believe), then the above schema would render like this:

image

philsturgeon commented 3 years ago

If we cannot handle finding names when there's no title we can just call them object #1 and object #2.

marcelltoth commented 3 years ago

If we cannot handle finding names when there's no title we can just call them object #1 and object #2.

That's OK, but that doesn't answer the question if we can - in general - use the $ref names for objects that lack a title?

philsturgeon commented 3 years ago

We are not trying to remove object and replace it with title everywhere, we are trying to offer a drop down that makes sense to people.

mallachari commented 3 years ago

I was trying to break it with multiple weird nested schemas and it behaves well. Only thing that brought my attention was an array of anyOf/oneOf:

Screenshot from 2021-03-24 13-49-26

Do we have defined how to render such cases? I guess it would require multiple select inputs

marcelltoth commented 3 years ago

I was trying to break it with multiple weird nested schemas and it behaves well. Only thing that brought my attention was an array of anyOf/oneOf:

Screenshot from 2021-03-24 13-49-26

Do we have defined how to render such cases? I guess it would require multiple select inputs

This is weird, the simple use-case should work. What is the exact schema you are testing with? I've tried changing the example schema I sent above to


  "properties": {
    "combiner": {
      "type": "array",
      "items": {
        "oneOf": [{"$ref": "#/definitions/address"}, {"$ref": "#/definitions/person"}]
      }
    },
    ...

And it seems to work just fine: image

This doesn't require nested selects though. The more complex case that does is handled separately at https://github.com/stoplightio/elements/issues/713

mallachari commented 3 years ago

This is weird, the simple use-case should work. What is the exact schema you are testing with?

I used this type of schema:

{
  "type": "array",
  "items": {
    "oneOf": [
      {
        "type": "object",
        "properties": {
          "aaa": {
            "type": "string"
          }
        }
      },
      {
        "type": "object",
        "properties": {
          "bbb": {
            "type": "string"
          }
        }
      }
    ]
  }
}

Though it is nested in another anyOf combiner so exactly the case that you mentioned of multiple depth anyOf/oneOf. It works the way you described if it's the plain property. So it's not in scope of this issue in the end.

stoplight-bot commented 3 years ago

:tada: This PR is included in version 4.0.0-beta.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

stoplight-bot commented 3 years ago

:tada: This PR is included in version 4.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: