Fixing Issue - 1655 - Dynamic form should not display any validation errors until the user clicks Save/Submit, or leaves the focus of a required control

[NishkalankBezawada]

Q	A
Bug fix?	[x]
New feature?	[ ]
New sample?	[ ]
Related issues?	fixes #1655

What's in this Pull Request?

Fixing Issue #1655

Observation

When loading a Dynamic Form with no list item id (new item), the form should not display any validation errors until the user clicks Save/Submit, or leaves the focus of a required control.

Actual behavior

When loading a Dynamic Form with no list item id (new item) all of the required fields display validation errors by default.

Solution

In DynamicField.tsx component, individual field is set and the validation is done through the methods getRequiredErrorText() and getNumberErrorText()

These methods currently are only checking with changedValue property, which will be always empty for a new item. An additional check is done on basis of listItemId assuming that the fact that for new item, this will be empty, we can achieve to not to show the validation errors on the form.

Below is the current implementation of the code,

  private getRequiredErrorText = (): string => {
    const {
      changedValue
    } = this.state;
    return (changedValue === undefined || changedValue === '' || changedValue === null || this.isEmptyArray(changedValue)) && this.props.required ? strings.DynamicFormRequiredErrorMessage : null;
  }

neccesary changes done as below,

  private getRequiredErrorText = (): string => {
    const {
      changedValue,
      listItemId
    } = this.state;

    /**
     * checks if listItemId is not undefined, not an empty string, and not null.
     * If that condition is true happens when new item is loaded, it evaluates the nested ternary operator to determine the return value.
     * If the condition is false, it returns null.
     */

    return listItemId !== undefined && listItemId !== '' && listItemId !== null
      ? ((changedValue === undefined || changedValue === '' || changedValue === null || this.isEmptyArray(changedValue)) && this.props.required 
          ? strings.DynamicFormRequiredErrorMessage 
          : null)
      : null;

  }

Screenshots

Thanks,
Nishkalank Bezawada

[michaelmaillot]

Hi @NishkalankBezawada, thanks for this PR!

I've reviewed it and couldn't make required field to display required error message during blur & submit event for new items.

And after digging a bit about how the DynamicForm / DynamicField behave, it seems that the display of error messages for required fields is happening during the (re-)rendering process, which could happen for existing items but also for new ones.

I think that the display error messages for required fields should rely on the events which occur (blur & submit), whether you're working on an existing item or a new one.

[michaelmaillot]

Sorry @NishkalankBezawada, I've added my review a moth ago but forgot to submit them... 🤦‍♂️

[NishkalankBezawada]

Hey @michaelmaillot ,

Sorry for taking a lot of time. I have been away for almost a month. Hope that you would review the changes.

Thanks,
Nishkalank

[michaelmaillot]

Hi @NishkalankBezawada,

No worries! I'll have a look at your update and will come back to you asap!

[michaelmaillot]

Hi @NishkalankBezawada I currently can't test your updates since the ControlsTest webpart is not the latest one (there are still Toggle buttons displayed instead of Property Pane options).

See here for reference.

Let me know if something's not clear regarding this.

[michaelmaillot]

Hi @NishkalankBezawada, are you still working on this PR? Do you need any help regarding your branch update?

[NishkalankBezawada]

Hi @NishkalankBezawada, are you still working on this PR? Do you need any help regarding your branch update?

Hey @michaelmaillot, I was held up with other work and couldnt find time. I will do this evening, and keep you posted :)

//Nishkalank

[NishkalankBezawada]

Hi @NishkalankBezawada, are you still working on this PR? Do you need any help regarding your branch update?

Unable to run gulp build or gulp serve on dev branch, Erroring out as below,