Summary

Tweak the heck out of the repository search box.

Review Request #4700 — Created Oct. 8, 2013 and submitted Oct. 8, 2013, 11:27 p.m.

Information

Owner

david

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Tweak the heck out of the repository search box.

The search box had some drawing problems, especially when scrollbars were
visible. I hadn't noticed this during the initial development because OSX.

This change tweaks the heck out of the javascript and CSS to try to make things
draw more reliably. The "Repositories" header is now hidden when the search box
is visible, to make it less likely that things will overlap. I've also tried to
tweak the positioning code to make it work better when scrollbars are present.

This is still not 100% perfect, but I'm about ready to hunt down the box model
spec authors and stab them in the face, so I'm going to leave it here for now.

Testing Done

Played around with this in Chrome, Safari, and Firefox, using a handful of zoom
levels. Turned on scrollbars always and saw that things were improved (though
not perfect).

Issues

Description	From	Last Updated
This should be in parens. While not 100% required right now, it'd be needed to pass lesscss strict mode, so …	chipx86	Oct. 8, 2013, 8:33 p.m.
What's 40? Can we math it instead?	chipx86	Oct. 8, 2013, 8:54 p.m.
What's 8? Undocumented magic numbers are just going to cause problems.	chipx86	Oct. 8, 2013, 8:54 p.m.

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
Ignored Files:
reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
reviewboard/static/rb/css/newReviewRequest.less
reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
Ignored Files:
reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
reviewboard/static/rb/css/newReviewRequest.less
reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js

Can you post a screenshot?

david

Oct. 8, 2013, 8:15 p.m.

There's no real difference between what you see at /r/new right now, unless you have scrollbars visible.

reviewboard/static/rb/css/newReviewRequest.less (Diff revision 1)

The issue has been resolved. Show all issues

This should be in parens. While not 100% required right now, it'd be needed to pass lesscss strict mode, so we should do it.

reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
What's 40? Can we math it instead?
```
1. david
  
  Oct. 8, 2013, 8:15 p.m.
  It's the size of the icon+padding. Computing it here wasn't working properly because browsers are terrible.

reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js (Diff revision 1)

The issue has been resolved. Show all issues

What's 8?

Undocumented magic numbers are just going to cause problems.

david

Oct. 8, 2013, 8:15 p.m.

8 is what I needed to put in to make it look right. I'm not sure why outerWidth() isn't giving me the right thing for the entry.

Diff:

Revision 2 (+24 -14)

Show changes

	reviewboard/static/rb/css/newReviewRequest.less
	reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
	reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (22f5b11).