Print Preview: Fixing pages textfield appearance when scrollbar is visible.

Print Preview: Fixing pages textfield appearance when scrollbar is visible.

BUG=85608
TEST=See bug description


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90110

[Evan Stade, 2011-06-22 20:06:22]

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.html (right):

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.html:40: <div
id="print-pages-div">
why are you adding a new div

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.html:43: <label
id="print-pages-label" for="print-pages">
don't add this if you're just using it for css, you can do #print-pages-div >
label

[dpapad, 2011-06-22 20:27:13]

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.html (right):

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.html:40: <div
id="print-pages-div">
On 2011/06/22 20:06:22, Evan Stade wrote:
> why are you adding a new div

I added this div because the parent of this div has #individual-pages-hint in it
and that hint is not displayed correcly otherwise (it appears on top of the
textbox instead of underneath it).

http://codereview.chromium.org/7215045/diff/2001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.html:43: <label
id="print-pages-label" for="print-pages">
On 2011/06/22 20:06:22, Evan Stade wrote:
> don't add this if you're just using it for css, you can do #print-pages-div >
> label

Done.

[Evan Stade, 2011-06-22 20:32:01]

lgtm

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:614: #individual-pages
{
minor nit: I would put these css rules in an order that matches their dom
hierarchy, i.e. print-pages-div, then the label, then individual-pages

[dpapad, 2011-06-22 20:36:29]

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:614: #individual-pages
{
On 2011/06/22 20:32:01, Evan Stade wrote:
> minor nit: I would put these css rules in an order that matches their dom
> hierarchy, i.e. print-pages-div, then the label, then individual-pages

Done. This sounds like a good practice, I will try to enforce this to the
remaining of this file in a future cleanup CL.

[Evan Stade, 2011-06-22 20:43:54]

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/7215045/diff/9001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:614: #individual-pages
{
On 2011/06/22 20:36:29, dpapad wrote:
> On 2011/06/22 20:32:01, Evan Stade wrote:
> > minor nit: I would put these css rules in an order that matches their dom
> > hierarchy, i.e. print-pages-div, then the label, then individual-pages
> 
> Done. This sounds like a good practice, I will try to enforce this to the
> remaining of this file in a future cleanup CL.

another thing that can make this file easier to read is to break it up by
section. You seem to have done that a little at the top but not for the last 600
or so lines.

[commit-bot: I haz the power, 2011-06-22 21:46:05]

Change committed as 90110