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

Support readOnly property from latest validation draft #888

Closed
wants to merge 1 commit into from

Conversation

norpan
Copy link

@norpan norpan commented Apr 9, 2018

Reasons for making this change

Since the readOnly property now is available in the latest validation draft (previously it was in hyper schema only), it makes sense to use this property to signal read only for fields.

Fixes #976

Checklist

  • I'm updating documentation
    • I've checked the rendering of the Markdown text I've added
    • If I'm adding a new section, I've updated the Table of Content
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
    • I've run npm run cs-format on my branch to conform my code to prettier coding style
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@epicfaace
Copy link
Member

Thanks for the PR, @norpan , can you please update the documentation in your PR as well?

@handrews , I have a question, how does the json schema draft process work -- is draft-07 finalized and won't change? Since readOnly is only in the the latest version of draft-07, I'm wondering if there's a chance that readOnly will be removed by the time draft-07 is finalized. (This is because in the past, react-jsonschema-form implemented enumNames but that move ended up being too early as enumNames was never actually finalized in jsonschema.)

@handrews
Copy link

@epicfaace draft-07 is published and will not change. While technically there were two sets of IETF documents referred to as "draft-07" (draft-handrews-json-schema*-00 and draft-handrews-json-schema*-01) there were no differences in implementation requirements. Which is why we kept "draft-07" for both- the -01 documents were just bugfixes and clarifications.

The whole thing where meta-schemas are called "draft-NN" and IETF documents are "draft-author-topic-NN" where the two values of NN don't match is way too confusing, so we will be moving to date-based meta-schema identifiers for the next draft. So instead of "draft-08", draft-handrews-json-schema*-02 will correspond to "draft/2019-02" or something like that.

@epicfaace
Copy link
Member

@norpan will you be able to update the doc? Once you do that, I can merge this PR.

@epicfaace
Copy link
Member

@norpan , just bumping this -- will you be able to update the docs?

@pahen
Copy link
Contributor

pahen commented May 13, 2019

I would love to see this merged very soon since I'm desperately in need of it.

@pahen
Copy link
Contributor

pahen commented May 13, 2019

I've rebased @norpan changes against the master branch and fixed one failing test and updated docs in this PR now: #1282

@@ -168,7 +168,9 @@ function SchemaFieldRender(props) {
const FieldComponent = getFieldComponent(schema, uiSchema, idSchema, fields);
const { DescriptionField } = fields;
const disabled = Boolean(props.disabled || uiSchema["ui:disabled"]);
const readonly = Boolean(props.readonly || uiSchema["ui:readonly"]);
const readonly = Boolean(
props.readonly || uiSchema["ui:readonly"] || schema["readOnly"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
props.readonly || uiSchema["ui:readonly"] || schema["readOnly"]
props.readonly || uiSchema["ui:readonly"] || props.schema.readOnly || schema.readOnly

To be consistent with the changes done in 497620f

@pahen
Copy link
Contributor

pahen commented May 14, 2019

This can be closed now @epicfaace

@epicfaace
Copy link
Member

Yes. Closing because of PR #1282

@epicfaace epicfaace closed this May 14, 2019
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.

Don't work "readOnly" attribute from Schema
4 participants