• 
      

    Convert the sidebar to react.

    Review Request #9557 — Created Jan. 31, 2018 and submitted

    Information

    student-sonar
    master
    f7351a8...

    Reviewers

    This change converts the SidebarView to use react internally. This is
    mostly a very straightforward change.

    The only unusual part of this change is the shim between react-router
    and Backbone.Router. For now, we're using react-router's <NavLink>
    components, but intercepting click events and redirecting them back to
    the old window.application.go() function. This triggers the existing
    routes properly. We also have a handler for when the route changes in
    Backbone to mirror that back into react-router's history object. Once
    the other views have been converted (or at least wrapped), the
    <Router> component can be moved into the application view, and we can
    get rid of the special route handling and just rely on react for all of
    that.

    • Verified that the sidebar items loaded correctly for a variety of
      scenarios.
    • Checked that the correct item was highlighted based on the route.
    • Checked that item highlighting updated correctly even when routes were
      triggered from elsewhere in the UI (such as the "All Users" list).
    Description From Last Updated

    Since we're on React 16.2, you can use a fragment: {loggedin && ( <> <SidebarItem to="/calendar" label="Calendar" /> <SidebarItem to="/status" …

    brenniebrennie

    Here, too.

    brenniebrennie
    brennie
    1. Tiny suggestions, but lgtm.

    2. lib/frontend/sidebar-view.js (Diff revision 1)
       
       
       
      Show all issues

      Since we're on React 16.2, you can use a fragment:

      {loggedin && (
        <>
          <SidebarItem to="/calendar" label="Calendar" />
          <SidebarItem to="/status" label="My Status Reports" />
        </>
      )}
      

      The fragment will render just the contents without any wrapping element.

      1. This is more or less how the original underscore template did it, but I decided that I actually preferred guarding each item individually. I feel like it makes it easier to read, compared to conditionals that might be grouped just depending on what order the items happen to be in.

    3. lib/frontend/sidebar-view.js (Diff revision 1)
       
       
       
      Show all issues

      Here, too.

    4. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (b495f61)