1146146 - Maximize the number of rows of tiles by reducing the suggested explanation maximum line count to 2 instead of 3

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Alice0775 White]

On 1280*1024 monitor,
3 rows on 2015-03-21 build, but 2 rows on 2015-03-22 build.

Comment 1

[Semtex]

It is more like after landing this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1126188 Nightly stops respect User settings for rows and columns.

Comment 2

[Ed Lee :Mardak]

(In reply to Semtex from comment #1)
> Nightly stops respect User settings for rows and columns.
The setting/pref has been a maximum value as opposed to an exact number of rows/columns to show. So 3 rows and 5 columns might show up as 1 row and 2 columns on smaller screens, and on very large screens, no more than 3x5 tiles.

This change for more whitespace around tiles is intended as designed and implemented.

emtwo, you did mention in bug 1126188 comment 12 that potentially we could conditionally have the extra whitespace only if a suggested tile is shown. This could be distracting/confusing if subsequent openings of the new tab page has 2 rows and other times 3 rows.

Technically, this may be tricky to do given how rows are hidden/shown with some calculation of the grid space.

Comment 3

[Semtex]

(In reply to Ed Lee :Mardak from comment #2)
> (In reply to Semtex from comment #1)
> > Nightly stops respect User settings for rows and columns.
> The setting/pref has been a maximum value as opposed to an exact number of
> rows/columns to show. So 3 rows and 5 columns might show up as 1 row and 2
> columns on smaller screens, and on very large screens, no more than 3x5
> tiles.
But I think on FHD monitors new tab page looks very bad without respecting user settings, see screen:

http://wstaw.org/m/2015/03/22/2.png

I suggest to add pref to disable this new future in about:config and let Users decide what they want, I use only pinned sites, I think not only I ;)

Comment 4

[Ed Lee :Mardak]

(In reply to Kevin Ghim from email)
> move up search box
The the space above the search box automatically scales based on how much whitespace is below all the tiles. It's only taking up that much space on your screen because there's so much whitespace below. If you shrink the height, you can see how little space it can take up. There could be some whitespace we can shafe off here.

> only allow Suggested Tiles to appear on the bottom row: spacing for top row
> remaining the same as pre-Suggested Tiles
This makes it much more difficult to place as the number of columns isn't fixed. Some people can fit all 5, many might only fit 3.

> Dynamically adjust spacing depending on the presence of Suggested Tiles
This could work, but in the case of when suggested tiles are shown, you would still be left with 1 row.

Comment 5

[Ed Lee :Mardak]

(In reply to Ed Lee :Mardak from comment #4)
> There could be some whitespace we can shafe off here.
kghim, could you try running this code from your new tab page:

1) cmd-alt-K
2) paste in this code:
$("#newtab-search-form").style.margin = 0
3) hit enter
4) close the console (click the x on the right or cmd-alt-I)

Although this anchors the search box really close to the top of the tiles.

Comment 6

[Ed Lee :Mardak]

emtwo, the space for explanation text always allocates for 3 lines? The english text doesn't use the full line although that depends on the subdomain being shown. I wonder if we can drop it down to 2 lines if that saves any?

Comment 7

[Ed Lee :Mardak]

There's a fix in bug 1146249 that should tighten up some of the margins -- potentially allowing another row to appear.

Comment 8

[Ed Lee :Mardak]

kghim, there should be a build here with the patch from bug 1146249 applied:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6c5947aadce7/try-macosx64/firefox-39.0a1.en-US.mac.dmg

Does that increase the number of rows shown for you?

Comment 9

[Marina Samuel [:emtwo]]

Attachment: Reduce line count from 3 to 2

Waiting for the build for this change to test out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=645d183a8ee5

Comment 10

[Marina Samuel [:emtwo]]

Attachment: v2: Reduce line count from 3 to 2

Needed to adjust margin-bottom as well:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eded5950666

Comment 11

[Ed Lee :Mardak]

Here's builds with emtwo's patch plus bug 1146249:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-afacfcd82776/try-win32/firefox-39.0a1.en-US.win32.zip
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-afacfcd82776/try-macosx64/firefox-39.0a1.en-US.mac.dmg
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-afacfcd82776/try-linux64/firefox-39.0a1.en-US.linux-x86_64.tar.bz2

Alice0775, does this fit 3 rows for you on 1280x1024?

Comment 12

[Alice0775 White]

(In reply to Ed Lee :Mardak from comment #11)
> Here's builds with emtwo's patch plus bug 1146249:
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> afacfcd82776/try-win32/firefox-39.0a1.en-US.win32.zip
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> afacfcd82776/try-macosx64/firefox-39.0a1.en-US.mac.dmg
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> afacfcd82776/try-linux64/firefox-39.0a1.en-US.linux-x86_64.tar.bz2
> 
> Alice0775, does this fit 3 rows for you on 1280x1024?

The try server did not foxid the 2 rows problem for me.

Maximized browser Windows7 1280x1024 monitor 
                                              trybuild Firefox37
No titlebar, No Menubar, No Bookmarks toolbar: 3rows    3rows
No titlebar, Menubar, No Bookmarks toolbar   : 2rows    3rows
No titlebar, Menubar, Bookmarks toolbar      : 2rows    3rows
Titlebar, Menubar, Bookmarks toolbar         : 2rows    3rows


Maximized browser Ubuntu14.04 GNOME classic 1280x1024 monitor
                                            trybuild  Firefox37
titlebar, No Menubar, No Bookmarks toolbar:  2rows    3rows
titlebar, Menubar, No Bookmarks toolbar   :  2rows    3rows
titlebar, Menubar, Bookmarks toolbar      :  2rows    3rows
Titlebar, Menubar, Bookmarks toolbar      :  2rows    3rows

Comment 14

[Marina Samuel [:emtwo]]

Ed, this fit 3 rows for me on my Mac when I set the resolution to 1680x1050.

I think we are fine with having a decrease in row count for now, with discussions in bug 1147218 or others on potential ways to fix spacing issues?

Does it sound fair to relabel this bug to "reduce number of rows for suggested text from 3 to 2" since we want to do that anyway and not worry about number of tile rows here?

Comment 15

[Ed Lee :Mardak]

(In reply to Marina Samuel [:emtwo] from comment #14)
> Ed, this fit 3 rows for me on my Mac when I set the resolution to 1680x1050.
Are you referring to the 2-line text patch instead of 3-line? If so, I think it'll be fine to take the patch as 3 is nice to have if we can squeeze it in.

All the designs from dcrobot have mocked it with 2 rows, so if we have 2 rows, I believe that's fine. Getting just 1 row would not be as okay.

Comment 16

[Ed Lee :Mardak]

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

Comment 17

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/mozilla-central/rev/af3c990c5865

Comment 18

[Guilherme Lima]

On a Nightly with this patch I'm still seeing only 1 row of tiles!

Comment 19

[Marina Samuel [:emtwo]]

Guilherme, could you please share what your screen resolution, Nightly version, and operating system are to help us with debugging. Thank you.

Comment 20

[Guilherme Lima]

Attachment: screenshot

1366x768 - 39.0a1 (2015-03-30) - Windows 7

Comment 21

[Ed Lee :Mardak]

Wow. You seem to have 633px height for page content. It looks like the minimum height to fit 2 rows is 636px. Just checking, do you have the taskbar on the bottom/top of your screen? Does the 2nd row appear if you temporarily hide your bookmark toolbar?

Comment 22

[Guilherme Lima]

Taskbar on the bottom. If I hide the bookmark toolbar, yes, there is 2 rows. If I zoom out 1 level (clicking ctrl+'-' just one time) it also makes the 2nd row appear.

Comment 23

[UK92]

When is "Show your top sites" checked, the bottom margin could be reduced back to 20px.

Comment 24

[Ed Lee :Mardak]

(In reply to UK92 from comment #23)
> When is "Show your top sites" checked, the bottom margin could be reduced
> back to 20px.
That's a pretty clever solution. I filed bug 1149689 with maybe a more general approach.

Comment 25

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-beta/rev/5fd426f495ff

Comment 26

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

Confirming the fix on Windows 7 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.9.5 using Firefox Aurora (build ID: 20150405004004) and on Latest Nightly (build ID: 20150405030238).

I've filled bug 1151433 - specific for HiDPI screens, where only a single row of tiles is visible.

Comment 27

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

Same behavior now on Firefox 38 beta 2, build ID: 20150406174117
Tested on Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.

Comment 28

[Guilherme Lima]

FYI, one of the bugs fixed on the May 10th Nightly (bug 1158853, bug 1138817, bug 1158859) also fixed my problem of seeing only 1 row. I now see 2 rows like a few months ago.