Update UI for the comments hint.

 






 

 


  Review Request #13672
  —
  Created March 27, 2024 and submitted 

 




















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  release-7.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This change ports the comments hint model and view to TypeScript, and

updates the UI to include the new commit range information. Along with

this, the implementation has been simplified a bit to attach event

handlers individually to each item in the list, rather than having a

generic event handler and setting data on the list items.

 








  


 
 

  
    

  • Ran js-tests.
    • 

  • Viewed a diff while having a review that had comments on lots of
    
  different revisions/interdiffs/commits. Saw that the comment hint
    
  correctly showed all of the revisions that had draft comments, and
    
  that clicking on all of them correctly loaded the associated revision.
    • 


 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Update UI for the comments hint.
      
This change ports the comments hint model and view to TypeScript, and
updates the UI to include the new commit range information. Along with
this, the implementation has been simplified a bit to attach event
handlers individually to each item in the list, rather than having a
generic event handler and setting data on the list items.

Testing Done:
- Ran js-tests.
- Viewed a diff while having a review that had comments on lots of
  different revisions/interdiffs/commits. Saw that the comment hint
  correctly showed all of the revisions that had draft comments, and
  that clicking on all of them correctly loaded the associated revision.

     


    		
    6ba2f4f635f172e1b3df75acd7e2eacb94407e4c

   



  
 




 













 

  
  


   

    

     

     

     

    

   


   

  

 







 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Is the babel-plugin-htm dependency update with all the package-lock.json and package.json changes supposed to be part of this change?

      		
      
       maubinmaubin
      
      
       
      
     




     

      
       
       
This one can be an import type.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Unless I missed runtime usage, this can be import type.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
We should avoid h1, as that's the main title foe the page. h2 should be the (in theory) the review …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Let's pull out this.model. We access it a lot.

      		
      
       chipx86chipx86
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 



 

  

   




david




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This one can be an import type.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Unless I missed runtime usage, this can be import type.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    We should avoid h1, as that's the main title foe the page. h2 should be the (in theory) the review request summary. This should probably be h3 or h4.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I agree, but all of our ancient styles make h3 have a crazy margin. I think I'd prefer to basically leave this box stuff as-is until we replace with a modern ink component.
      
 
      

      2. 


 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/reviews/views/diffCommentsHintView.ts
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Let's pull out this.model. We access it a lot.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  6. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 




  

 
















 



 

  

   




david




   

   

  

 



 















 





 

  

   




maubin




   

   







  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Is the babel-plugin-htm dependency update with all the package-lock.json and package.json changes supposed to be part of this change?
    
  
    
 
    

 
    
  



 
    

    3. 













  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




david




   

   

  

 



 















 





 

  

   




maubin




   

   







  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-7.x (e5a1f25)