1
\$\begingroup\$

I was recently building a form to add contacts to a play app I'm making and I finally got it to work. The problem is I'm not too happy with it.

It's my first time building an application with React, so I don't really have any experience on my toolbelt, but I can see the responsabilities aren't too well sorted out. I can imagine in an ideal scenario I would have smaller components, but I can't see how to break this down any further.

If you guys can correct me overall good/bad practices when coding with JS/React and also suggest how I could further break down the form into smaller pieces, I would be supergrateful!

Just as a heads up, I'm trying not to use Redux or Flux. Just plain JS and React. I wanted to learn these two properly before adding more pieces to the puzzle.

I'll attach the two main components involved in the application, as well as a simplified container for them, just so you guys can simply copy-paste it to get it working :)

This component represents a simple form field.

import React, { PureComponent } from 'react';
import { FormGroup, ControlLabel, FormControl, HelpBlock } from 'react-bootstrap';

export class FormField extends PureComponent {

    constructor(props) {
        super(props);
        this.state = { value: '' };
    }

    render() {
        return (
            <FormGroup controlId={this.props.id}>
                <ControlLabel>{this.props.label}</ControlLabel>
                <FormControl {...this.props} onChange={this.props.handleChange}  />
                {this.props.help && <HelpBlock>{this.props.help}</HelpBlock>}
            </FormGroup>
        );
    }
}

But then, the actual form looks like this:

import React, { Component } from 'react';
import { Modal, Glyphicon, Form, FormGroup, Col, Row, Button } from 'react-bootstrap';
import { FormField } from '../common/FormField';

export default class NewContactModal extends Component {
    constructor(props, context) {
        super(props, context);

        this.handleClose = this.handleClose.bind(this);

        this.state = {
            show: this.props.show,
            name: '',
            email: '',
            username: ''
        };
    }

    handleClick = () => {
        var opts = {
            method: 'POST',
            body: JSON.stringify({
                name: this.state.name,
                username: this.state.username,
                email: this.state.email
            }),
            headers: {
                "Content-Type": "application/json"
            }
        };

        fetch('api/contacts', opts)
            .then(response => response.json())
            .then(data => {
                console.log('Added!')
            });
    }

    handleNameChange = event => {
        this.setState({
            name: event.target.value,
        });
    };

    handleUsernameChange = event => {
        this.setState({
            username: event.target.value,
        });
    };

    handleEmailChange = event => {
        this.setState({
            email: event.target.value,
        });
    };

    handleClose() {
        this.setState({ show: false });
    }

    componentWillReceiveProps(nextProps) {
        // You don't have to do this check first, but it can help prevent an unneeded render
        if (nextProps.show !== this.state.show) {
            this.setState({ show: nextProps.show });
        }
    }

    render() {
        return (
            <div>
                <Modal show={this.state.show} onHide={this.handleClose}>
                    <Form horizontal>
                        <Modal.Header closeButton>
                            <Modal.Title>
                                <Glyphicon glyph='user' /> New contact
                            </Modal.Title>
                        </Modal.Header>
                        <Modal.Body className="mx-auto" style={{ margin: '20px' }}>
                            <Row>
                                <Col md={5}>
                                    <FormField
                                        id="formName"
                                        label="Name"
                                        placeholder="Name"
                                        handleChange={this.handleNameChange} />
                                </Col>
                                <Col md={5} mdOffset={2}>
                                    <FormField
                                        id="formUsername"
                                        label="Username"
                                        placeholder="Username"
                                        handleChange={this.handleUsernameChange} />
                                </Col>
                            </Row>
                            <Row>
                                <Col md={5}>
                                    <FormField
                                        id="formEmail"
                                        label="Email"
                                        placeholder="Email"
                                        type="email"
                                        handleChange={this.handleEmailChange} />
                                </Col>
                                <Col md={5} mdOffset={2}>
                                    <FormField
                                        id="formGithub"
                                        label="Github"
                                        placeholder="Github"
                                        handleChange={this.handleUsernameChange} />
                                </Col>
                            </Row>
                        </Modal.Body>
                        <Modal.Footer>
                            <FormGroup style={{ 'margin-right': '5px' }}>
                                <Button bsStyle="primary" type="submit" onClick={(evt) => this.handleClick()}>Save</Button>
                            </FormGroup>
                        </Modal.Footer>
                    </Form>
                </Modal>
            </div>
        );
    }
}

Here are the relevant parts of the container code:

import React, { Component } from 'react';
import { Button } from 'react-bootstrap';
import NewContactModal from './NewContactModal';


export class ContainerComponent extends Component {
    displayName = ContainerComponent.name

    constructor(props) {
        super(props);

        this.state = {
            show: false
        }

        this.handleShow = this.handleShow.bind(this);
    }

    handleShow() {
        this.setState({ show: true });
    }

    render() {
        return (
            <div>
                <Button bsStyle="primary" onClick={this.handleShow}>Add Contact</Button>
                <NewContactModal show={this.state.show}/>
            </div>
        );
    }
}
\$\endgroup\$
  • \$\begingroup\$ Please note that "somewhat like this" makes this a marginal Code Review question, since the help center rules require you to post real code. If you choose to simplify the code before posting, then you also bear the risk that answers might be based on incorrect assumptions about your code. \$\endgroup\$ – 200_success Aug 22 '18 at 20:00
  • \$\begingroup\$ Can you clarify what you mean by the // This doesn't work comment in handleShow()? Does this code work or not? \$\endgroup\$ – 200_success Aug 22 '18 at 20:00
  • \$\begingroup\$ @200_success by somewhat, I meant that I simply removed surounding code which does not affect the example. The code is exact and works perfectly as I stated in the question. Regarding the comment, I removed it. It was some legacy comment which no longer made sense. \$\endgroup\$ – Javier García Manzano Aug 22 '18 at 21:16
  • \$\begingroup\$ OK, excerpts are fine. I've reworded it to clarify that. \$\endgroup\$ – 200_success Aug 22 '18 at 21:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.