Added form readonly prop (#2553) #2554
Conversation
|
@epicfaace small feature that I could really use... I know I have a work-around with |
|
@epicfaace Anyone besides yourself who can review/approve? |
|
@newt10 also has write / reviewer access |
|
|
||
| render(( | ||
| <Form schema={schema} | ||
| readonly /> |
There was a problem hiding this comment.
Do you think it's better to call this "readOnly", for consistency with the readOnly react prop?
There was a problem hiding this comment.
Given that all the fields and widgets use readonly, I'd rather keep it consistent within the project. Perhaps we can deprecate readonly in favor of readOnly in the future?
There was a problem hiding this comment.
Ok, sounds good. I'd note that we did already deprecate autocomplete in favor of autoComplete (see https://github.com/rjsf-team/react-jsonschema-form/blob/master/docs/api-reference/form-props.md#autocomplete), but I guess we don't have an analogous situation there (because autoComplete is not passed down as props to fields / widgets)
There was a problem hiding this comment.
Can you update types?
Done!
Updated the playground to add the `Readonly whole form` flag Added the `form-readonly` section in the documentation Updated the tests to verify the `read-only` status
heath-freenome
force-pushed
the
add-form-level-readonly
branch
from
6d79170
to
0ed2f2f
Compare
I rebased locally then force pushed... after adding the |
| expect(node.querySelectorAll("input:read-only")).to.have.length.of(0); | ||
| }); | ||
|
|
||
| it("should readonly all items", () => { |
There was a problem hiding this comment.
Can we add a test just to ensure it works on nested objects / arrays?
There was a problem hiding this comment.
Sure... I originally followed the pattern for the disabled tests above. Should those also be updated?
There was a problem hiding this comment.
Updated the one test to use a nested object
There was a problem hiding this comment.
The playground works with arrays
|
Yeah, probably, but maybe not necessary in this PR?
…On Fri, Sep 24, 2021, 6:40 PM Heath C ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/core/test/Form_test.js
<#2554 (comment)>
:
> + const schema = {
+ type: "object",
+ properties: {
+ foo: { type: "string" },
+ bar: { type: "string" },
+ },
+ };
+ const formData = { foo: "foo", bar: "bar" };
+
+ it("should not have any readonly items", () => {
+ const { node } = createFormComponent({ schema, formData });
+
+ expect(node.querySelectorAll("input:read-only")).to.have.length.of(0);
+ });
+
+ it("should readonly all items", () => {
Sure... I originally followed the pattern for the disabled tests above.
Should those also be updated?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2554 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM4MXZYHN3LTHYZ72RSOH3UDT45FANCNFSM5EISWOIA>
.
|
Thanks for the good review... when are you anticipating releasing the build? |
|
Probably sometime this month. The changelog process should make it much
easier / faster :)
…--
Ashwin Ramaswami
On Fri, Sep 24, 2021 at 8:57 PM Heath C ***@***.***> wrote:
Looks good, thanks!
Thanks for the good review... when are you anticipating releasing the
build?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2554 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM4MXYMS5DWKWDJGCUYRJDUDUM53ANCNFSM5EISWOIA>
.
|
|
Good. But I must complain it that I've done the same pr months ago without any feedback. #2388 Did I miss something which is a rule that I should create a issue for it or mention someone to review? |
|
@Icemic apologies, I haven't really kept track of all PRs as there's too much, so I probably didn't notice it. In the future, if you don't hear back about a PR review, feel free to ping me. thanks for your other PR! |
Reasons for making this change
fixes #2553
Updated the playground to add the
Readonly whole formflagAdded the
form-readonlysection in the documentationUpdated the tests to verify the
read-onlystatusChecklist