• 
      

    Convert the project ideas views to react.

    Review Request #9548 — Created Jan. 28, 2018 and submitted

    Information

    student-sonar
    master
    8cbcead...

    Reviewers

    This is the first step in rewriting sonar's frontend to use React
    instead of Backbone. To start, I chose a relatively isolated set of
    views (the project descriptions).

    The old TaskView has now been converted to Project, which is a pure
    react component. This is used by two backbone views, the student project
    list and the individual project view. Both of these are still backbone
    views, but the innards are predominantly react-based now.

    Loaded the affected pages and tested the various features thoroughly.
    The only issue is some weirdness with the active item highlighting when
    clicking links in the project list nav, but that seems like it might be
    a regression inside the bootstrap scrollspy implementation.

    Description From Last Updated

    Instead of this, why not "extends": [ "eslint:recommended", "plugin:react/recommended", ] We're already using the plugin, why not all recommended settings?

    brenniebrennie

    I know its technically the same, but IMO, the following is more clear { tags || projects ? ( <div> …

    brenniebrennie

    Component refs are the recommended way of doing this: render() { return ( <div className={this.props.className} ref={el => this._$el = $(el)}> …

    brenniebrennie

    Col: 17 Expected an identifier and instead saw '<'.

    reviewbotreviewbot

    Col: 18 Expected ')' and instead saw 'div'.

    reviewbotreviewbot

    Col: 21 Missing semicolon.

    reviewbotreviewbot

    Col: 32 Missing semicolon.

    reviewbotreviewbot

    Col: 21 Expected an identifier and instead saw '<'.

    reviewbotreviewbot

    Col: 21 Unrecoverable syntax error. (84% scanned).

    reviewbotreviewbot

    You can add .jsx to the list of webpack extensions so that you can import Project from './project'. See the …

    brenniebrennie

    Your NavSection isn't a <section> :p

    brenniebrennie
    Checks run (1 failed, 1 succeeded)
    flake8 passed.
    JSHint failed.

    JSHint

    brennie
    1. 
        
    2. .eslintrc.json (Diff revision 1)
       
       
      Show all issues

      Instead of this, why not

      "extends": [
          "eslint:recommended",
          "plugin:react/recommended",
      ]
      

      We're already using the plugin, why not all recommended settings?

    3. lib/frontend/project.jsx (Diff revision 1)
       
       
       
       
       
       
       
      Show all issues

      I know its technically the same, but IMO, the following is more clear

      { tags || projects
        ? (
          <div> ... </div>
        )
        : null}
      
    4. lib/frontend/scrollspy.jsx (Diff revision 1)
       
       
      Show all issues

      Component refs are the recommended way of doing this:

      render() {
        return (
          <div className={this.props.className} ref={el => this._$el = $(el)}>
            {this.props.children}
          </div>
        );
      }
      
    5. Show all issues

      You can add .jsx to the list of webpack extensions so that you can import Project from './project'.

      See the webpack docs.

    6. Show all issues

      Your NavSection isn't a <section> :p

    7. 
        
    david
    brennie
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (9f6ac96)