746725 - Give Filelink notification toggle and spinner a bit more bottom margin

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Attachment: 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)]

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) (:⚙️)]

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

Attachment: 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) (:⚙️)]

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

Attachment: 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) (:⚙️)]

Attachment: Side-by-side comparison on Windows 7

Original is on the left, patches applied is on the right.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Side-by-side on OSX

Same as above, except on OSX.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 616722 [details] [diff] [review]
Patch v2

Blake - thoughts?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

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

Attachment: 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)]

Attachment: 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) (:⚙️)]

Thanks Richard.  Just going to wait for bug 746307 to stabilize before testing this.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

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) (:⚙️)]

Committed to comm-central as http://hg.mozilla.org/comm-central/rev/c074ae4d363e

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/07480ef159ca