Tweak the heck out of the repository search box.

Review Request #4700 — Created Oct. 8, 2013 and submitted

Information

Review Board
master

Reviewers

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.

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

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 …

chipx86chipx86

What's 40? Can we math it instead?

chipx86chipx86

What's 8? Undocumented magic numbers are just going to cause problems.

chipx86chipx86
reviewbot
  1. 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

  2. 
      
reviewbot
  1. 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

  2. 
      
chipx86
  1. Can you post a screenshot?

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

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

  3. Show all issues

    What's 40? Can we math it instead?

    1. It's the size of the icon+padding. Computing it here wasn't working properly because browsers are terrible.

  4. Show all issues

    What's 8?

    Undocumented magic numbers are just going to cause problems.

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

  5. 
      
david
reviewbot
  1. 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

  2. 
      
reviewbot
  1. 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

  2. 
      
chipx86
  1. Ship It!

  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (22f5b11).

Loading...