Closed
Bug 1158853
Opened 9 years ago
Closed 9 years ago
Replace tile titles with base domains and move them on top of the tile image
Categories
(Firefox :: New Tab Page, defect)
Firefox
New Tab Page
Tracking
()
People
(Reporter: emtwo, Assigned: emtwo)
References
(Blocks 1 open bug)
Details
(Whiteboard: .?)
Attachments
(5 files, 2 obsolete files)
2.66 MB,
image/png
|
Details | |
8.05 KB,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
1.43 MB,
image/png
|
Details | |
8.30 KB,
patch
|
Details | Diff | Splinter Review | |
189.63 KB,
image/png
|
Details |
Based on mocks here: https://bug1150228.bugzilla.mozilla.org/attachment.cgi?id=8592387
Assignee | ||
Comment 1•9 years ago
|
||
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.
Attachment #8599551 -
Flags: ui-review?(athornburgh)
Assignee | ||
Comment 2•9 years ago
|
||
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?
Attachment #8599553 -
Flags: ui-review?(athornburgh)
Assignee | ||
Comment 3•9 years ago
|
||
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.
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.
Updated•9 years ago
|
Whiteboard: .?
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8599558 -
Attachment is obsolete: true
Attachment #8599885 -
Flags: review?(adw)
Assignee | ||
Updated•9 years ago
|
Attachment #8599551 -
Flags: ui-review?(athornburgh)
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8599553 -
Attachment is obsolete: true
Attachment #8599553 -
Flags: ui-review?(athornburgh)
Comment 8•9 years ago
|
||
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;
Attachment #8599885 -
Flags: review?(adw) → review+
Updated•9 years ago
|
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3a0579ae7667
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•9 years ago
|
Points: --- → 5
Comment 11•9 years ago
|
||
Comment 12•9 years ago
|
||
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
Status: RESOLVED → VERIFIED
status-firefox41:
--- → verified
Comment 13•9 years ago
|
||
Comment 14•9 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•