914960 - Tweak TBPL's whitespace

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Stupid Windows x64 Debug builds...

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: widen the OS column

I used ems so that it'll scale better should someone use non-default fonts. Since this column width is directly related to the text displayed in it, it seems logical to me that the width would scale with it.

Otherwise, 160px also works.

Comment 2

[Ed Morley [:emorley]]

Comment on attachment 802770 [details] [diff] [review]
widen the OS column

13.5em is still too narrow for me, as is 160px. I have to crank it up to 166 pixels, which is another 22 above present, and a whopping 51 pixels wider than the column width used to be a year ago. I think we've already increased it more than we should have done - can we switch back to "Win" instead perhaps?

Comment 3

[Ed Morley [:emorley]]

(Or the other option is to re-jigg the whitespace in the changeset list etc) :-)

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ed Morley [:edmorley UTC+1] from comment #3)
> (Or the other option is to re-jigg the whitespace in the changeset list etc)
> :-)

Funny, I forgot you made this comment and had the same thought recently :D

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Attachment: various whitespace tweaks

See what you think of this. As best I can tell, it doesn't create any functional regressions from the status quo with respect to how much you can see on a line (in fact, I think a bit more fits on a line than before). But obviously that's only on one screen :)

There's probably more that could be done, but this at least accomplishes the goal of not cutting off any more text than before while allowing for the OS column to be properly widened to avoid cutting off any build names.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 817613 [details] [diff] [review]
various whitespace tweaks

Review of attachment 817613 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for looking at this :-)

I found it pretty hard to tell what you had intentionally adjusted vs had just switched from px to ems. Ended up just having to have two TBPL windows side by side.

Looks ok overall, but a few things need tweaking slightly before we land this, notably:
* When scrolling the results list, the results now appear partly under #topbar, a midway point between what it was before and now would probably work well.
* The changeset list comes a little close to the platform list IMO.

How about tweaking the font size slightly for the changeset list (or even just the hash?), so gain us some extra space? :-)

Comment 7

[Ed Morley [:emorley]]

Attachment: v1 screenshot

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: address review comments

This addresses the issues pointed out in the screenshot AFAICT. I played with the font sizing a bit but found that even dropping it from 0.95em to 0.9em made it a LOT smaller, so I bagged it for now.

I probably won't be around to land this by the time you take a look at it, so please go ahead and push if you're satisfied.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Also, if you end up pushing this, please note that the commit message is from when this bug was first filed and doesn't reflect the nature of the patch anymore.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 819275 [details] [diff] [review]
address review comments

Always a smart idea posting patches late at night right before heading off on vacation...

/me wanders back off

Comment 11

[Ed Morley [:emorley]]

Comment on attachment 819275 [details] [diff] [review]
address review comments

Tested locally side by side again and looks good - thank you :-)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/webtools/tbpl/rev/bde0726f862a

Comment 13

[Ed Morley [:emorley]]

In production :-)