Aaron Reporter
	Description • 10 years ago

Problem: The bolded, blue titles beneath the tiles does not clearly indicate "pinned" and introduces confusion.

Solution: For pinned tiles, SHOW AN ICON instead of changing the styling of the tile label.

Attached is a comp and the image assets.

Please place the icon beneath the title – 8 pixels below the bottom edge of the tile – and aligned LEFT.

This change is approved by the FX Desktop Team. Is it possible to make this change in time for our initial release?

	Aaron Reporter
	Comment 1 • 10 years ago

This is the SVG icon to indicate pinned tiles.

	Ed Lee :Mardak Assignee
	Comment 2 • 10 years ago

What should happen if the title is long? Is this in addition to the blue pinned indicator on hover? Compared to what's currently in Release, the description in comment 0 seems to be that the bold/blue indicator caused this confusion, so why not revert to what it was before?

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 3 • 10 years ago

(In reply to Ed Lee :Mardak from comment #2)
> What should happen if the title is long? Is this in addition to the blue
> pinned indicator on hover? Compared to what's currently in Release, the
> description in comment 0 seems to be that the bold/blue indicator caused
> this confusion, so why not revert to what it was before?

I can't find an example right now, but I'm assuming we already have a way of dealing with collisions between the titles and the [Sponsored] indicator. We can shrink the size of the text area in general to make room for the pin.

And yes, this is in addition to the blue pin :)

Some kind visible indication for pinned tabs makes sense because especially with enhanced tiles, the image of tiles might change quite a bit over time. Showing the pin is a good way to reassure the user that this is still the same tile he pinned earlier, without getting in their way too much.

	Ed Lee :Mardak Assignee
	Comment 5 • 10 years ago

There's no explicit handling of [sponsored] and long titles as the title shown is controlled and should short. But pinned tiles can be for any tile so history titles could be long. I'm assuming if we do shorten the title so that it doesn't overlap the pinned indicator, the title should still appear visually centered with the tile? Should the title extend all the way to the right edge or crop sooner so it's balanced?

Centered ignoring the [p] pin indicator space:
[tile image goes here.]
[p]     center

Longer title extending all the way right:
[tile image goes here.]
[p] title title titl...

Longer title equally balanced center (as if there was an invisible [p] on the right):
[tile image goes here.]
[p] center title...

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 6 • 10 years ago

Yeah, it should be cut off equally to look centered at all times (so option 3).

Perhaps we can move it a few pixels to the right to balance out the »...« which have less visual weight.
But that last part is fine-tuning and could totally be a follow-up

Aaron, please chime in here if you disagree with any of that :)

	Aaron Reporter
	Comment 7 • 10 years ago

I agree! This pin is only 10 pixels wide... add 5 pixels for padding, and you only lose 15 pixels altogether.

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 8 • 10 years ago

(In reply to Aaron from comment #7)
> I agree! This pin is only 10 pixels wide... add 5 pixels for padding, and
> you only lose 15 pixels altogether.

Well, it's really 30 pixels since it's on both sides, right?
Should still be unproblematic though.

	Ed Lee :Mardak Assignee
	Comment 9 • 10 years ago

Who would know how to convert attachment 8490237 [details] svg to be like http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/newtab/controls.svg?raw=1 ?

The attached file seems to have a shadow encoded in base64. I'm guessing the <path> would be similar to the existing     <path    id="glyphShape-pin" d="M19,15v-5h2V7H11v3h2v5l-3,2v2h5c0,4.5,0.4,8,1,8s1-3.5,1-8h5v-2L19,15z" /> if not the same? There just wouldn't be a #glyphShape-circle for the icon. ?

	Ed Lee :Mardak Assignee
	Comment 10 • 10 years ago

Here's my attempt at editing the svg file directly. I've never done this before, so danger! :p

	Ed Lee :Mardak Assignee
	Comment 11 • 10 years ago

Resulting controls.svg from attachment 8490339 [details] [diff] [review] patch.

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 12 • 10 years ago

Hey Ed, is it possible to implement and uplift this to beta before tiles move to Release in ~3 weeks?

	Ed Lee :Mardak Assignee
	Comment 13 • 10 years ago

You can set the various backlog and tracking flags (e.g., tracking-firefox33) with explanations of why this needs to be uplifted.

In terms of implementation, I would expect the changes to be css, svg, xul changes; so no js code to review. Should be relatively quick to fix when prioritized.

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 14 • 10 years ago

[Tracking Requested - why for this release]: Tiles will hit GA with the 33 train. The current way of indicating pinned tiles (blue and bold text) is ambiguous and has caused some confusion. In order not to carry that confusion to GA, we need to move this to beta before the trains.

	Sylvestre Ledru [:Sylvestre]
	Comment 15 • 10 years ago

OK. If possible, I would like that to land before beta 8 (September, 29th).

	Tim Taubert [:ttaubert] (inactive)
	Comment 18 • 10 years ago

So why are we using an SVG here? Shouldn't we just export that to PNG in 1x and 2x resolution like we do for every other icon?

	Tim Taubert [:ttaubert] (inactive)
	Comment 19 • 10 years ago

Oh, we're already using an SVG for the controls, I see. Nevermind then, didn't know we changed that.

	Tim Taubert [:ttaubert] (inactive)
	Comment 20 • 10 years ago

Comment on attachment 8494993 [details] [diff] [review]
v1

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

::: browser/themes/shared/newtab/newTab.inc.css
@@ +146,5 @@
>  }
>  
>  .newtab-site[pinned] .newtab-title {
> +  padding: 0 15px;
> +}

This leads to an unnecessary padding on the right with long titles as seen in your screenshot. We can remove this if below we remove |position: absolute|, |left: 0|, and insert:

float: left;
-moz-margin-end: 5px;

And don't forget RTL :)

-moz-locale-dir(rtl) {
  float: right;
}

	Ed Lee :Mardak Assignee
	Comment 21 • 10 years ago

(In reply to Tim Taubert [:ttaubert] from comment #20)
> This leads to an unnecessary padding on the right with long titles as seen
> in your screenshot.
Thanks. I was thinking about that as well, but comment 6 says to keep the title centered at all times although it also mentions perhaps offsetting it a little because the ... isn't as heavy as text.

The tricky aspect of putting margins on one side is that the text might not be perfectly centered with the tile (even without the ...).

	Ed Lee :Mardak Assignee
	Comment 22 • 10 years ago

(In reply to Tim Taubert [:ttaubert] from comment #20)
> float: left;
> -moz-margin-end: 5px;
Here's a screenshot where the title is no longer centered with the tile. Pinning and unpinning the tile causes the title to jump right/left a little bit.

	Tim Taubert [:ttaubert] (inactive)
	Comment 23 • 10 years ago

You're right, that is worse than losing 15px on the right I guess. Too bad, sorry. The only thing left to fix is then doing {left: auto; right: 0} for RTL builds.

	Tim Taubert [:ttaubert] (inactive)
	Comment 25 • 10 years ago

Comment on attachment 8495055 [details] [diff] [review]
v2

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

Thanks!

	Ed Lee :Mardak Assignee
	Comment 26 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/a4c45db3f0a6

	Carsten Book [:Tomcat]
	Comment 27 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/a4c45db3f0a6

	Sylvestre Ledru [:Sylvestre]
	Comment 28 • 10 years ago

Ed, I am sure you are going to ask for an uplift in aurora & beta but n-i to make sure this happens asap. Thanks

	Ed Lee :Mardak Assignee
	Comment 30 • 10 years ago

Comment on attachment 8496362 [details] [diff] [review]
for uplift

Approval Request Comment
[Feature/regressing bug #]: Bug 1030832 / Enhanced Tiles
[User impact if declined]: Confusion about why titles are blue and bold
[Describe test coverage new/current, TBPL]: None
[Risks and why]: Low - style tweaks to css and svg files
[String/UUID change made/needed]: None

	Ed Lee :Mardak Assignee
	Comment 31 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/aba04e434a11

status-firefox33 wontfix per bug 1073823

	Ed Lee :Mardak Assignee
	Comment 32 • 10 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/02da3cf36508

	Cornel Ionce [:noni] [Hubs QA]
	Comment 33 • 10 years ago

Verified fixed on Windows 7 64-bit, Windows 8.1 64-bit, Ubuntu 12.04 and Mac OS X 10.9 using:
- latest Nightly, build ID: 20140928030206
- latest Aurora, build ID: 20140928004003

	Cornel Ionce [:noni] [Hubs QA]
	Comment 34 • 10 years ago

Confirming this fix on Firefox 33 beta 8, build ID: 20140929180120.

User Agents:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:33.0) Gecko/20100101 Firefox/33.0
Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0