Regression of #1168: Empty string should be undefined

[bennbollay]

Describe the bug

Changes made in #1168 were lost at some point, resulting in empty strings satisfying "required" validation checks.

Expected behavior

Manually deleting the contents of a required field will generate a validation error.

Clearing a field via the [X] button will also generate a validation error.

Steps to reproduce the issue

1. Download the seed app
2. Add "description" to the "required" clause in src/schema.json
3. Start the seed app
4. Clear the 'description' field by deleting all of the characters.
   • Note: no red error text generated.
   • Note: Data rendering on the left shows that the description has an "empty string" value.
5. Press the [X] button to force-clear the MaterialUI field.
   • Note: red error text is now generated.
   • Note: Data rendering on the left shows that no description parameter is present on the data object.

Screenshots

No response

In which browser are you experiencing the issue?

Version 1.29.81 Chromium: 93.0.4577.82 (Official Build) (arm64)

Framework

React

RendererSet

Material

Additional context

This was reproduced on the 3.0.0 alpha-1 release as well as 2.5.1.

[sdirix]

Hi @bennbollay! Thanks for the report. However the reported issue is not really a regression but a design decision. The mentioned #1168 PR introduced the "empty field => undefined" behavior but it turned out to not be flexible enough. This is why we later introduced the additional "clear" button to distinguish between empty string and undefined.

If an empty string should not be allowed in your data set I would recommend adding a "minLength": 1 attribute to your string property in JSON Schema. Then an empty string input will also show a validation error.

If you would like to overwrite the current behavior of JSON Forms you need to provide a custom renderer. That should be straightforward. You can simply reuse the existing renderer and just overwrite the handleChange method to also place undefined in the data when the handed over string is empty.

[bennbollay]

The "minLength": 1 attribute, however, generates entirely the wrong type of error message - that the length is insufficient, not that the field is required.

In the <JsonForms handleChange> property, changes to the passed in data do not impact the form validation. Is there a handler we can hook on to do custom changes like this across the entire form that will generate errors in the validation phase?

If a small example of a custom renderer that does this is available, it would be hugely useful here for the next traveler to come along this issue.

[sdirix]

The custom renderer should look somewhat like this (untested).

import { Unwrapped } from '@jsonforms/material-renderers`;

const { MaterialTextControl } = Unwrapped;

const MyTextControl = (props: ControlProps) => {
  const myHandleChange = useCallback((path: string, newValue: any) => {
    if (newValue === '') {
      props.handleChange(path, undefined);
    } else {
      props.handleChange(path, newValue);
    }
  }, [props.handleChange]);
  return <MaterialTextControl {...props} handleChange={myHandleChange} />;
}

export default withJsonFormsControlProps(MyTextControl);

[collinr3]

Whilst noting the suggested custom renderer option, I have observed, during user testing, that the out of the box option of not setting empty string to undefined, creates a usability problem on desktop and tablet, where the instinctive way of clearing the field is from the keyboard using the delete key. I did not observe one user instinctively switch to use the X option.

Could further consideration be given to reinstating this behaviour in such a way that you retain the flexibility also.

[sdirix]

@collinr3 Very interesting!. Are there any other user testing observations you made which apply to JSON Forms?

If you need this behavior right now I would recommend using a custom renderer. As shown above it's only about 15 lines of code, so it's definitely feasible to add this to any renderer set.

[collinr3]

@sdirix agreed, that is what we will likely do for now.

The one other significant observation was that the 'required property' message doesn't resonate with end users - typically they expect to be told that a 'field is required', so a simple way of changing that message would do a lot for our user acceptance. If it helps, I'm happy to raise that as a separate issue for tracking purposes.

[sdirix]

Hi @collinr3, up until recently the way to customize AJV messages was by customizing the AJV instance. For this you could either use ajv-errors (Note that we used AJV v6 up until now) or you could wrap AJV yourself and hand over a wrapped version to JSON Forms. In it you could modify the error (messages) before handing them over to JSON Forms (basically a custom ajv-errors), see this example.

However we just recently released 3.0.0-alpha.2 which contains an update to AJV 8 and a newly integrated i18n support. This allows you to hand over your own translate function to JSON Forms. JSON Forms will request a message for the key error.required for the required errors. So by handing in your own translate function this can now also be customized. As a second parameter the translate function receives the default message which JSON Forms intends to show, so if you return that in all other cases the remaining strings in JSON Forms will be shown as they are now.

[collinr3]

@sdirix Thank you, we will give that a try.

[Ketec]

Came upon the same issue now with angular. And since the controls I'm using it for do not have any x/clear like material - required fields are always valid.
HAs there been any progress on making this configurable or a way to verride this behaviour?

[sdirix]

Hi @Ketec, this behavior can always be overridden by registering a custom renderer for string controls.

[Ketec]

Problem is - setting the getEventValue result to null results in "must be a string".
Setting it to undefined shows required - but also now prefills the input with "undefined" as text/value.

[sdirix]

We changed the default to remove the string attribute when the input is empty with #1984. The previous behavior of storing an empty string can be restored using a custom renderer.