Closed Bug 1146146 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: New Tab Page, defect)

39 Branch
defect
Not set
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 39
Iteration:
39.3 - 30 Mar
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: alice0775, Assigned: emtwo)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

On 1280*1024 monitor,
3 rows on 2015-03-21 build, but 2 rows on 2015-03-22 build.
Flags: needinfo?(msamuel)
Summary: Number of rows of tile is too small → Number of rows of new tab tile is too small
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.
(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.
Flags: needinfo?(msamuel)
(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 ;)
(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.
(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.
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?
There's a fix in bug 1146249 that should tighten up some of the margins -- potentially allowing another row to appear.
kghim, there should be a build here with the patch from bug 1146249 applied:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/edilee@gmail.com-6c5947aadce7/try-macosx64/firefox-39.0a1.en-US.mac.dmg

Does that increase the number of rows shown for you?
Attached patch Reduce line count from 3 to 2 (obsolete) — Splinter Review
Waiting for the build for this change to test out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=645d183a8ee5
Attachment #8581752 - Attachment is obsolete: true
Flags: needinfo?(alice0775)
(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/edilee@gmail.com-
> afacfcd82776/try-win32/firefox-39.0a1.en-US.win32.zip
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/edilee@gmail.com-
> afacfcd82776/try-macosx64/firefox-39.0a1.en-US.mac.dmg
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/edilee@gmail.com-
> 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
Flags: needinfo?(alice0775)
Duplicate of this bug: 1147832
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?
(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.
Summary: Number of rows of new tab tile is too small → Maximize the number of rows of tiles by reducing the suggested explanation maximum line count to 2 instead of 3
Attachment #8581810 - Flags: review?(adw)
Attachment #8581810 - Flags: review?(adw) → review+
Assignee: nobody → msamuel
Points: --- → 1
https://hg.mozilla.org/integration/fx-team/rev/af3c990c5865
OS: Windows 7 → All
Hardware: x86_64 → All
Iteration: --- → 39.3 - 30 Mar
Flags: firefox-backlog+
Blocks: 1120311
https://hg.mozilla.org/mozilla-central/rev/af3c990c5865
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Blocks: 1148859
On a Nightly with this patch I'm still seeing only 1 row of tiles!
Guilherme, could you please share what your screen resolution, Nightly version, and operating system are to help us with debugging. Thank you.
Flags: needinfo?(guijoselito)
Attached image screenshot
1366x768 - 39.0a1 (2015-03-30) - Windows 7
Flags: needinfo?(guijoselito)
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?
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.
Flags: qe-verify+
QA Contact: cornel.ionce
When is "Show your top sites" checked, the bottom margin could be reduced back to 20px.
(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.
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.
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.
Status: RESOLVED → VERIFIED
Blocks: 1155443
No longer blocks: 1155443
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.
You need to log in before you can comment on or make changes to this bug.