I am an experience backend programmer wanting to bring my frontend skills up to par. (My js experience up to this point is mashing things until they work rather than structured frontend applications).

To that end I'm working on a React project - I'd love to get some feedback on the code below to make sure I'm heading in the right direction.

What does the code do?

The code is for some editable text on a webpage. There's an edit link that you can click to turn the text into a text box for editing. Enter submits, clicking off shows the text (rather than the textbox) again, clicking cancel shows the text and removes your edits.

Specific Qs

  • Is this a reasonable solution to this problem? Any obvious gains in simplicity or elegance?
  • Any formatting best practices I'm missing?
  • The render function is disgusting - any tips to format this better, or does JSX just force this kind of thing to happen?
  • Any other tips?

import React, {PropTypes} from 'react';
import Spinner from './utils';

export default class EditableField extends React.Component {
  propTypes: {
    className: React.PropTypes.string,
    text: React.PropTypes.string,
    fieldType: React.PropTypes.string,
    submitCallback: React.PropTypes.func,

  constructor(props) {

    this.state = {
      editing: false,
      submitting: false,
      draftText: this.props.text,
      text: this.props.text,
      error: false,

    this.handleCancel = this.handleCancel.bind(this);
    this.handleClickOutside = this.handleClickOutside.bind(this);
    this.handleKeyDown = this.handleKeyDown.bind(this);
    this.toggleEditing = this.toggleEditing.bind(this);
    this.handleChange = this.handleChange.bind(this);
    this.triggerSubmit = this.triggerSubmit.bind(this);
    this.success = this.success.bind(this);
    this.failure = this.failure.bind(this);

  success() {
    this.setState({submitting: false});

  failure() {
      submitting: false,
      error: true

  handleClickOutside(e) {
    if (this.wrapperRef && !this.wrapperRef.contains(e.target)) {

  handleCancel(e) {
    this.setState({draftText: this.state.text});

  toggleEditing() {
    // so we can close on outside click
    if (this.state.editing) {
      document.removeEventListener('mousedown', this.handleClickOutside);
    } else {
      document.addEventListener('mousedown', this.handleClickOutside);

    this.setState({editing: !this.state.editing});

  handleChange(e) {
    this.setState({draftText: e.target.value});

  // actually submit the request etc.
  triggerSubmit() {
      text: this.state.draftText,
      submitting: true,
    // submit
    this.props.submitCallback(this.state.draftText, this.success, this.failure);

  // check for enter or cancel
  handleKeyDown(e) {
    const enter_key = 13;
    const escape_key = 27;

    if (e.keyCode == enter_key) {
    } else if (e.keyCode == escape_key) {

  render() {
    let controlLinks;
    let editing;
    let textOrInput;
    let spinner;
    let error;

    if (this.state.error) {
      error = <span className="error">error!</span>

    if (this.state.submitting) {
      spinner = <Spinner />

    if (this.state.editing) {
      textOrInput = <input

      controlLinks = (
          <a className="edit-link blue-link" onClick={this.handleCancel} href="#">CANCEL</a>

      editing = 'editing';
    } else {
      textOrInput = <span className="text">{this.state.text}</span>
      controlLinks = <div><a className="edit-link blue-link" onClick={this.toggleEditing} href="#">EDIT</a></div>
      editing = '';

    return (

        className={`${this.props.className} ${editing} editable-field`}
        ref={(node) => { this.wrapperRef = node; }}
  • \$\begingroup\$ Personally I would go with arrow function expression instead of binding in constructor. success = () => { ... } does not bind its own this, and now you can remove those ugly looking bindings from constructor ;) This is a good reference: medium.freecodecamp.org/… \$\endgroup\$ – Liren Yeo Oct 3 '17 at 8:48

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.