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

Adding support for $ref pointers to additionalProperties. #1402

Merged
merged 4 commits into from
Aug 22, 2019
Merged

Adding support for $ref pointers to additionalProperties. #1402

merged 4 commits into from
Aug 22, 2019

Conversation

erunion
Copy link
Collaborator

@erunion erunion commented Aug 12, 2019

Reasons for making this change

This adds support for $ref pointers on additionalProperties objects.

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! Can you also add a test in Form_test.js, so that the logic you added in ObjectField.js is covered?

@erunion
Copy link
Collaborator Author

erunion commented Aug 14, 2019

@epicfaace Done!

@epicfaace
Copy link
Member

epicfaace commented Aug 15, 2019

By the way, here's a playground link to an example that doesn't work currently on the main playground using $ref with additionalProperties. (Testing it on the deploy preview works fine!)

src/components/fields/ObjectField.js Outdated Show resolved Hide resolved
src/components/fields/ObjectField.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
@epicfaace
Copy link
Member

Thanks!

@jschaufele
Copy link

I just forked this and found there is an issue w monaco-editor-webpack-plugin version - took the '^' out and it compiled - I am new to contributing hopefully this is the correct format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants