Closed Bug 872544 Opened 8 years ago Closed 7 years ago

Button labels in the Australis menu panel get truncated too easily

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: ehsan, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:M8][Australis:P2])

Attachments

(5 files, 5 obsolete files)

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".
The biggest part of this bug will be handling the word wrapping and trying not to truncate the labels.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
OS: Windows 8 → All
Hardware: x86_64 → All
Whiteboard: [Australis:M?] → [Australis:M6]
(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...
Maybe just have tooltips and drop the labels, just like for buttons in the toolbar?
Or just have the label on two rows.
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.
Attachment #754877 - Flags: ui-review?(shorlander)
Attachment #754877 - Attachment is patch: false
Attachment #754877 - Attachment mime type: text/plain → image/png
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.
One downside to comment #6 is that it isn't clear what we would do for touch UI.
Summary: The private browsing command in the chrome wrench dialog is suboptimal → The private browsing command in the Australis menu panel is suboptimal
Summary: The private browsing command in the Australis menu panel is suboptimal → Button labels in the Australis menu panel get truncated too easily
This is slipping to M7.
Whiteboard: [Australis:M6] → [Australis:M7]
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.
Attachment #754877 - Flags: ui-review?(shorlander)
Whiteboard: [Australis:M7] → [Australis:M?]
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P2]
Duplicate of this bug: 889242
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.
Updating this patch because of the switch to type rather than textwrap attributes.
Attachment #777082 - Attachment is obsolete: true
Assignee: jaws → gijskruitbosch+bugs
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.
Assignee: gijskruitbosch+bugs → jaws
(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.
Assignee: jaws → gijskruitbosch+bugs
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)
Attachment #778466 - Flags: review?(mdeboer)
Attachment #777701 - Attachment is obsolete: true
Unbitrotted.
Attachment #779122 - Flags: review?(mdeboer)
Attachment #778466 - Attachment is obsolete: true
Attachment #778466 - Flags: review?(mdeboer)
(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.
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.
Attachment #779193 - Flags: review?(mdeboer)
Attachment #779122 - Attachment is obsolete: true
Attachment #779122 - Flags: review?(mdeboer)
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!
Attachment #779193 - Flags: review?(mdeboer) → review+
Attachment #779207 - Flags: ui-review?(shorlander)
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.
(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!
Per comment 21/22, let's just do this.
Attachment #780022 - Flags: review?(jaws)
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 on attachment 780022 [details] [diff] [review]
remove ellipsis from 'Private Browsing...'

r=me with the change to "New Private Window"
Attachment #780022 - Flags: review?(jaws) → review+
You also need to remove the l10n note.
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.
Attachment #780022 - Attachment is obsolete: true
Comment on attachment 780411 [details] [diff] [review]
update private browsing label to match menu,

Carrying over r+.
Attachment #780411 - Flags: review+
Gijs, this looks very good. I'll cancel the ui-r, since this update obsoletes my initial concern.
Attachment #779207 - Flags: ui-review?(shorlander)
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.
Whiteboard: [Australis:M?][Australis:P2] → [Australis:M8][Australis:P2][fixed-in-ux]
Blocks: 897496
https://hg.mozilla.org/mozilla-central/rev/01fb059f99db
https://hg.mozilla.org/mozilla-central/rev/40a2ec68fb77
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.