Closed Bug 1158858 Opened 9 years ago Closed 9 years ago

Move 'suggested' and 'sponsored' labels to the top left of the tile

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
normal
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.3 - Jun 29
Tracking Status
firefox40 --- verified
firefox41 --- verified

People

(Reporter: emtwo, Assigned: emtwo)

References

(Blocks 1 open bug)

Details

(Whiteboard: .?)

Attachments

(9 files, 5 obsolete files)

Blocks: 1140185
Iteration: --- → 40.3 - 11 May
Whiteboard: .?
Blocks: 1160372
No longer blocks: 1160372
No longer blocks: 1150228
Depends on: 1150228
Iteration: 40.3 - 11 May → 41.1 - May 25
dcrobot, how important is it to have the tag on the left side? It being on the left makes it more likely to cover up the beginning of the title. Ideally we don't cover up the beginning or end of a title, but I would think having the first few letters covered up makes it much harder to guess at what the word actually is.
Flags: needinfo?(athornburgh)
Attached patch v1 (obsolete) — Splinter Review
emtwo, I put together a patch so that we have patches for all sponsored suggested tiles bugs. Sorry if you had something wip!
Assignee: msamuel → edilee
Status: NEW → ASSIGNED
Mardak, I can't provide you with any proof that left is "better" than right. So, if you think it's a good idea, then let's keep the labels on the right.
Flags: needinfo?(athornburgh)
Let's hold off on this for now. We need to think more about what behavior we want to better deal with potentially overlapping or avoiding overlapping in the first place.
No longer blocks: 1140185
Iteration: 41.1 - May 25 → ---
Whiteboard: .?
Assignee: edilee → nobody
Status: ASSIGNED → NEW
Attached image Screen Shot.png
Looks like there is a noticeable difference for the title and the label between the mock up and implementation. Aaron, are the specs correct?
Flags: needinfo?(athornburgh)
Indeed... it looks like old styling, and not according to the spec. And yes, my specs are correct.
Flags: needinfo?(athornburgh)
Ed, on another note... maybe we ditch the favicon? That alone would free up 22 pixels for longer titles.

OR, what if we lost the ".com" and ".org" parts?
There shouldn't be a [suggested] tag for history tiles that are showing favicon or domain. Suggested/sponsored shows up for things that we have a custom title. So we just need to make sure we don't use long-ish titles, but that seems somewhat fragile.
Depends on: 1168898
From the design bug 1168898, sounds like we'll be moving the suggested tag to the top left of the tile outside of the title area.
Blocks: 1140185
Summary: Move 'suggested' label to the left of the base domain on the tile → Move 'suggested' label to the top left of the tile
Whiteboard: .?
Summary: Move 'suggested' label to the top left of the tile → Move 'suggested' and 'sponsored' labels to the top left of the tile
Iteration: --- → 41.3 - Jun 29
Assignee: nobody → msamuel
Attachment #8608473 - Attachment is obsolete: true
Attachment #8621694 - Flags: review?(edilee)
Attached image sponsored_tag.png
Attached image sponsored_tag_hover.png
Marina, "Suggested" tags are gray, and "Sponsored" tags are white w/ a gray border.

Otherwise, this looks good!
Attached image hover opacity (obsolete) —
dcrobot, is it okay to have the tag fade on hover? This example has 0.5 opacity so that the blue shadow/glow doesn't look completely cut off.
Attachment #8621716 - Flags: ui-review?(athornburgh)
Attached image hover without transparency (obsolete) —
fyi, how things look with v1 patch on hover
A fade on hover is fine. But the blue glow was intended to show on top of the label anyway.
Comment on attachment 8621694 [details] [diff] [review]
v1: Move sponsored tag to the top left

>+++ b/browser/base/content/newtab/newTab.css
>@@ -202,37 +202,45 @@ input[type=button] {
> .newtab-sponsored {
>-  border: 1px solid #dcdcdc;
>-  border-radius: 2px;
>+  background-color: #E2E2E2;
>+  border: 1px solid #DCDCDC;
>+  color: #4A4A4A;
>+  border-radius: 3px;
>   cursor: pointer;
>   display: none;
>   font-family: Arial;
>-  font-size: 10px;
>+  font-size: 9px;
>   height: 17px;
>-  line-height: 17px;
>-  margin: 5px;
>-  padding: 0 4px;
>+  line-height: 6px;
>+  padding: 4px;
>+  top: -15px;
>+  left: 0;
>+  right: auto;
nit: could you keep these sorted alphabetically
Attachment #8621694 - Flags: review?(edilee) → review+
quick hack moving tag as a sibling of .newtab-site and setting tag z-index 0 and site z-index 1 (probably don't need z-index of other items anymore)
Some changes I wanted confirmation for:

* Moving the glow around .newtab-link instead of .newtab-site so that we don't need to make .newtab-site and .newtab-sponsored siblings

* Added appropriate colour styles for sponsored vs. suggested tags.
Attachment #8621694 - Attachment is obsolete: true
Attachment #8621876 - Flags: review?(edilee)
Comment on attachment 8621876 [details] [diff] [review]
v2: Move sponsored tag to the top left

>+++ b/browser/base/content/newtab/grid.js
>@@ -154,25 +154,25 @@ let gGrid = {
>    */
>   _createSiteFragment: function Grid_createSiteFragment() {
>     let site = document.createElementNS(HTML_NAMESPACE, "div");
>     site.classList.add("newtab-site");
>     site.setAttribute("draggable", "true");
> 
>     // Create the site's inner HTML code.
>     site.innerHTML =
>+      '<input type="button" title="' + newTabString("pin") + '"' +
>+      '       class="newtab-control newtab-control-pin"/>' +
>+      '<input type="button" title="' + newTabString("block") + '"' +
>+      '       class="newtab-control newtab-control-block"/>' +
>       '<a class="newtab-link">' +
>       '  <span class="newtab-thumbnail"/>' +
>       '  <span class="newtab-thumbnail enhanced-content"/>' +
>       '  <span class="newtab-title"/>' +
>       '</a>' +
>-      '<input type="button" title="' + newTabString("pin") + '"' +
>-      '       class="newtab-control newtab-control-pin"/>' +
>-      '<input type="button" title="' + newTabString("block") + '"' +
>-      '       class="newtab-control newtab-control-block"/>' +
>       '<span class="newtab-sponsored">' + newTabString("sponsored.button") + '</span>' +
Move this newtab-sponsored to be the first sibling (before <input button>), and we won't need any of the z-index styling.

>+++ b/browser/themes/shared/newtab/newTab.inc.css
> /* LINKS */
> .newtab-link {
>   border-radius: 6px;
>+  z-index: 1;
As mentioned earlier none of these z-index will be necessary. But we do need to increase the radius to correctly crop the "smaller" link (as opposed to "wider" site). See attached.

Add:

border-radius: 10px;
overflow: hidden;

This will also fix bug 1172721.
Attachment #8621876 - Flags: review?(edilee) → review+
Attached image radius overflow issues
Here's a screenshot with the existing radius: 6px no overflow vs radius: 10px overflow: hidden
Attachment #8621716 - Attachment is obsolete: true
Attachment #8621721 - Attachment is obsolete: true
Attachment #8621716 - Flags: ui-review?(athornburgh)
(In reply to Ed Lee :Mardak from comment #22)
> Move this newtab-sponsored to be the first sibling (before <input button>),
> and we won't need any of the z-index styling.
Actually, I'm not sure if the inputs need to be moved if we do this... ?
Sorry. ;) Removing z-index means we definitely want the <input button> to be after the newtab-link as before.

This bug should also fix bug 1168521 by moving the tag outside of the usual hover area.
Blocks: 1168521, 1172721
Comment on attachment 8621876 [details] [diff] [review]
v2: Move sponsored tag to the top left

>+++ b/browser/base/content/newtab/newTab.css
>+.newtab-site[suggested=true] > .newtab-sponsored {
>+  background-color: #E2E2E2;
>+  border: none;
> }
This has a stronger css rule match.

> .newtab-sponsored:-moz-any(:hover, [active]) {
>-  background-color: #3a72b1;
>+  background-color: #4A90E2;
>   border: 0;
>   color: white;
> }
> 
>+.newtab-sponsored[active] {
>+  background-color: #000000;
>+}
>+
So these also need .newtab-site in front to override the background-color. Yay css ;)
Attached patch v2.1Splinter Review
Attachment #8621876 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/19ba04cd72c2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Points: --- → 5
Flags: qe-verify+
Flags: firefox-backlog+
QA Contact: cornel.ionce
Approval Request Comment
[Feature/regressing bug #]: bug 1168898

[User impact if declined]: Tiles with long titles  will be covered by the sponsored tag

[Describe test coverage new/current, TreeHerder]: tested on nightly and locally on aurora

[Risks and why]: low risk - mostly CSS

[String/UUID change made/needed]: N/A
Attachment #8622745 - Flags: approval-mozilla-aurora?
Comment on attachment 8622745 [details] [diff] [review]
[aurora]  Move sponsored tag to top left

low risk and new feature, taking it.
Attachment #8622745 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1175475
The labels were properly moved on the top left side of the Tile.

Tested on Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 64-bit using:
- latest Nightly, build ID: 20150616030201;
- latest Aurora, build ID: 20150617004006.

This change causes issue 1175475.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.