Buttons in Account Provisioner do not wrap long text

RESOLVED FIXED in Thunderbird 11.0

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

11 Branch
Thunderbird 11.0
x86
All

Thunderbird Tracking Flags

(thunderbird10+ fixed)

Details

Attachments

(3 attachments, 4 obsolete attachments)

Created attachment 582813 [details]
Screenshot of problem in action

The text inside the Account Provisioner buttons is not wrapped for long text.  See screenshot.
tracking-thunderbird10: --- → +
Depends on: 700536
Created attachment 582815 [details] [diff] [review]
Patch v1

Andreas:

Does this make sense, or will I be introducing a regression?

-Mike
Assignee: nobody → mconley
Attachment #582815 - Flags: feedback?(nisses.mail)
Target Milestone: --- → Thunderbird 11.0
Comment on attachment 582815 [details] [diff] [review]
Patch v1

I'm unable to test with a german locale, but I don't see any regressions to the english version, so I think we should be good. We really want to fit german and dutch.
Attachment #582815 - Flags: feedback?(nisses.mail) → feedback+
Comment on attachment 582815 [details] [diff] [review]
Patch v1

Okie doke, let's see what bwinton thinks.
Attachment #582815 - Flags: ui-review?(bwinton)
Attachment #582815 - Flags: review?(bwinton)
Attachment #582815 - Flags: approval-comm-aurora?
Comment on attachment 582815 [details] [diff] [review]
Patch v1

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

Cool, r=me.

And ui-r-, based on the demo you gave.  :(

Thanks,
Blake.
Attachment #582815 - Flags: ui-review?(bwinton)
Attachment #582815 - Flags: ui-review-
Attachment #582815 - Flags: review?(bwinton)
Attachment #582815 - Flags: review+
Created attachment 582962 [details] [diff] [review]
Updated patch

This should take care of the issue with text overflowing the button itself.
Still needs a tiny bit of polish in the case where one button has one line and one have two lines of text.
Attachment #582962 - Flags: feedback?(mconley)
Comment on attachment 582962 [details] [diff] [review]
Updated patch

So far so good.  Assuming we get things lined up in the case you describe (one button having more lines than the other), and assuming we apply the changes to all themes, I think we're on the right track.
Attachment #582962 - Flags: feedback?(mconley) → feedback+
Attachment #582815 - Attachment is obsolete: true
Attachment #582815 - Flags: approval-comm-aurora?
Created attachment 582984 [details] [diff] [review]
patch v3

Ok, this should take care of the buttons looking misaligned if one have more lines of text than the other. Both are now aligned to the top.
Attachment #582962 - Attachment is obsolete: true
Attachment #582984 - Flags: feedback?(mconley)
Comment on attachment 582984 [details] [diff] [review]
patch v3

Works brilliantly!  If you can prep it for Windows / OSX too, we'll get bwinton to r/ui-r it, and we're off to the races.
Attachment #582984 - Flags: feedback?(mconley) → feedback+
Created attachment 582995 [details] [diff] [review]
patch v4

Fixes from previous patch carried over to Qute and Pinstripe as well.
Attachment #582984 - Attachment is obsolete: true
Attachment #582995 - Flags: ui-review?(bwinton)
Attachment #582995 - Flags: review?(bwinton)
Created attachment 583023 [details]
screenshot of patch v4 to ease review
Comment on attachment 582995 [details] [diff] [review]
patch v4

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

Aside from the question below, I like it, so r=me.

And ui-r=me based on the screenshot.

Thanks,
Blake.

::: mail/themes/gnomestripe/mail/newmailaccount/accountProvisioner.css
@@ +518,5 @@
>    margin-left: 7px;
>    margin-right: 8px;
> +  display: inline;
> +  max-width: 260px;
> +  min-width: 260px;

If we're setting the max and min width to the same thing, can't we just set the width?
Attachment #582995 - Flags: ui-review?(bwinton)
Attachment #582995 - Flags: ui-review+
Attachment #582995 - Flags: review?(bwinton)
Attachment #582995 - Flags: review+
Created attachment 583156 [details] [diff] [review]
updated patch with just width instead of max and min-width

Indeed. Used min-width first and then just added max-width as well and forgot about it. Here is a patch with just width.
Carrying over review+ and ui-review+ from previous patch.
Attachment #582995 - Attachment is obsolete: true
Attachment #583156 - Flags: ui-review+
Attachment #583156 - Flags: review+
Keywords: checkin-needed
Comment on attachment 583156 [details] [diff] [review]
updated patch with just width instead of max and min-width

Our localizers will want this for TB 10.
Attachment #583156 - Flags: approval-comm-aurora?
Committed to trunk as http://hg.mozilla.org/comm-central/rev/8b6248df4f24
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Attachment #583156 - Flags: approval-comm-aurora? → approval-comm-aurora+
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/28c353f24518
status-thunderbird10: --- → fixed
tracking-thunderbird10: + → ---
tracking-thunderbird10: --- → +
Keywords: checkin-needed
The latest build of thunderbird 10 is from dec 20 and there it is not fixed yet (same with nightly). but i assume that in the next build it will be....

Thanks for picking up my report in 700536 so quickly.
Tim:

Hm - the fix *should* be in our nightly build.  Could you try last night's nightly and see if the fix is there?

-Mike
(In reply to Mike Conley (:mconley) from comment #17)
> Tim:
> 
> Hm - the fix *should* be in our nightly build.  Could you try last night's
> nightly and see if the fix is there?
> 
> -Mike

The build from ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/ doesn't run on mij sytem and from ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-aurora-l10n/ doesn't have te fix.
Hm...Mark, any idea why this fix wouldn't be in the l10n builds yet?
(In reply to Tim Maks van den Broek from comment #18)
> (In reply to Mike Conley (:mconley) from comment #17)
> > Tim:
> > 
> > Hm - the fix *should* be in our nightly build.  Could you try last night's
> > nightly and see if the fix is there?
> > 
> > -Mike
> 
> The build from
> ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/
> doesn't run on mij sytem and from
> ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-aurora-l10n/
> doesn't have te fix.

ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-aurora-l10n/ 28 dec build is fixed (earlybird 11)

ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/ i had the wrong architecture :-(, 28 dec build is fixed (daily 12)

ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-candidate-comm-beta/linux-i686/nl/ is the account provisioner in English?!?
(In reply to Tim Maks van den Broek from comment #20)

> ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-candidate-comm-beta/
> linux-i686/nl/ is the account provisioner in English?!?

= Buildnaam: Mozilla/5.0 (X11; Linux i686; rv:10.0) Gecko/20111222 Thunderbird/10.0
You need to log in before you can comment on or make changes to this bug.