Closed Bug 1036284 Opened 6 years ago Closed 6 years ago

Update styling of newtab tiles to enhanced tiles spec

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
normal
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.1
Tracking Status
firefox33 --- verified
firefox34 --- verified

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

Attachments

(2 files, 13 obsolete files)

Update tile-related styling from bug 1030832: (unhover, hover, pinned): resize tiles to 290x180, center text, radius, shadow, fading/opacity
Flags: firefox-backlog+
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #8452905 - Flags: review?(adw)
Attached image v1 screenshot (obsolete) —
Attached patch v1.1 (obsolete) — Splinter Review
Patch didn't include the gradient overlay on history thumbnail (depends on bug 1036280).
Attachment #8452905 - Attachment is obsolete: true
Attachment #8452905 - Flags: review?(adw)
Attachment #8452916 - Flags: review?(adw)
Depends on: 1036280
QA Whiteboard: [qa+]
Iteration: 33.3 → ---
Attached patch v1.2 (obsolete) — Splinter Review
Added the test change for pinned attr check
Attachment #8452916 - Attachment is obsolete: true
Attachment #8452916 - Flags: review?(adw)
Attachment #8453190 - Flags: review?(adw)
dcrobot says the page's background texture needs to be updated and search button font size reduced
Attached image Enhanced_Tiles_Icons.svg (obsolete) —
Attached image Enhanced_Tiles_Icons.svg (obsolete) —
dcrobot's updated svg
Attachment #8454068 - Attachment is obsolete: true
Attached image SVG Icons (obsolete) —
shorlander, do you know why dcrobot's svg files don't work with -moz-image-rect? Is it because it doesn't specify a height and width? Even if I specify height="56px", it seems that the icons are not completely left aligned nor are they exactly 56x56 (probably because of the shadow)?

How would you create these svg files to be the correct size and spacing?
(In reply to Aaron from comment #8)
> Created attachment 8454088 [details]
> SVG Icons
There's actually 6 states for the pin button:
1) unpinned static
2) unpinned rollover
3) unpinned active
4) pinned static
5) pinned rollover
6) pinned active

We could just combine (1 and 6), (2 and 5), and (3 and 4).

See http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/newtab/controls@2x.png
Attached patch v2 (obsolete) — Splinter Review
Added texture and adjusted search sizing.
Attachment #8452906 - Attachment is obsolete: true
Attachment #8453190 - Attachment is obsolete: true
Attachment #8454086 - Attachment is obsolete: true
Attachment #8453190 - Flags: review?(adw)
Attached image v2 screenshot (obsolete) —
(In reply to Ed Lee :Mardak from comment #11)
> Created attachment 8454324 [details] [diff] [review]
> v2
> 
> Added texture and adjusted search sizing.

Two questions:
1) Why are we re-adding texture? Our incontent UI has been moving away from that.
2) Why is the border-radius so large on the tile? It isn't consistent with any of our other rounded corners.
(In reply to Ed Lee :Mardak from comment #9)
> shorlander, do you know why dcrobot's svg files don't work with
> -moz-image-rect? Is it because it doesn't specify a height and width? Even
> if I specify height="56px", it seems that the icons are not completely left
> aligned nor are they exactly 56x56 (probably because of the shadow)?
> 
> How would you create these svg files to be the correct size and spacing?

Well, the SVG coordinate system is kind of wonky and I don't think the way Illustrator exports SVG is helping here. Looks like all of the coordinates are offset pretty far into the canvas and the viewBox is just defining the visible area. It is also adding a decimal place (or several).

I have had the best results using SVG in product by making the canvas the actual size I want and matching the viewBox to that. Also by sticking to only whole numbers to avoid rounding errors.

You could clean it up a lot by referencing common shapes with xlink:href and by translating them the exact distance you want for your sprites. It also appears to be embedding the shadow as a bitmap, you could use a filter for that.
As for the dimensions, the base circle is ~58px with the shadow it is more like ~72px. Are they supposed to be 56px?
Actually, the base circle is 28 x 28 pixels. I thought I had fixed the SVG file yesterday, but it didn't save out. I'm attaching a new version.
The base circles should all be 28 x 28 pixels now.
(In reply to Aaron from comment #17)
> Created attachment 8454490 [details]
> Enhanced_Tiles_Icons.svg (CORRECT VERSION)
> The base circles should all be 28 x 28 pixels now.
dcrobot, not sure if you noticed but the SVG files went from 85.02 KB to 17.42 KB to now 239.28 KB.

I'm not sure what's the different (I'm thinking it's a width/height being set), shorlander's svg https://bug879921.bugzilla.mozilla.org/attachment.cgi?id=8396689 file causes the icons to be displayed in the top left corner of the browsing window whereas your attachment is vertically centered.

I'm thinking that's related to what's preventing the code from correctly slicing the image into buttons.
Attached image Enhanced_Tiles_Icons-edited.svg (obsolete) —
Edited by shorlander derived from the existing close.svg that works in Firefox already.
Attachment #8454088 - Attachment is obsolete: true
Attachment #8454490 - Attachment is obsolete: true
(In reply to Stephen Horlander [:shorlander] from comment #13)
> 1) Why are we re-adding texture? Our incontent UI has been moving away from
> that.
> 2) Why is the border-radius so large on the tile? It isn't consistent with
> any of our other rounded corners.
Are there guidelines or history for dcrobot to refer to?
Horlander -

1.) When I first joined, I heard that some users felt that Firefox was feeling more like a "chrome" experience than "firefox". I added the texture to create a more natural feel and to better differentiate the new Tiles experience from Google's version. Also, I haven't been privy to any history concerning design ethos... just some general guidelines and whatever I can see with the Chameleon Sketch file. Having said all that, I'm happy to loose the texture if you'd like.

2.) The corner radius is 12 pixels. My reasoning is that I want the content work we're doing to feel related to the browser UI, but not an exact duplication. I don't want users confusing content with actual browser tools / functionality. More importantly, we've been thinking about moving towards circles for the actual tiles. The larger corner radius is intended to be a transition from rectangles to circles. Again, this is all just context. If you would prefer a different radius, then please specify what the standard is and I'll have Ed update the prototype.

-A
Ed, I don't think I ever gave you specs for the empty tile state...

Size = 290 x 180
Border thickness = 2
HEX = C1C1C1
Dash = 5
Gap = 5

No rollover effects or active states at this time.
Blocks: 1036299
Attached patch v2.1 (obsolete) — Splinter Review
Attachment #8454324 - Attachment is obsolete: true
Attachment #8458450 - Flags: review?(adw)
Attached image v2.1 screenshot
Attachment #8454325 - Attachment is obsolete: true
Attachment #8454562 - Attachment is obsolete: true
Attached patch v2.2 (obsolete) — Splinter Review
Fixes a test failure with drag/drop because of the overlaid gradient used to prevent immediate dragging.
Attachment #8458450 - Attachment is obsolete: true
Attachment #8458450 - Flags: review?(adw)
Attachment #8458768 - Flags: review?(adw)
Depends on: 1040282
Comment on attachment 8458768 [details] [diff] [review]
v2.2

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

::: browser/themes/shared/newtab/newTab.inc.css
@@ +130,5 @@
>    background-position: center center;
>    background-size: auto;
>  }
>  
> +.newtab-site[type=history]:not(:hover) .newtab-thumbnail:first-child:after {

A brief comment explaining this selector or at least the .newtab-thumbnail part would be helpful.
Attachment #8458768 - Flags: review?(adw) → review+
I had to backout for Windows 7 bc1 crashes:
https://hg.mozilla.org/integration/fx-team/rev/3b09d30c885f

XP and 8 seemed to run fine though:
https://tbpl.mozilla.org/?tree=Fx-Team&rev=0d2f538bddde
Attached patch v3Splinter Review
Turns out the windows 7 failures were because the window wasn't wide enough. Updated head.js to also adjust the innerWidth in addition to innerHeight.
Attachment #8458768 - Attachment is obsolete: true
Attachment #8460512 - Flags: review?(adw)
Comment on attachment 8460512 [details] [diff] [review]
v3

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

::: browser/base/content/test/newtab/head.js
@@ +27,5 @@
>  // Default to empty directory links
>  let gDirectorySource = "data:application/json,{}";
>  
> +// The tests assume all 3 rows and all 3 columns of sites are shown, but the
> +// window may be too short to actually show three rows.  Resize it if necessary.

too short to actually show three rows -> too small to show everything
Attachment #8460512 - Flags: review?(adw) → review+
https://hg.mozilla.org/mozilla-central/rev/8b0f27654cc8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Iteration: --- → 34.1
QA Contact: cornel.ionce
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:34.0) Gecko/20100101 Firefox/34.0
Mozilla/5.0 (X11; Linux i686; rv:34.0) Gecko/20100101 Firefox/34.0

Verified fixed using latest Nightly, build ID: 20140727030204.
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
Depends on: 1044602
Depends on: 1048137
Depends on: 1044477
Depends on: 1048148
Depends on: 1046693
Depends on: 1045769, 1045751
Blocks: 1057602
Uplift has been managed in bug 1057602
Duplicate of this bug: 998058
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 i686; rv:33.0) Gecko/20100101 Firefox/33.0

Verified fixed on Windows 7 64bit, Mac OS X 10.8.5 and Ubuntu 14.04 32bit using latest Aurora, build ID: 20140828004002.
You need to log in before you can comment on or make changes to this bug.