1158853 - Replace tile titles with base domains and move them on top of the tile image

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Marina Samuel [:emtwo]]

Based on mocks here: https://bug1150228.bugzilla.mozilla.org/attachment.cgi?id=8592387

Comment 1

[Marina Samuel [:emtwo]]

Attachment: suggested.png

Aaron, I've made the changes to include the title/url as part of the tile here.

History tiles display the domain while directory + suggested tiles have the ability to provide a custom title. Just confirming this is what we wanted and that the suggested tile's text 'suggested for <xyz> visitors' should no longer be selectable.

Comment 2

[Marina Samuel [:emtwo]]

Attachment: overlay.png

I specifically wanted to ask about the overlay as seen here. Is the 'suggested' tag meant to be clickable? Or is it the entire title bar on the tile that is now clickable?

Should the overlay be covering the title bar as it is in this screenshot? or be above it?

Comment 3

[Marina Samuel [:emtwo]]

Attachment: WIP

Comment 4

[Aaron]

Clicking the "suggested" label will launch the black overlay – as it currently exists – on top of the screenshot. The overlay should NOT cover the title bar.

Clicking anywhere else in the title bar would launch a New Tab with the detestation page loaded.

Comment 5

[Aaron]

Also, you are correct in Comment #1 regarding tile titles... main-level domain only for History (nfl.com) and client suggested tiles, or a custom title for Mozilla Directory Tiles.

Comment 6

[Marina Samuel [:emtwo]]

Attachment: v1: Replace history tile titles with base domains and include titles as part of tiles

Comment 7

[Marina Samuel [:emtwo]]

Attachment: overlay.png

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 8599885 [details] [diff] [review]
v1: Replace history tile titles with base domains and include titles as part of tiles

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

::: browser/base/content/newtab/sites.js
@@ +121,5 @@
>     */
>    _render: function Site_render() {
>      let enhanced = gAllPages.enhanced && DirectoryLinksProvider.getEnhancedLink(this.link);
>      let url = this.url;
> +    let title = (enhanced && enhanced.title ? enhanced.title : (this.link.type == "history" ? this.link.baseDomain : this.title || this.url));

`this.title || this.url` at the end shouldn't be necessary, since you changed the title getter to return the same thing, right?  Only this.title should be OK.

And could you please write this like:

let title = enhanced && enhanced.title ? enhanced.title :
            this.link.type == "history" ? this.link.baseDomain :
            this.title;

Comment 9

[Pulsebot]

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

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3a0579ae7667

Comment 11

[Ed Lee :Mardak]

Attachment: for beta (Mardak will land)

Comment 12

[Ed Lee :Mardak]

verified with nightly 2015-05-24

1) visited air.mozilla.org
2) saw tile on new tab page with "air.mozilla.org" instead of "Air Mozilla | Mozilla, in Video"
3) saw tile edges contain the title

Comment 13

[Ed Lee :Mardak]

Attachment: verify screenshot

Comment 14

[Cornel Ionce [:noni] [Hubs QA]]

Confirming the fix using latest Aurora, build ID: 20150528004000 on Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit.