-
Notifications
You must be signed in to change notification settings - Fork 175
Store initial form state to post new fields #4830
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
base: main
Are you sure you want to change the base?
Store initial form state to post new fields #4830
Conversation
- Changed omitExtraData from true to false in ResourceForm to ensure empty string values are included in form submissions - Added comprehensive unit tests for removeReadOnlyProps function to validate empty value handling - Updated CHANGELOG.md with bug fix entry Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
- Addressed code review feedback by removing unreachable duplicate case Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
This alongside previous form changes means that we will be able to send empty-but-changed values in forms
|
@matthew-a-dunlap how about I close the copilot generated PR and you target this one at main? I can review/approve it that way, but can't if I created it. :) You good with that? |
|
@marrobi Sounds great! |
|
@matthew-a-dunlap k, let us know when ready to review. |
JC-wk
left a comment
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.
I think this is an rjsf issue rather than a TRE one.
see rjsf-team/react-jsonschema-form#3728 (comment)
I'm not convinced this pr is required, can you confirm if you've tried this?
You can use the uiSchema option ui:emptyValue to ensure the empty case falls back to the empty string instead of removing it from the formData.
Happy to discuss and test different options but I think there may be more elegant solutions.
There are quite a number of issues on rjsf github discussing this situation
|
@JC-wk Thanks James for the thoughts. I'm definitely very naive and would love to discuss. My instinct is to find a solution that works for all forms and all field types, to ensure fields that go from a value to empty during an update are always passed. Setting a default value for each field in each template seems cumbersome. I think this approach utilizing |
|
Hi @matthew-a-dunlap It looks like if you set uiSchema to have ui:emptyValue = "" everywhere then you'd run in to this issue rjsf-team/react-jsonschema-form#605 where it overrides a field being required so I think you could be best of just adding this to the templates where you think it is needed (i.e you definitely want to allow something that has a value to be set to blank) I don't think this is an issue for most TRE users and this seems to solve it acceptably for your use case. |
This alongside previous form changes means that we will be able to send empty-but-changed values in forms