Improve and optimize the autosize text area widget.

Review Request #209 — Created Jan. 24, 2008 and submitted

Information

Review Board SVN (deprecated)
trunk

Reviewers

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
  1. 
      
  2. /trunk/reviewboard/htdocs/scripts/comments.js (Diff revision 3)
     
     
     
     
     
     
    Does this have too many });s?
    1. No idea how that happened. I must have done that just before posting, since the code was working.
  3. What's the 100 magic value for?
    1. 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.
  4. Can you add a big comment explaining the proxy element?
  5. /trunk/reviewboard/htdocs/scripts/rb/widgets.js (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
    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. Ah, couldn't remember if JavaScript had a max function. Thanks.
  6. 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.
david
  1. Looks good now.
  2. /trunk/reviewboard/htdocs/scripts/rb/widgets.js (Diff revisions 3 - 4)
     
     
     
     
    I'd rather put the +'s on the next lines, just to separate the two parameters visually a little more.
  3. 
      
Loading...