Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dynamic Form] Disable issue on "fieldOverrides" field control when "onBeforeSubmit" return true #1715

Closed
wuxiaojun514 opened this issue Nov 29, 2023 · 4 comments · Fixed by #1757
Assignees
Labels
status:fixed Issue fixed in current or prior release. type:bug
Milestone

Comments

@wuxiaojun514
Copy link
Contributor

Category

[ ] Enhancement

[x] Bug

[ ] Question

Version

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

This issue also occurs on old version like 3.14

Expected / Desired Behavior / Question

I am using DynamicForm control and created several customized rendered fields with "fieldOverrides".
I also put validation logic (e.g. "start date" should be before "end date") in "onBeforeSubmit" method.
Then I found that if the validation failed (onBeforeSubmit return true), then my customized fields will always be disabled in the form, user cannot modify them. However user can still modify the fields which are not mentioned in "fieldOverrides".

image

Steps to Reproduce

  1. Create a spfx form customizer extension
  2. Add Dynamic Form control in it (like TestForm )
  3. Create a customized render field control method ( I copied most of the content from DynamicField.tsx )

I used disabled={disabled} in TextField Control

private renderTextField = (fieldProperties: IDynamicFieldProps): React.ReactElement<IDynamicFieldProps> => {

    
    const {
      options,
      fieldTermSetId,
      fieldAnchorId,
      lookupListID,
      lookupField,
      fieldType,
      fieldDefaultValue,
      fieldTitle,
      context,
      disabled,
      label,
      required,
      isRichText,
      //bingAPIKey,
      dateFormat,
      firstDayOfWeek,
      columnInternalName,
      principalType,
      description
  } = fieldProperties;

  const labelText = fieldTitle !== null ? fieldTitle : label;
  const defaultValue = fieldDefaultValue;
  const isRequired= fieldProperties.required ? fieldProperties.required:required;

  const labelEl = <label className={(isRequired) ? styles.fieldRequired + ' ' + styles.fieldLabel : styles.fieldLabel}>{labelText}</label>;
  const descriptionEl = <text className={styles.fieldDescription}>{description}</text>;
  const placeHolder=fieldProperties.placeholder;

    return (
      <div>
      <div className={styles.titleContainer}>
        <Icon className={styles.fieldIcon} iconName={"TextField"} />
        {labelEl}
      </div>

      <TextField
        defaultValue={defaultValue}
        placeholder={placeHolder}
        className={styles.fieldDisplay}


        onChange={(e, newText) => { (newText: any): void => { 
          const {
            onChanged,
            columnInternalName
          } = fieldProperties;
      
          if (onChanged) {
            onChanged(columnInternalName, newText);
          }
          this.setState({
            changedValue: newText
          });
        } }}
        disabled={disabled}

      />
      {descriptionEl}
    </div>
    )
  }

Then use this renderTextField method on DynamicForm and let onBeforeSubmit return true (validation failed)

 <DynamicForm
          context={this.props.context}
          listId={this.props.context.list.guid.toString()}
          listItemId={this.props.context.itemId}
          fieldOverrides={{
            "JobTitle": this.renderTextField  //use any text field
          }}

          onBeforeSubmit={
            async (listItem) => { 
              alert("validation failed!");
              return true; 
            }
          }
      />

This demo field (Job Title in my example) will be read-only after submit button clicked while the other fields still be editable

image

If your form has a lot fields, then user has to refresh page and fill in again after validation failed

My Trouble shooting

I did some trouble shooting on dynamic form source code.

I found that it will modify "fieldOverrides" fields's disable property when it is isSaving however it won't change "DynamicField"'s disable property. So the those "Dynamic" fields would still use its original "disable" property

image

Right now my temporary solution is not use disabled={disabled} in customized field control.
Hope we can find a better solution.

@ghost
Copy link

ghost commented Nov 29, 2023

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

@ghost ghost added the Needs: Triage 🔍 label Nov 29, 2023
@wuxiaojun514
Copy link
Contributor Author

I found a solution, I replaced

 v.disabled = v.disabled || isSaving;
 return fieldOverrides[v.columnInternalName](v);

with
return fieldOverrides[v.columnInternalName]({ ...v,disabled: v.disabled || isSaving} )

,then it works well.
I will submit a PR for this hotfix.

@joelfmrodrigues
Copy link
Collaborator

Thanks @wuxiaojun514 ! I will assign this issue to you so no one else works on it

wuxiaojun514 pushed a commit to wuxiaojun514/sp-dev-fx-controls-react that referenced this issue Dec 4, 2023
@wuxiaojun514 wuxiaojun514 mentioned this issue Dec 4, 2023
@wuxiaojun514
Copy link
Contributor Author

Hi @joelfmrodrigues ,
I have created the PR #1716 for it.
I downloaded the latest code and tested it.

@joelfmrodrigues joelfmrodrigues added the status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. label Jan 30, 2024
@joelfmrodrigues joelfmrodrigues added the status:fixed-next-drop Issue will be fixed in upcoming release. label Jan 30, 2024
@joelfmrodrigues joelfmrodrigues added this to the 3.17.0 milestone Jan 30, 2024
@joelfmrodrigues joelfmrodrigues mentioned this issue Feb 5, 2024
@joelfmrodrigues joelfmrodrigues added status:fixed Issue fixed in current or prior release. and removed status:fixed-next-drop Issue will be fixed in upcoming release. status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. labels Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:fixed Issue fixed in current or prior release. type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants