Review Board

beta

Improve and optimize the autosize text area widget.

Updated 11 months, 2 weeks ago

Christian Hammond Reviewers
trunk reviewboard
None Review Board SVN 
The autosize text area widget we use in the review request page and comments dialog got really slow with large amounts of text. We used a pretty crappy but commonly used method for this widget. Now we use a sane method where we keep a proxy element that is the same shape and style of the text area but wraps and has a dynamic height. We use this to calculate the correct height for our text area. This seems to improve the speed considerably and simplifies the code.
Tested on Firefox and IE6. Works correctly on both.
David Trowbridge
Posted 11 months, 2 weeks ago (January 25th, 2008, 11:09 p.m.)

   

  

   
    
    
   




   



    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Does this have too many });s?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 25th, 2008, 11:30 p.m.)
      
     
    
     
    No idea how that happened. I must have done that just before posting, since the code was working.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
What's the 100 magic value for?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 25th, 2008, 11:38 p.m.)
      
     
    
     
    It's the extra height we expand to. Like how you get the height of the content + some value. That's the value. I'll define it somewhere.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Can you add a big comment explaining the proxy element?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 25th, 2008, 11:32 p.m.)
      
     
    
     
    Will do.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
I'd prefer writing this as:
var newHeight = Math.max(this.minHeight,
                         this.probyEl.getHeight(true)
                         + this.el.getBorderWidth('tb')
                         + this.el.getPadding('tb'));

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 25th, 2008, 11:32 p.m.)
      
     
    
     
    Ah, couldn't remember if JavaScript had a max function. Thanks.
    
    
    
   
    2. 


   



    

    

   


  
   
Mostly good.  I'd really like to see documentation added every time we touch the JavaScript code, since it's a lot harder to suss than the python.

   

  

   
    
    
   



  
 




  

 


















 

  




 

 

  
Ship it!

  
David Trowbridge

  
Posted 11 months, 2 weeks ago (January 25th, 2008, 11:46 p.m.)

 

 

   
Looks good now.

   

  

   
    
    
   




   



    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
I'd rather put the +'s on the next lines, just to separate the two parameters visually a little more.