Rendering the form of a model with many different complex inputs in React.js

Question

I have a React component that renders the form of a model (called riskModel). This form is used in two different context; for creation and edition of a specific model.

In this form, there are many different complex inputs, such as:

• Dynamic graph sections that include a .json file of a specific format, a title and a description
• Graph section that include an image of type .jpeg or .png, a title, a description, and a display type
• File section that include a file of any format (xlsx, pdf, image, ...) up to 3Go maximum
• The model name, type, and description

And there can be any number of each input (it's up to the user to add or remove a section). I have moved into other components the dynamic graph section, graph section and file section (respectively DynamicGraphSectionsInputs, GraphSectionInputs and FileInputs).

However, the rendering method gets heavy to read, to maintain and to understand. Is there some good practices to follow to make it easier to read, to maintain and to understand?

Here is the render() method:

render () {
 const { riskModel } = this.state;
 const { loading } = this.state;
 const initialDataLoaded = this.props.initialDataLoaded;
 return (
 <div>
 {loading || (typeof initialDataLoaded !== 'undefined' && !initialDataLoaded)
 ? <LoaderComponent />
 : (
 <Form>
 <Card className="model-form-container">
 <Grid columns={16} className="project-upload" style={{ margin: 10 }}>
 <Grid.Row style={{ paddingTop: 0, paddingBottom: 10 }}>
 <div style={{ width: '100%' }}>
 <p className="section-title">Model Name</p>
 <Input
 data-testid="model-title"
 className="input-model"
 placeholder="name of the model"
 onChange={(e) => this.handleFormChange('label', e.target.value)}
 value={riskModel.label}
 />
 </div>
 </Grid.Row>
 <Grid.Row style={{ paddingTop: 10, paddingBottom: 0 }}>
 <div style={{ width: '100%' }}>
 <p className="section-title">Model Type</p>
 <Dropdown
 className="input-model"
 data-testid="model-type-dropdown"
 placeholder="Type of model"
 search
 selection
 options={this.props.modelTypeOptions}
 onChange={(e, data) => this.handleDropdownChange('type', data)}
 value={riskModel.type}
 />
 </div>
 </Grid.Row>
 <Grid.Row>
 <Grid.Column computer={16} style={{ paddingRight: 0, paddingLeft: 0 }}>
 <Wysiwyg
 description={riskModel.interpretation}
 handleChange={(description) => this.handleFormChange('interpretation', description)}
 />
 </Grid.Column>
 </Grid.Row>
 <DynamicGraphSectionsInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 />
 <GraphSectionInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 />
 <FileInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 documentsToDelete={this.state.documentsToDelete}
 setDocumentsToDelete={(documents) => this.setState({ documentsToDelete: documents })}
 />
 <Grid.Row>
 <Grid.Column computer={16} style={{ paddingRight: 0, paddingLeft: 0 }}>
 <div className="creation-form__button">
 <Button
 id="cancel-risk-model"
 className="button-cancel"
 data-testid="button-cancel"
 onClick={() => this.props.handleCancel()}
 >
 Cancel
 </Button>
 <Button
 id="submit-risk-model"
 data-testid="submit-risk-model-button"
 disabled={!RiskModelForm.isFormValid(riskModel)}
 loading={loading}
 onClick={() => this.handleSubmit()}
 >
 Save
 </Button>
 </div>
 </Grid.Column>
 </Grid.Row>
 </Grid>
 </Card>
 </Form>
 )}
 </div>
 );
 }

Answer 1

Separate style out into the CSS if you want - this will de-clutter the JSX code a bit, allowing it to focus primarily on content and logic rather than style and presentation. For example, it'd be nice if you could change this:

<Grid columns={16} className="project-upload" style={{ margin: 10 }}>
 <Grid.Row style={{ paddingTop: 0, paddingBottom: 10 }}>

to

<Grid columns={16} className="project-upload">
 <Grid.Row>

Destructuring improvements This:

const { riskModel } = this.state;
const { loading } = this.state;
const initialDataLoaded = this.props;

can be

const { riskModel, loading } = this.state;
const { initialDataLoaded, handleCancel } = this.props;
// destructure everything you ever use from props in the line above
// so you don't have to go through props later

initialDataLoaded? This line is a bit suspicious:

loading || (typeof initialDataLoaded !== 'undefined' && !initialDataLoaded)

A typeof check shouldn't be necessary - unless you're dealing with a very unpredictable codebase where an undefined identifier may well actually not refer to undefined, comparing against undefined directly would be a bit nicer:

loading || (initialDataLoaded !== undefined && !initialDataLoaded)

But do you really need both of those checks? Might you, for example, just do loading || !initialDataLoaded?

If you do need to separately check that it's defined and falsey, consider comparing directly against the falsey value it'll be instead. (Hopefully there won't be multiple non-undefined falsey values, that'd be pretty weird)

loading || initialDataLoaded === ''

Separate out larger components if you want to make the overall structure easier to understand at a glance. For example, you might want to refactor to something like this:

return (
 <div>
 {loading || (typeof initialDataLoaded !== 'undefined' && !initialDataLoaded)
 ? <LoaderComponent />
 : <ModalForm /* add in needed props */ />
 </div>
);

// ModalForm.jsx render:
<Form>
 <Card className="model-form-container">
 <Grid columns={16} className="project-upload">
 <ModelNameRow {...{ riskModel, handleFormChange }} />
 <ModelTypeRow {...{ modelTypeOptions, handleDropdownChange }} />
 <Grid.Row>
 <Grid.Column computer={16} style={{ paddingRight: 0, paddingLeft: 0 }}>
 <Wysiwyg
 description={riskModel.interpretation}
 handleChange={(description) => this.handleFormChange('interpretation', description)}
 />
 </Grid.Column>
 </Grid.Row>
 <DynamicGraphSectionsInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 />
 <GraphSectionInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 />
 <FileInputs
 riskModel={riskModel}
 setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}
 documentsToDelete={this.state.documentsToDelete}
 setDocumentsToDelete={(documents) => this.setState({ documentsToDelete: documents })}
 />
 <RiskModelButtons {...{ handleCancel, handleSubmit }} />
 </Grid>
 </Card>
</Form>

Maybe you'd want to abstract away the Inputs as well, or separate different sections - that's the general idea, it's up to you.

Functional components will make handlers a bit easier, and are generally recommended over class components by React for new code. For example, it'd be nice to change

setRiskModel={(nextRiskModel) => this.setState({ riskModel: nextRiskModel })}

to

setRiskModel={setRiskModel}

Or, even better:

Spreading props can reduce syntax noise, as you might've noticed I used above. Using functional components, additional destructuring of documentsToDelete, and spread will mean that the 3 Inputs can be made to look like:

<DynamicGraphSectionsInputs {...{ riskModel, setRiskModel }} />
<GraphSectionInputs {...{ riskModel, setRiskModel }} />
<FileInputs {...{ riskModel, setRiskModel, documentsToDelete, setDocumentsToDelete }} />

Avoid unnecessary anonymous callbacks Unless handleCancel or handleSubmit are doing something funny with conditional arguments, these:

onClick={() => this.props.handleCancel()}

onClick={() => this.handleSubmit()}

can be

onClick={handleCancel} // destructure this in the beginning alongside initialDataLoaded

onClick={handleSubmit} // destructure like above, or use functional component

If you're still using a class component and the anonymous wrapper is needed for the proper this calling context, define the methods using class fields instead so that this will point to the instance regardless of where the method is invoked - eg, change:

handleSubmit() {
 // ...
}

to

handleSubmit = () => {
 // ...
}

• Orlyyn: Wow, thank you for all your tips and details, they'e truly helpful !! I've tried to change my class component into a functional component several times, but each time something wrong came up, so I had to rollback... Thanks again, and I bid you a good day :)