Last Comment Bug 675481 - Space reserved for expand button should be determined at run-time
: Space reserved for expand button should be determined at run-time
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 8
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on: 673825
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-30 23:23 PDT by Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
Modified: 2016-04-12 14:00 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (2.55 KB, patch)
2011-08-09 01:58 PDT, Raymond Lee [:raymondlee]
ttaubert: review+
Details | Diff | Splinter Review
Patch for checkin (2.64 KB, patch)
2011-08-14 20:26 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Splinter Review

Description Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-07-30 23:23:26 PDT
Follow-up from bug 673825 comment #5.

We currently reserve 33px of space for the expand button at the bottom of stacked groups. This value (the height of the expand button plus some margin) should be determined at run-time to allow theming.
Comment 1 Raymond Lee [:raymondlee] 2011-08-09 01:58:43 PDT
Created attachment 551706 [details] [diff] [review]
v1
Comment 2 Raymond Lee [:raymondlee] 2011-08-09 06:06:18 PDT
Passed Try.  http://tbpl.mozilla.org/?tree=Try&rev=fca2473d10d4
Comment 3 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-14 07:08:28 PDT
Comment on attachment 551706 [details] [diff] [review]
v1

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

Looks great! Could you please add a little padding to the expand button's height? It's now 24px and we assumed 33px before. So maybe a padding of 9-10px would be good. And please don't forget to add a little comment that tells why we're adding these 9px if that isn't obvious :)
Comment 4 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-14 07:09:39 PDT
Comment on attachment 551706 [details] [diff] [review]
v1

I forgot to say r=me with the little padding fix but I guess you figured that :)
Comment 5 Raymond Lee [:raymondlee] 2011-08-14 20:26:56 PDT
Created attachment 553093 [details] [diff] [review]
Patch for checkin

(In reply to Tim Taubert [:ttaubert] from comment #3)
> Comment on attachment 551706 [details] [diff] [review]
>
> Looks great! Could you please add a little padding to the expand button's
> height? It's now 24px and we assumed 33px before. So maybe a padding of
> 9-10px would be good. And please don't forget to add a little comment that
> tells why we're adding these 9px if that isn't obvious :)
Done
Comment 6 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-08-15 07:10:14 PDT
http://hg.mozilla.org/integration/fx-team/rev/6137de4b8036
Comment 7 Rob Campbell [:rc] (:robcee) 2011-08-16 08:39:55 PDT
http://hg.mozilla.org/mozilla-central/rev/6137de4b8036

Note You need to log in before you can comment on or make changes to this bug.