2
\$\begingroup\$

I am trying to build a multi page form widget with React for a website inspired by this. Since quite some time has passed since I last touched JavaScript, I'm directly adapting to ES6/7 and tried to port the concept of Tommy Marshall (link above).

I'm on a good way so far, thinking about using Redux Form for the actual fields and validation etc. I'm reading a lot about HOC (High Order Components) and that Mixins are bad and whatnot. So I'm totally fine with not using Mixins cause apparently I didn't even learn them yet. But I do not quite understand still what's HOC all about even after reading quite some blogs and documentations.


Even so my biggest concern for now is that in my main component, that has all the different form views as children, there's a lot going on in the render() function and I think I read somewhere (React docs?) that it's obviously a good idea to keep render() clean and simple.

Everytime the state of my component changes, there's variables set, functions called and after all, the content rendered, one of which even spreads through the views and only changes css depending on state (maybe not too bad?)

import React, { Component, PropTypes } from 'react';
import AskForDate from './partials/date';
import AskForPeople from './partials/people';
import AskForPlan from './partials/plan';
import AskForMail from './partials/mail';
import Confirmation from './partials/confirmation';
import Success from './partials/success';

// ES6 React Component:
export default class SurveyForm extends Component {
  static propTypes = {
    initialCount : PropTypes.number
  }
  static defaultProps = {
    initialCount : 1
  }
  state = {
    count : this.props.initialCount
  }
  constructor(props) {
    super(props)
  }
  advanceForm = () => this.setState( {count: this.state.count + 1} )
  retreatForm = () => this.setState( {count: this.state.count - 1} )
  render() {

    // Define variables to get 'this' to work:
    const advanceForm = this.advanceForm
    const retreatForm = this.retreatForm
    const appState    = this.state.count
    const style       = { width : (appState / 5 * 100) + '%' }

    // Make an object with the different form views:
    function showStep(e) {
      const steps = {
        1: function() { 
          return <AskForDate   advanceForm = {advanceForm} />
        },
        2: function() {
          return <AskForPeople retreatForm = {retreatForm}
                               advanceForm = {advanceForm} />
        },
        3: function() {
          return <AskForPlan   retreatForm = {retreatForm}
                               advanceForm = {advanceForm} />
        },
        4: function() {
          return <AskForMail   retreatForm = {retreatForm}
                               advanceForm = {advanceForm} />
        },
        5: function() {
          return <Confirmation retreatForm = {retreatForm}
                               advanceForm = {advanceForm} />
        },
        6: function() {
          return <Success />
        }
      }
      return steps[e]()
    }

    // Hide the progress bar at end and close form after timeout:
    // This especially is something I would like to move somewhere else since it get's called with every render, maybe into the <Success /> component, would that be possible?
    function hack() {
      if (appState == 6) {
        document.querySelector('.prog-num').classList.add('hidden')
        setTimeout(function() {
          document.getElementById('form').classList.remove('shown')
        }, 1000)
      }
    } hack()

    // Begin render:
    return (
      <div id='heightSetter'>
        <span className='prog-num'>{appState}/5</span>
        <div className='prog-bar'style={style}></div>
        {showStep(appState)}
      </div>
    )
  }
};

As I said, I don't feel well looking at all those declarations made in render(). But without using variables to pass down the props the component didn't work for me. The hack() function is something that maybe should go inside the child component <Success />? And I didn't want to use a switch case because of this read (which wouldn't have been shorter anyways) and have no idea on how I could provide another solution to only serve a desired child component into render() depending on the state of the parent.

If you see a way to optimize this in one or another way (I'm totally open to ES7) I would highly appreciate your opinion.

\$\endgroup\$
1
\$\begingroup\$

Possible improvement and simplification could be done the following way. It should be mentioned that I used classnames package for managing classes.

import React, { Component, PropTypes } from 'react';
import classnames from 'classnames';
import AskForDate from './partials/date';
import AskForPeople from './partials/people';
import AskForPlan from './partials/plan';
import AskForMail from './partials/mail';
import Confirmation from './partials/confirmation';
import Success from './partials/success';

// ES6 React Component:
export default class SurveyForm extends Component {

  // ---
  // Props
  // ---
  static propTypes = {
    initialStep : PropTypes.number
  }
  static defaultProps = {
    initialStep : 1
  }

  // ---
  // Initialization
  // ---
  constructor(props) {
    super(props)

    // State should be in constructor
    this.state = {
      step : props.initialStep,
      isFormHidden: props.initialStep === 6
    }
  }

  // ---
  // Handlers
  // ---
  advanceForm = () => {
    if (this.state.step === 5)
      setTimeout(() => this.setState({isFormHidden: true}), 1000)
    this.setState({step: this.state.step + 1})
  };

  retreatForm = () => { 
    this.setState( {step: this.state.step - 1} )
  };

  // ---
  // Render method
  // ---
  render() {

    // Variables setup
    const advanceForm = this.advanceForm
    const retreatForm = this.retreatForm
    const step        = this.state.step
    const style       = { width : (step / 5 * 100) + '%' }
    const progNumClasses    = classnames({'prog-num': true, 'hidden': step === 6})
    const containerClassses = classnames({'hidden': this.state.isFormHidden})

    // Render
    return (
      <div id='heightSetter' className={containerClassses}>
        <span className={progNumClasses}>{step}/5</span>
        <div className='prog-bar'style={style}></div>
        {
          [
            <AskForDate advanceForm={advanceForm}/>,
            <AskForPeople retreatForm={retreatForm} advanceForm={advanceForm}/>,
            <AskForPlan retreatForm={retreatForm} advanceForm={advanceForm}/>,
            <AskForMail retreatForm={retreatForm} advanceForm={advanceForm}/>,
            <Confirmation retreatForm={retreatForm} advanceForm={advanceForm}/>,
            <Success/>
          ][step-1]
        }
      </div>
    )
  }
};
\$\endgroup\$
  • \$\begingroup\$ Thank you so much for reviewing this! I'm in a hurry right now, gonna check this good when I get a little free time! \$\endgroup\$ – Marian Feb 4 '16 at 18:31
  • \$\begingroup\$ Implemented this and works like a charm, also added some more arrow functions in the same way to save and check data values. Thanks a ton! \$\endgroup\$ – Marian Feb 29 '16 at 20:04

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.