Last Comment Bug 746725 - Give Filelink notification toggle and spinner a bit more bottom margin
: Give Filelink notification toggle and spinner a bit more bottom margin
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: Thunderbird 14.0
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on: 746307
Blocks: BigFiles
  Show dependency treegraph
 
Reported: 2012-04-18 13:52 PDT by Mike Conley (:mconley) - (Away until June 29th)
Modified: 2012-04-23 13:36 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Notice how the toggle / spinner crowded everything else a bit (9.62 KB, image/png)
2012-04-18 13:52 PDT, Mike Conley (:mconley) - (Away until June 29th)
no flags Details
Patch (677 bytes, patch)
2012-04-19 12:07 PDT, Richard Marti (:Paenglab)
mconley: review-
Details | Diff | Review
Patch v2 (1.07 KB, patch)
2012-04-19 13:09 PDT, Richard Marti (:Paenglab)
mconley: review+
mconley: ui‑review-
Details | Diff | Review
Side-by-side comparison on Windows 7 (99.23 KB, image/png)
2012-04-20 12:35 PDT, Mike Conley (:mconley) - (Away until June 29th)
no flags Details
Side-by-side on OSX (104.45 KB, image/png)
2012-04-20 12:53 PDT, Mike Conley (:mconley) - (Away until June 29th)
no flags Details
Patch v3 (1.07 KB, patch)
2012-04-20 14:31 PDT, Richard Marti (:Paenglab)
richard.marti: review+
mconley: ui‑review+
mozilla: approval‑comm‑aurora+
Details | Diff | Review
Screenshot showing there the link goes (11.29 KB, image/png)
2012-04-20 14:35 PDT, Richard Marti (:Paenglab)
no flags Details

Description Mike Conley (:mconley) - (Away until June 29th) 2012-04-18 13:52:16 PDT
Created attachment 616277 [details]
Notice how the toggle / spinner crowded everything else a bit

The toggle and spinner seem to crowd in a bit with both the menulist and the management pane for a selected provider.  We might want to give the toggle / spinner more bottom margin.

See attachment.
Comment 1 Richard Marti (:Paenglab) 2012-04-19 09:07:33 PDT
I'm not sure what you mean needs more bottom margin. Is it the richlistitem with the Dropbox-/Yousendit icon? If yes, I checked with DOMi and it looks correct aligned.

The paper flyer (is it that?) of the YSI icon isn't centered on the icon. So this could give the impression of a to low positioned icon.
Comment 2 Mike Conley (:mconley) - (Away until June 29th) 2012-04-19 10:23:17 PDT
Hey Richard,

Sorry - I wasn't clear.

I'm talking about both the richlist, and the large "Dropbox" title. Both seem to be a little bit close to the offer toggle and number spinner that are above them.

It doesn't make the UI unusable or unreadable by any means - it's mostly just an aesthetic thing.  It *feels* crowded.

Know what I mean?

-Mike
Comment 3 Richard Marti (:Paenglab) 2012-04-19 12:07:53 PDT
Created attachment 616686 [details] [diff] [review]
Patch

I gave a padding-bottom of 6px. On top the tabpanels has a padding of 8px. On bottom the richlistbox has a margin of 2px. With this the cloudFileToggleAndThreshold box should be correctly centered.
Comment 4 Mike Conley (:mconley) - (Away until June 29th) 2012-04-19 12:18:49 PDT
Comment on attachment 616686 [details] [diff] [review]
Patch

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

Hey Paenglab,

Thanks for tackling this. :) A few things:

1)  I think this is a problem on OSX too, so we'll likely need something for pinstripe.  gnomestripe might be OK.
2)  I'm getting scrollbars in the management pane with this patch.  I wonder if Andreas's work in bug 746307 might help here - I think it axes some of our hardcoded dimensions.

-Mike
Comment 5 Richard Marti (:Paenglab) 2012-04-19 13:09:31 PDT
Created attachment 616722 [details] [diff] [review]
Patch v2

> Review of attachment 616686 [details] [diff] [review]:
> 
> 1)  I think this is a problem on OSX too, so we'll likely need something for
> pinstripe.  gnomestripe might be OK.

I checked before pinstripe and gnomestripe. On OSX I thought it looked okay without change. This is also why I now only added 3px. Linux has a bottom padding of 15px which maybe is a little bit to much. With a bottom padding of 6px it would also be centered like on Windows. Should I do this?

> 2)  I'm getting scrollbars in the management pane with this patch.  I wonder
> if Andreas's work in bug 746307 might help here - I think it axes some of
> our hardcoded dimensions.

First I had Lightning enabled and saw no scrollbars. I tested with the rules of the patch of bug 746307 and the scrollbars disappeared also with Lightning disabled.

If this patch becomes r+ we could wait for bug 746307 to land this.
Comment 6 Mike Conley (:mconley) - (Away until June 29th) 2012-04-20 12:35:16 PDT
Created attachment 617066 [details]
Side-by-side comparison on Windows 7

Original is on the left, patches applied is on the right.
Comment 7 Mike Conley (:mconley) - (Away until June 29th) 2012-04-20 12:53:59 PDT
Created attachment 617071 [details]
Side-by-side on OSX

Same as above, except on OSX.
Comment 8 Mike Conley (:mconley) - (Away until June 29th) 2012-04-20 12:55:01 PDT
Comment on attachment 616722 [details] [diff] [review]
Patch v2

Blake - thoughts?
Comment 9 Mike Conley (:mconley) - (Away until June 29th) 2012-04-20 13:35:22 PDT
Comment on attachment 616722 [details] [diff] [review]
Patch v2

Hey Richard,

Code looks good - I think I want a little more margin on OSX, though.  Can we double it to 6px from 3px?

Note that when we do that, the link at the bottom to go to each provider's website will be too low, and will need to be adjusted accordingly (unless Andreas's patch takes care of that when he makes one for OSX...because it might).

-Mike
Comment 10 Richard Marti (:Paenglab) 2012-04-20 14:31:29 PDT
Created attachment 617098 [details] [diff] [review]
Patch v3

Only change: gave OSX a padding-bottom of 6px.

With Andreas's patch applied the link on bottom jumps beside the graphic.  So I don't change anything there.

Carrying over the r+
Comment 11 Richard Marti (:Paenglab) 2012-04-20 14:35:05 PDT
Created attachment 617099 [details]
Screenshot showing there the link goes

Screenshot on Win7 with the patch from bug 746307 applied. The link is now beside the graphic. I suppose the same happens on Mac but I can't set up a Filelink provider on my Mac VM to check this.
Comment 12 Mike Conley (:mconley) - (Away until June 29th) 2012-04-23 07:33:14 PDT
Thanks Richard.  Just going to wait for bug 746307 to stabilize before testing this.
Comment 13 Mike Conley (:mconley) - (Away until June 29th) 2012-04-23 13:23:39 PDT
Comment on attachment 617098 [details] [diff] [review]
Patch v3

With Andreas's patch for bug 746307 landed, I tried this out, and the link didn't seem to move around like in your screenshot.

It looked awesome.  So I'm ui-r+'ing this.

Thanks Richard,

-Mike
Comment 14 Mike Conley (:mconley) - (Away until June 29th) 2012-04-23 13:32:26 PDT
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/c074ae4d363e
Comment 15 Mike Conley (:mconley) - (Away until June 29th) 2012-04-23 13:36:00 PDT
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/07480ef159ca

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