sp-dev-fx-controls-react icon indicating copy to clipboard operation
sp-dev-fx-controls-react copied to clipboard

Dynamic Form Loads with Validation Errors if no List Item Id is set

Open Paul-Colucci opened this issue 1 year ago • 9 comments

[ ] Enhancement

[X ] Bug

[ ] Question

Version

Please specify what version of the library you are using: [3.15.0]

Expected / Desired Behavior / Question

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.

Observed Behavior

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

Steps to Reproduce

Create A Dynamic Form with no list item id set, and link it to a list that has a required field. You will see the errors load by default.

You can see this error reported by @sallaadithya in a sub comment for an existing bug here: https://github.com/pnp/sp-dev-fx-controls-react/issues/1586#issuecomment-1637459697

Paul-Colucci avatar Sep 25 '23 17:09 Paul-Colucci

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

ghost avatar Sep 25 '23 17:09 ghost

Thank you for submitting your first issue to this project.

github-actions[bot] avatar Sep 25 '23 17:09 github-actions[bot]

Hey @Paul-Colucci

Can you try the same scenario with Beta version of the controls?

Thanks, Nishkalank

NishkalankBezawada avatar Sep 26 '23 11:09 NishkalankBezawada

@NishkalankBezawada Yes it is still happening in the beta: 3.16.0-beta.6304745

Paul-Colucci avatar Sep 26 '23 16:09 Paul-Colucci

Hello @Paul-Colucci

I'm working on it, will be creating a PR soon.

@michaelmaillot could you assign this to me.

Thanks, Nishkalank

NishkalankBezawada avatar Sep 28 '23 14:09 NishkalankBezawada

Hello @Paul-Colucci,

Below is my solution. I have created a draft PR on this, as I still need to run a few more checks. Will keep you updated once I submit the PR soon.

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

Issue-1655

Thanks, Nishkalank Bezawada

NishkalankBezawada avatar Sep 28 '23 16:09 NishkalankBezawada

@NishkalankBezawada great. Let me know when it is ready for testing and I'll re-install the latest beta and try it out

Paul-Colucci avatar Oct 04 '23 14:10 Paul-Colucci

@NishkalankBezawada great. Let me know when it is ready for testing and I'll re-install the latest beta and try it out

Hey @Paul-Colucci

I would definitely let you know once this PR is merged.

Hey @michaelmaillot, kindly review this once you have time 😇

Thanks, Nishkalank

NishkalankBezawada avatar Oct 04 '23 16:10 NishkalankBezawada

Hi @NishkalankBezawada,

Excellent work! Sure, I'll have a look soon 😉

michaelmaillot avatar Oct 05 '23 06:10 michaelmaillot