Skip to content
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

AnyOf/OneOf Merge Top Level Schema #3808

Closed
4 tasks done
cwendtxealth opened this issue Aug 2, 2023 · 5 comments · Fixed by #3821
Closed
4 tasks done

AnyOf/OneOf Merge Top Level Schema #3808

cwendtxealth opened this issue Aug 2, 2023 · 5 comments · Fixed by #3821
Labels
feature Is a feature request help wanted

Comments

@cwendtxealth
Copy link
Contributor

Prerequisites

What theme are you using?

core

Version

5.11.x

Current Behavior

In 5.11 there is a fix merging top level schema of oneOf/anyOf into the selected option field. This adds duplicated field on the UI; is this expected behavior? Before the fix it still had the top level field correctly and then showed the selected anyOf/oneOf fields.

Here is playground of the schema that is used in the unit test. Here is what is looks like
image

Before fix:
image

Expected Behavior

Only show the field once

Steps To Reproduce

No response

Environment

- OS:
- Node:
- npm:

Anything else?

No response

@cwendtxealth cwendtxealth added bug needs triage Initial label given, to be assigned correct labels and assigned labels Aug 2, 2023
@heath-freenome heath-freenome added help wanted feature Is a feature request and removed bug needs triage Initial label given, to be assigned correct labels and assigned labels Aug 4, 2023
@heath-freenome
Copy link
Member

heath-freenome commented Aug 4, 2023

@cwendtxealth I believe that this is actually a duplicate of #3787, at least in its solution:

This requires implementing a new feature similar to ui:fieldReplacesAnyOrOneOf except that it would be ui:anyOrOneOfReplacesField. In the SchemaField we render both the field and the anyOf/oneOf. With this flag turned on then I would imagine that the field being rendered on line 288 is skipped since the anyOf/oneOf will do it. This would required some verification of it working properly, including some decent unit tests. We could use some help for this.

Also, since you have been doing some great contributing, I would love to have you join our weekly meeting if you can

@cwendtxealth
Copy link
Contributor Author

@heath-freenome I will play around with that solution and see what I can come up with!

Sure, I will see if I can make next weeks meeting!

@cwendtxealth
Copy link
Contributor Author

@heath-freenome just from some quick initial tests when anyOf/oneOf schema field is present not rendering the top field works. However in my opinion I think it makes more sense to keep the top common field and then have it removed from the anyOf/oneOf options schema. Just from a UI prospective that field will be the same across all anyOf/oneOf options so having at the top outside I think is a nice visual of the schema.

I wanted to get your thoughts on possibly that type of solution. Im not sure exactly the best way of solving that. Two solutions come to mind

  1. If the ui flag is set remove the top level properties before passing the anyOf/oneOf schema to the components
  2. if the ui flag is set, in the MultiSchemaField hide the top level property widget in the anyOf/oneOf component

@heath-freenome
Copy link
Member

Can you show me screenshots of what you have working now?

@cwendtxealth
Copy link
Contributor Author

Sure, I am using the example from this issue as it has a little more complexity to show.

So here is the view without any changes (current). The important field is Select Application Type as that controls all of the if then statements. And you can see the duplication of that field and Untitled Design
image

Here is the view when not rendering the top level field and only showing anyOf/oneOf component. This solves the duplicate fields but I don't like this option as much because of two things. One you don't see the top level title anymore Upload Design File and now its hard to really tell which field is the driving factor for changing the if/then schema. In the one above because its at the top if gives you a better visual of the hierarchy.
image

Here is one of the solutions I suggested, where I am removing the top level properties in the schema before passing the anyOf/oneOf schema. This to me is the best solution for visually showing the schema as it keeps the top level fields and title while not repeating them in the oneOf/anyOf options. The only thing I don't know is if there are any side effects from not passing the full schema with top level properties of the anyOf/oneOf schema.
image

cwendtxealth added a commit to cwendtxealth/react-jsonschema-form that referenced this issue Aug 11, 2023
cwendtxealth added a commit to cwendtxealth/react-jsonschema-form that referenced this issue Aug 24, 2023
Manish3323 pushed a commit to Manish3323/react-jsonschema-form that referenced this issue Aug 31, 2023
heath-freenome added a commit that referenced this issue Sep 8, 2023
* update antd version to v5 from v4

* update playground

* Keep peer dependency of ant to v4

* update package json of playground

* update changelog.md

* update docs for antd

* update docs for antd

* Update uiSchema.md

* incorporate feedback

* fix: Fixes #3808 Duplciate AnyOf/OneOf Description (#3841)

* Update package.json

---------

Co-authored-by: Christian Wendt <54559756+cwendtxealth@users.noreply.github.com>
Co-authored-by: Heath C <51679588+heath-freenome@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is a feature request help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants