Summary

Improve and optimize the autosize text area widget.

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

Information

Owner

chipx86

Repository

Review Board SVN (deprecated)

Branch

trunk

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

Tested on Firefox and IE6. Works correctly on both.

/trunk/reviewboard/htdocs/scripts/comments.js (Diff revision 3)

Does this have too many });s?

chipx86 Jan. 25, 2008, 3:30 p.m.

No idea how that happened. I must have done that just before posting, since the code was working.

/trunk/reviewboard/htdocs/scripts/rb/widgets.js (Diff revision 3)

What's the 100 magic value for?

chipx86 Jan. 25, 2008, 3: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.

/trunk/reviewboard/htdocs/scripts/rb/widgets.js (Diff revision 3)
```
Can you add a big comment explaining the proxy element?
```
1. chipx86 Jan. 25, 2008, 3:32 p.m.
  Will do.

/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'));

chipx86 Jan. 25, 2008, 3:32 p.m.

Ah, couldn't remember if JavaScript had a max function. Thanks.

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!

```
Looks good now.
```

/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.