Make collapse diff buttons keyboard accessible

 






 

 


  Review Request #10925
  —
  Created Feb. 27, 2020 and updated 

 


















 

  

   Information

  





 
 

  hxqlin
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 









 
 

  10951
 









 
 

  8679dc4...
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Previously, the collapse diff button on reviewable diffs and diffs in 

comment fragments could not be used with the keyboard as shown in the 

attached video collapse-btn-old.mov, unlike the expand diff button. This

review request makes the collapse diff button keyboard accessible. The new

behavior is documented in the attached video collapse-btn-new.mov. There

is also a side-by-side screenshot comparison of the old and new button.


The collapse button used to be a <div>, which is not focusable by 

default. The fix is to change the <div> to an <a>, which made the button 

focusable but did not invoke the collapse diff action. (Using an <a> is 

consistent with the expand diff button.) By adding the href=# attribute, 

both clicking on the element and using the enter key while the element is 

focused invokes the click handler to collapse the diff.

 








  


 
 

  
Manual testing on reviewable diffs and comment fragment diffs:

- used tab & shift+tab to check the collapse diff button can be tabbed to

- used enter key to invoke the collapse diff action and re-expand diff 

using only the keyboard

 







 









 





  








 

  
  


   

    

     

     

     

    

   


   

    

     

     

     

    

   


   

    

     

     

     

    

   


   

  

 







 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
A few notes on the change description: The text is wrapping far too early. Can you wrap at ~75 characters? …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Can you show a side-by-side screenshot of an old collapse button and a new one? There's some fundamental changes to …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
The attributes aren't aligned correctly.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
The attributes aren't aligned correctly. Since you're updating this line anyway to wrap, let's move the data-... attributes to the …

      		
      
       chipx86chipx86
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    A few notes on the change description:
    

      

    • The text is wrapping far too early. Can you wrap at ~75 characters?
      • 

    • HTML tags should become string literals (backticks)
      • 

    • The Asana link isn't going to be useful to outside contributors, and this change description will be used when landing. You can just remove that from here.
      • 

    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 





  3. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can you show a side-by-side screenshot of an old collapse button and a new one? There's some fundamental changes to the display type that's been brought in through this, and I want to make sure it's going to be consistent.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 















  4. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    The attributes aren't aligned correctly.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    The attributes aren't aligned correctly.
    

    Since you're updating this line anyway to wrap, let's move the data-... attributes to the second line, keep things under our line length limits.
    

    And fix our variable references to be {{comment.id}} instead of {{ comment.id }} (the latter must have snuck in).
    

    These comments apply to both modified lines in the file.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  6. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 




  

 
















 



 

  

   




hxqlin




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
Fix issues from review





 



  

   
Description:

   
    
 ~   
Previously, the collapse diff button on reviewable diffs and 
 ~   diffs in comment fragments could not be used with the keyboard 
 ~   as shown in the attached video collapse-btn-old.mov, unlike
 ~   the expand diff button. This review request makes the collapse 
 ~   diff button keyboard accessible. The new behavior is documented 
 ~   in the attached video collapse-btn-new.mov.
   ~ 
Previously, the collapse diff button on reviewable diffs and diffs in 
   ~ comment fragments could not be used with the keyboard as shown in the 
   ~ attached video collapse-btn-old.mov, unlike the expand diff button. This
   ~ review request makes the collapse diff button keyboard accessible. The new
   ~ behavior is documented in the attached video collapse-btn-new.mov. There
   ~ is also a side-by-side screenshot comparison of the old and new button.
     

 ~   
The collapse button used to be a <div>, which is not focusable
 ~   by default. The fix is to change the <div> to an <a>, which made
 ~   the button focusable but did not invoke the collapse diff action.
 ~   (Using an <a> is consistent with the expand diff button.) By 
 ~   adding the href=# attribute, both clicking on the element and 
 ~   using the enter key while the element is focused invokes the 
   ~ 
The collapse button used to be a <div>, which is not focusable by 
   ~ default. The fix is to change the <div> to an <a>, which made the button 
   ~ focusable but did not invoke the collapse diff action. (Using an <a> is 
   ~ consistent with the expand diff button.) By adding the href=# attribute, 
   ~ both clicking on the element and using the enter key while the element is 
   ~ focused invokes the click handler to collapse the diff.
 -   click handler to collapse the diff. 
 -   

 -   
Asana link: https://app.asana.com/0/40883733502513/1157098340882771/f

   		
  




  

   
Testing Done:

   
    
     
Manual testing on reviewable diffs and comment fragment diffs:
 ~   - used tab & shift+tab to check the collapse diff button can be 
 ~   tabbed to
 ~   - used enter key to invoke the collapse diff action and re-expand 
   ~ - used tab & shift+tab to check the collapse diff button can be tabbed to
   ~ - used enter key to invoke the collapse diff action and re-expand diff 
   ~ using only the keyboard
 -   diff using only the keyboard

   		
  




  

   
Commit:

   
    
-1c430fa229dd65539005593b50cd3a8ae087b4b4
+8679dc402cabef2f347d59d2fc2afc49360e024c

   		
  




  

   
Diff:

   
    
 Revision 2 (+9 -11)
Show changes
 
    reviewboard/static/rb/css/pages/diffviewer.less 
    reviewboard/templates/diffviewer/diff_file_fragment.html 
    reviewboard/templates/reviews/diff_comment_fragment.html 
 

   		
  






  

   
Added Files:

   
    






   		
  



 









 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 









     

    

   

  

  

   
   Loading...