fix how we handle empty values by kchobantonov · Pull Request #2481 · eclipsesource/jsonforms
kchobantonov
changed the title
fix how we handle empty values when not in dynamic context (e.g. addi…
fix how we handle empty values
@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)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@sdirix it is ready to be reviewed again - check my comments regarding the null value cases.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters