Render the popup for the list of integrations in JavaScript.

 






 

 


  Review Request #10440
  —
  Created March 6, 2019 and submitted 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Djblets
 









 
 

  release-1.0.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  djblets
 









 
 

  
 






 








 





  


 
 

  
The new integrations popup was previously hard-coded into the page,

hidden by default and then wrapped in the JavaScript view, much like the

configuration list until a recent change. That information has since

been added to the list of data passed to the view, meaning we're sending

it over the wire twice. This isn't efficient and isn't worth doing,

particularly if the popup's never going to be used.


This change updates the popup rendering to happen dynamically in the

JavaScript view the first time it's clicked. This is done in a way that

minimizes render time while also allowing subclasses to override the

display.


The Add Integration button has also received an icon. This uses the same

look (a white plus symbol on a green circle) that the Add button for

condition fields use, which we'll be able to use in more places

throughout our config form UIs.

 








  


 
 

  
Tested that the popup displayed properly and that integrations could be

added.


Unit tests pass.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Render the popup for the list of integrations in JavaScript.
      
The new integrations popup was previously hard-coded into the page,
hidden by default and then wrapped in the JavaScript view, much like the
configuration list until a recent change. That information has since
been added to the list of data passed to the view, meaning we're sending
it over the wire twice. This isn't efficient and isn't worth doing,
particularly if the popup's never going to be used.

This change updates the popup rendering to happen dynamically in the
JavaScript view the first time it's clicked. This is done in a way that
minimizes render time while also allowing subclasses to override the
display.

The Add Integration button has also received an icon. This uses the same
look (a white plus symbol on a green circle) that the Add button for
condition fields use, which we'll be able to use in more places
throughout our config form UIs.

     


    		
    764da093df5f8ea8f6551d9c83072d82f1f4f0e4

   



  
 




 













 

  
  


   

    

     

     

     

    

   


   

  

 







 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
What's the idea behind waiting to run _.template?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Col: 36 Missing semicolon.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
E501 line too long (86 > 79 characters)

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (2 failed)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 failed.
 













 
flake8

 


 










 
JSHint

 


 







  

 





 














 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       djblets/static/djblets/js/integrations/views/addIntegrationPopupView.es6.js
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    What's the idea behind waiting to run _.template?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      An attempt toward reducing startup time of the JS side of the codebase. _.template isn't exactly free, and we run it for every single view that's in the JS bundle, even if the view is never used. If we're smarter about when we compile templates, we can reduce both memory usage and startup time in the browser.
      

      In the case here, we can guarantee that only one of the two templates is ever going to be used (we either use the "there's no integrations" template or we use the integration item template multiple times), and that's only if the popup is ever clicked (which won't be the case when just visiting the Team Admin page in RBCommons).
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Makes sense.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-1.0.x (3a48e6a)