Convert the view and model for the diff viewer page to ES6.

Review Request #9152 - Created Aug. 25, 2017 and submitted

Christian Hammond
Review Board
release-3.0.x
0acc7cb...
reviewboard

This updates the diff viewer page's view and model to ES6 and fixes up
the docs for our modern standards. The logic for these classes remains
the same, but the code has been streamlined by the usage of template
literals, consts/lets, and fat arrow functions.

Upcoming work will build upon this to simplify the view further.

Unit tests pass.

Tested all functionality of the diff viewer (revision selection,
interdiffs, pagination, anchor navigation, commenting) to ensure
nothing broke.

  • 0
  • 0
  • 9
  • 1
  • 10
Description From Last Updated
David Trowbridge
  1. 
      
  2. Instead of a function-scoped variable created at the top, how about just setting const parts here?

  3. Convert this to a .forEach loop?

  4. This can be const [base, tip] = revisions;

    1. Going to keep this as-is. const [base, tip] = revisions results in a lot more code in each file that uses it (Babel injects a decent-sized file-level function that checks for different sets of conditions, calls that to normalize the array, and then does exactly what this original code does, but with an additional variable in the scope). This isn't done in any common place, unfortunately, so it ends up ballooning the size of the bundles.

      I love that the syntax exists in ES6, but Babel's output isn't at all worth it right now.

  5. forEach?

    This can also be a lot more terse:

    this.diffReviewableViews.forEach(view => view.remove());
    
  6. 
      
Christian Hammond
David Trowbridge
  1. Ship It!
  2. 
      
Christian Hammond
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (0e29a8d)
Loading...