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

 






 

 


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

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-3.0.x
 









 
 

  
 









 
 

  
 









 
 

  0acc7cb...
 






 




 

  

   Reviewers

  





 
 

  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.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Args/Returns?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Trailing comma?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Args?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Instead of a function-scoped variable created at the top, how about just setting const parts here?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
"Return".

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Convert this to a .forEach loop?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
forEach?

      		
      
       daviddavid
      
      
       
      
     




     

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

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
forEach? This can also be a lot more terse: this.diffReviewableViews.forEach(view => view.remove());

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Trailing comma.

      		
      
       daviddavid
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Args/Returns?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Trailing comma?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Args?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Instead of a function-scoped variable created at the top, how about just setting const parts here?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    "Return".
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 






  7. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Convert this to a .forEach loop?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    8. 






  8. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    forEach?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    9. 






  9. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    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.
      
 
      

      2. 


 
    


    

 
    

    10. 






  10. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    forEach?
    

    This can also be a lot more terse:
    

    

    this.diffReviewableViews.forEach(view => view.remove());

    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    11. 






  11. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Trailing comma.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    12. 




  12. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    13. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to release-3.0.x (0e29a8d)





 


 










  

 









     

    

   

  

  

   
   Loading...