Closed
Bug 1146146
Opened 10 years ago
Closed 10 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)
Tracking
()
People
(Reporter: alice0775, Assigned: emtwo)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
1005 bytes,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
223.91 KB,
image/png
|
Details |
On 1280*1024 monitor,
3 rows on 2015-03-21 build, but 2 rows on 2015-03-22 build.
Flags: needinfo?(msamuel)
Reporter | ||
Updated•10 years ago
|
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.
Comment 2•10 years ago
|
||
(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 ;)
Comment 4•10 years ago
|
||
(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•10 years ago
|
||
(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•10 years ago
|
||
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•10 years ago
|
||
There's a fix in bug 1146249 that should tighten up some of the margins -- potentially allowing another row to appear.
Comment 8•10 years ago
|
||
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?
Assignee | ||
Comment 9•10 years ago
|
||
Waiting for the build for this change to test out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=645d183a8ee5
Assignee | ||
Comment 10•10 years ago
|
||
Needed to adjust margin-bottom as well:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eded5950666
Assignee | ||
Updated•10 years ago
|
Attachment #8581752 -
Attachment is obsolete: true
Comment 11•10 years ago
|
||
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?
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(alice0775)
Reporter | ||
Comment 12•10 years ago
|
||
(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)
Assignee | ||
Comment 14•10 years ago
|
||
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•10 years ago
|
||
(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.
Assignee | ||
Updated•10 years ago
|
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
Assignee | ||
Updated•10 years ago
|
Attachment #8581810 -
Flags: review?(adw)
Updated•10 years ago
|
Attachment #8581810 -
Flags: review?(adw) → review+
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → msamuel
Assignee | ||
Updated•10 years ago
|
Points: --- → 1
Comment 16•10 years ago
|
||
OS: Windows 7 → All
Hardware: x86_64 → All
Updated•10 years ago
|
Iteration: --- → 39.3 - 30 Mar
Flags: firefox-backlog+
Comment 17•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment 18•10 years ago
|
||
On a Nightly with this patch I'm still seeing only 1 row of tiles!
Assignee | ||
Comment 19•10 years ago
|
||
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)
Comment 21•10 years ago
|
||
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•10 years ago
|
||
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.
Updated•10 years ago
|
Flags: qe-verify+
QA Contact: cornel.ionce
Comment 23•10 years ago
|
||
When is "Show your top sites" checked, the bottom margin could be reduced back to 20px.
Comment 24•10 years ago
|
||
(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•10 years ago
|
||
status-firefox38:
--- → fixed
Comment 26•10 years ago
|
||
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•10 years ago
|
||
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
Comment 28•10 years ago
|
||
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.
Description
•