Update styling of newtab tiles to enhanced tiles spec

VERIFIED FIXED in Firefox 33

Status

()

VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

Trunk
Firefox 34
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox33 verified, firefox34 verified)

Details

Attachments

(2 attachments, 13 obsolete attachments)

(Assignee)

Description

5 years ago
Update tile-related styling from bug 1030832: (unhover, hover, pinned): resize tiles to 290x180, center text, radius, shadow, fading/opacity
Flags: firefox-backlog+
(Assignee)

Comment 1

5 years ago
Created attachment 8452905 [details] [diff] [review]
v1
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #8452905 - Flags: review?(adw)
(Assignee)

Comment 2

5 years ago
Created attachment 8452906 [details]
v1 screenshot
(Assignee)

Comment 3

5 years ago
Created attachment 8452916 [details] [diff] [review]
v1.1

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)
(Assignee)

Updated

5 years ago
Depends on: 1036280
QA Whiteboard: [qa+]
Iteration: 33.3 → ---
(Assignee)

Comment 4

5 years ago
Created attachment 8453190 [details] [diff] [review]
v1.2

Added the test change for pinned attr check
Attachment #8452916 - Attachment is obsolete: true
Attachment #8452916 - Flags: review?(adw)
Attachment #8453190 - Flags: review?(adw)
(Assignee)

Comment 5

5 years ago
dcrobot says the page's background texture needs to be updated and search button font size reduced
(Assignee)

Comment 6

5 years ago
Created attachment 8454068 [details]
Enhanced_Tiles_Icons.svg
(Assignee)

Comment 7

5 years ago
Created attachment 8454086 [details]
Enhanced_Tiles_Icons.svg

dcrobot's updated svg
Attachment #8454068 - Attachment is obsolete: true

Comment 8

5 years ago
Created attachment 8454088 [details]
SVG Icons
(Assignee)

Comment 9

5 years ago
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?
(Assignee)

Comment 10

5 years ago
(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
(Assignee)

Comment 11

5 years ago
Created attachment 8454324 [details] [diff] [review]
v2

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)
(Assignee)

Comment 12

5 years ago
Created attachment 8454325 [details]
v2 screenshot
(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?

Comment 16

5 years ago
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.

Comment 17

5 years ago
Created attachment 8454490 [details]
Enhanced_Tiles_Icons.svg (CORRECT VERSION)

The base circles should all be 28 x 28 pixels now.
(Assignee)

Comment 18

5 years ago
(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.
(Assignee)

Comment 19

5 years ago
Created attachment 8454562 [details]
Enhanced_Tiles_Icons-edited.svg

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
(Assignee)

Comment 20

5 years ago
(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?

Comment 21

5 years ago
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

Comment 22

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 1036299
(Assignee)

Comment 23

5 years ago
Created attachment 8458450 [details] [diff] [review]
v2.1
Attachment #8454324 - Attachment is obsolete: true
Attachment #8458450 - Flags: review?(adw)
(Assignee)

Comment 24

5 years ago
Created attachment 8458451 [details]
v2.1 screenshot
Attachment #8454325 - Attachment is obsolete: true
Attachment #8454562 - Attachment is obsolete: true
(Assignee)

Comment 25

5 years ago
Created attachment 8458768 [details] [diff] [review]
v2.2

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)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 27

5 years ago
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
(Assignee)

Comment 28

5 years ago
Created attachment 8460512 [details] [diff] [review]
v3

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
Last Resolved: 5 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

Updated

5 years ago
Depends on: 1048137
Depends on: 1044477
(Assignee)

Updated

5 years ago
Depends on: 1048148
(Assignee)

Updated

5 years ago
Depends on: 1046693
(Assignee)

Updated

5 years ago
Depends on: 1045769, 1045751
(Assignee)

Updated

5 years ago
Blocks: 1057602
Uplift has been managed in bug 1057602
status-firefox33: --- → fixed
status-firefox34: --- → fixed
status-firefox34: fixed → verified
(Assignee)

Updated

5 years ago
status-firefox33: fixed → affected
(Assignee)

Updated

4 years ago
status-firefox33: affected → fixed

Updated

4 years ago
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.
status-firefox33: fixed → verified
You need to log in before you can comment on or make changes to this bug.