fix how we handle empty values #2481
Conversation
…tionalProperties case)
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
kchobantonov
changed the title
|
@sdirix please review - when we update the handling of empty values it looks like deleting (not using the x - clear ) was setting the value to empty string rather to remove the property when we are not under dynamic context (e.g. additionalProperties where we need to preserve the key) |
sdirix
left a comment
sdirix
left a comment
There was a problem hiding this comment.
Changes make sense to me in general. However there are a number of "non-changes" in the PR. Also I just noticed that we might have an overall enum issue.
packages/vue-vuetify/src/extended/AutocompleteEnumControlRenderer.vue
Outdated
Show resolved
Hide resolved
packages/vue-vuetify/src/extended/AutocompleteOneOfEnumControlRenderer.vue
Outdated
Show resolved
Hide resolved
packages/vue-vuetify/src/extended/AutocompleteEnumControlRenderer.vue
Outdated
Show resolved
Hide resolved
|
@sdirix it is ready to be reviewed again - check my comments regarding the null value cases. |
lucas-koehler
force-pushed
the
master
branch
4 times, most recently
from
24c8cb2 to
d86047e
Compare
lucas-koehler
left a comment
lucas-koehler
left a comment
There was a problem hiding this comment.
Hi @kchobantonov , thanks for the updates. The code looks pretty good to me. However, it seems to break something with the enum value display. See the video below. On master the same text is shown in the dropdown and the enum renderer's text field. Please have a look
jf-vuetify-enums.webm
looks like adding :return-object="true" breaks how the component shows the value and the reason why we have added that was to be able to distinguish when the value was cleared vs when the actual enum value is null since if we do not use the item but the value of the item in the original case we can't be sure if the value null represents an item with value null vs when the item is not selected. Will look more into that but at this point looks like some strange behavior with the vuetify component when return-object is used. |
|
will revert the return-object change and will wait until this issue with vuetify vuetifyjs/vuetify#22126 is fixed to change the comparison for the null value to perhaps undefined |
|
@lucas-koehler thanks for the report - please review again |
lucas-koehler
left a comment
lucas-koehler
left a comment
There was a problem hiding this comment.
@kchobantonov Thanks for the update! The enum controls in the example work as expected again. As stated inline, we still do not handle the problem of valid null values. However, this is the same as without this PR. Thus, I'll merge this and this problem can be solved in a follow up if needed.
| return useVuetifyControl(useJsonFormsOneOfEnumControl(props), (value) => | ||
| value !== null ? value : undefined, | ||
| value === null ? clearValue : value, |
There was a problem hiding this comment.
We still have the problem of this not working with values that are null but should not be cleared. However, as this was also the case before this PR, this is fine for me and could potentially be fixed in a follow up.
4 participants
fix how we handle empty values when not in dynamic context (e.g. addtionalProperties case)