872544 - Button labels in the Australis menu panel get truncated too easily

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[(no longer active)]

First of all, its text gets cut off and ellipsis are shown, also its tooltip reads "Open a new Private Browsing window", it should be "Open a new private window".

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

The biggest part of this bug will be handling the word wrapping and trying not to truncate the labels.

Comment 2

[(no longer active)]

(In reply to comment #1)
> The biggest part of this bug will be handling the word wrapping and trying not
> to truncate the labels.

We should also have a strategy for languages where this text might be exceedingly long...

Comment 3

[Dão Gottwald [:dao]]

Maybe just have tooltips and drop the labels, just like for buttons in the toolbar?

Comment 4

[u428464]

Or just have the label on two rows.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot with no labels

Stephen, this is what it looks like without the labels. What do you think about it?

I think it might be too hard for people to figure out what some of these icons are without the label (like Save Page As...). There are tooltips present, but without stopping the mouse the user won't see it.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

An alternative approach may be to have a larger text area that is used to show the labels when the mouse hovers over an item.

Please excuse my ASCII-art quality...

                   /\
/----------------------\
| [Cut] [Copy] [Paste] |
|
| [ - ] [ 100% ] [ + ] |
|                      |
| [[ x ]] [ x ]  [ x ] |
| [  x  ] [ x ]  [ x ] |
| [  x  ] [ x ]  [ x ] |
|                      |
| Save this webpage... |
|                      |
------------------------
| [help]    [customize]|
\----------------------/

Where [[ x ]] means that the button is being hovered.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

One downside to comment #6 is that it isn't clear what we would do for touch UI.

Comment 8

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

This is slipping to M7.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

We're going to go with showing two lines of text and clipping any longer text. The text will also be available as a tooltip. I am going to try to fade out the text from the bottom using an SVG mask.

Comment 11

[:Gijs (he/him)]

Attachment: wrap toolbarbutton labels in panels

If/when the patch I added to bug 880918 gets review, I think this is what's needed (at a minimum) to get this right in the panel. Note that the fadeout that Jared mentioned isn't implemented yet. Otherwise, I've tested this in concert with my other patch, and this seems to do what we want.

Comment 12

[:Gijs (he/him)]

Attachment: wrap toolbarbutton labels in panels

Updating this patch because of the switch to type rather than textwrap attributes.

Comment 13

[:Gijs (he/him)]

Egh, didn't want to steal this. Bad bzexport, bad.

However, I could if that helps you focus on performance stuff - but I'm not sure what the plan was in terms of how to get the cut-off stuff right.

Jared: how were you planning on establishing "two lines" and cutting off the rest? Styling .toolbarbutton-text with a max-height of the appropriate em value, or some other way? I'm also not entirely clear on how the SVG mask would come in - for the 3rd-nth line? How big would it be? And would having a vertical fadeout look OK? I guess I'm afraid it would look silly... but maybe I'm worrying too much about it.

Anyway, feel free to assign to me if you could quickly describe what your plan was in this respect.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gijs Kruitbosch from comment #13)
> Egh, didn't want to steal this. Bad bzexport, bad.
 
For all intents and purposes, it is better that the bug be associated with you since you are the one working on it now :)

> Jared: how were you planning on establishing "two lines" and cutting off the
> rest? Styling .toolbarbutton-text with a max-height of the appropriate em
> value, or some other way? I'm also not entirely clear on how the SVG mask
> would come in - for the 3rd-nth line? How big would it be? And would having
> a vertical fadeout look OK? I guess I'm afraid it would look silly... but
> maybe I'm worrying too much about it.

I was thinking that we could set .toolbarbutton-text { max-height: 2.75em; } and then use an SVG mask that is positioned at the bottom of the box that fades from opacity:1 to opacity:0 for .75em. This should make a third line look faded out.

As to if it looks silly, I guess we may not know until it is implemented? As a quick hack (and not using the same implementation approach), this is what it looks like when if we were to fade out the bottom line by 75%. I found that 50% cut off too much of the characters since the majority of characters don't have ascenders.

> Anyway, feel free to assign to me if you could quickly describe what your
> plan was in this respect.

I hope that this explanation is clearer, but please ask for any further explanations if needed.

Comment 15

[:Gijs (he/him)]

Attachment: wrap toolbarbutton labels in panels

Updated once more now the type attribute is shorter... (I'll do a separate patch for the styling changes; that can land separately because we don't actually have buttons with such long labels right now)

Comment 16

[:Gijs (he/him)]

Attachment: wrap toolbarbutton labels in panels

Unbitrotted.

Comment 17

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] from comment #14)
> (In reply to :Gijs Kruitbosch from comment #13)
> > Egh, didn't want to steal this. Bad bzexport, bad.
>  
> For all intents and purposes, it is better that the bug be associated with
> you since you are the one working on it now :)
> 
> > Jared: how were you planning on establishing "two lines" and cutting off the
> > rest? Styling .toolbarbutton-text with a max-height of the appropriate em
> > value, or some other way? I'm also not entirely clear on how the SVG mask
> > would come in - for the 3rd-nth line? How big would it be? And would having
> > a vertical fadeout look OK? I guess I'm afraid it would look silly... but
> > maybe I'm worrying too much about it.
> 
> I was thinking that we could set .toolbarbutton-text { max-height: 2.75em; }
> and then use an SVG mask that is positioned at the bottom of the box that
> fades from opacity:1 to opacity:0 for .75em. This should make a third line
> look faded out.
> 
> As to if it looks silly, I guess we may not know until it is implemented? As
> a quick hack (and not using the same implementation approach), this is what
> it looks like when if we were to fade out the bottom line by 75%. I found
> that 50% cut off too much of the characters since the majority of characters
> don't have ascenders.

So setting max-height on the label with overflow-y: hidden doesn't seem to work; the text still shows up. Setting overflow-y: hidden on the toolbarbutton fixes that but starts stretching the images on the toolbar button horizontally (!?). I... don't understand how it does that. If you have ideas, they'd be welcome.

Regarding the SVG mask: you can't position SVG masks relative to the element they're masking (discussed at length in bug 658467) so I went with trying to overlay a fadeout linear gradient using an absolutely positioned :after element, but I'm not having much luck with that either. Not entirely sure why yet.

Comment 18

[:Gijs (he/him)]

Attachment: wrap toolbarbutton labels in panels

This last version was suboptimal and assigned the attribute to non-toolbarbuttons. Instead, this should follow the same path as tabindex and add/remove as appropriate.

Comment 19

[Mike de Boer [:mikedeboer]]

Comment on attachment 779193 [details] [diff] [review]
wrap toolbarbutton labels in panels

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

As a first pass/ part of the complete solution, the code looks fine.

I'm a bit concerned though if this yields the effect we'd like. In the screenshot you'll see that the centered labels look badly aligned, whilst technically they are aligned perfectly when you take the '...' in account.

If UX is ok with signing this off, I'd say it's good to go!

Comment 20

[Mike de Boer [:mikedeboer]]

Attachment: indentation of multiline labels

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Now that "Private Browsing..." acts the same as "New Window" (just that it's a private window), I think we can drop the ellipsis from the label anyways.

Comment 22

[(no longer active)]

(In reply to comment #21)
> Now that "Private Browsing..." acts the same as "New Window" (just that it's a
> private window), I think we can drop the ellipsis from the label anyways.

Yes please!

Comment 23

[:Gijs (he/him)]

Attachment: remove ellipsis from 'Private Browsing...'

Per comment 21/22, let's just do this.

Comment 24

[Dão Gottwald [:dao]]

Comment on attachment 780022 [details] [diff] [review]
remove ellipsis from 'Private Browsing...'

Why is this "Private Browsing" rather than "New Private Window" like in the menu bar?

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 780022 [details] [diff] [review]
remove ellipsis from 'Private Browsing...'

r=me with the change to "New Private Window"

Comment 26

[Dão Gottwald [:dao]]

You also need to remove the l10n note.

Comment 27

[:Gijs (he/him)]

Attachment: Indentation of "New Private Window"

Here's a screenshot with the new label for Stephen's reference. Apologies for it being a huge retina one, my windows build is busted so I couldn't easily try it there.

Comment 28

[:Gijs (he/him)]

Attachment: update private browsing label to match menu,

Comment 29

[:Gijs (he/him)]

Comment on attachment 780411 [details] [diff] [review]
update private browsing label to match menu,

Carrying over r+.

Comment 30

[Mike de Boer [:mikedeboer]]

Gijs, this looks very good. I'll cancel the ui-r, since this update obsoletes my initial concern.

Comment 31

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/40a2ec68fb77
https://hg.mozilla.org/projects/ux/rev/01fb059f99db

Pushed. I'm filing a followup for fading out the bottom, which was trickier than Jared and I initially thought. IMHO that is lower prio now that we shouldn't be cutting things off anymore.

Comment 32

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/01fb059f99db
https://hg.mozilla.org/mozilla-central/rev/40a2ec68fb77