522598 - Finalize padding for text-beside toolbar buttons

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect, P3)

Description

[Phil Ringnalda (:philor)]

Three issues, two of them because I was in too much of a hurry to land bug 519413:

* we need to decide whether we like the toolbar's current 3px, or the message reader's current 2px

* switching from text-beside to text-only shouldn't get the text-beside padding

* because the buttons that are in decks, like Junk and Delete, get a label the width of the longer of the Junk/Not Junk labels, we need to switch them from text-align: center to text-align: start, so that the extra space (which will still make a big space between the label and the next button) isn't as obvious

I think p3-blocking is the one I mean, "we'd hold for an hour," which ought to be enough since Blake already has most if not all of it done.

Comment 1

[Phil Ringnalda (:philor)]

Forgot my blocking justification, after all that time waiting for mxr - adding blank space half the width of "Not " before "Junk" in en-US isn't all that horrible, but adding half the width of "Không phải " before "Thư rác" is going to be pretty gappy.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

The patch in bug 519688 at least makes the blank space problem better, so once that lands, it's possible that this might no longer block.

Comment 3

[Phil Ringnalda (:philor)]

It does? I don't see anything in that patch that would apply to toolbar buttons, only to msgHeaderView buttons. But this is well in hand, since it's just the patch Blake already had and wanted to land in bug 519413 plus text-align for a couple of buttons. We can slip it if we have to, but I doubt we'll have to.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Right you are; I didn't read comment 0 quite closely enough.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Attachment: The first cut at the patch.

I've gone with 2px, and fixed the other two bugs, so let's see how we all like this patch.

Thanks,
Blake.

Comment 6

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 406968 [details] [diff] [review]
The first cut at the patch.

this looks good to me, though my speed of review has bitrotted this patch in the qute primaryToolbar.css

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Attachment: The de-rotted version of the patch.

And here's the patch, de-rotted.

Thanks,
Blake.

Comment 8

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/0ad0ee8ded17