-
Notifications
You must be signed in to change notification settings - Fork 373
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
Add a test for combinators and pass resolved subschema to createLabel #2165
Add a test for combinators and pass resolved subschema to createLabel #2165
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hi @sdirix do I need to do anything additional to get this PR reviewed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @DrewHoo,
Thanks for the contribution!
While this fixes the use case of the test case, another use case is now broken:
{
type: 'object',
properties: {
widget: {
anyOf: [
{
title: 'DuaOverride',
$ref: '#/definitions/Dua',
},
{
title: 'LipaOverride',
$ref: '#/definitions/Lipa',
},
],
},
},
definitions: {
Dua: {
title: 'Dua',
type: 'object',
properties: { name: { type: 'string' } },
},
Lipa: {
title: 'Lipa',
type: 'object',
properties: { name: { type: 'string' } },
},
},
};
DuaOverride
and LipaOverride
are no longer considered.
We should check both title
and use the outer one in case both exist.
@sdirix Thanks for the suggestion! I added the test case you described and merged the two schemas before passing them to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! one more small suggestion ;)
@DrewHoo Thanks a lot for the contribution ❤️ |
Resolves #2164