Remove margins on default home screen panels

RESOLVED FIXED in Firefox 35

Status

()

Firefox for Android
Awesomescreen
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: wesj, Assigned: rnewman)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Firefox 35
All
Android
regression
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox32 unaffected, firefox33 unaffected, firefox34 ?, firefox35 fixed, fennec35+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Our built in panels have margins on the left and right on tablets. The new hub panels don't. It makes lists look kinda silly. We should use the same style we use for our built in panels.

Updated

4 years ago
Blocks: 941312
Component: General → Awesomescreen
Flags: needinfo?(alam)
Hey guys, I noticed this too and have it in my head but I'm going to have to push forward with some other bugs before I can get to this. I'm also going to leave the NI on for that purpose :)
Our default panels should definitely not have those margins left and right (looking at the History panel on a N7 vert.).

I'd love to do a design pass at these panels altogether. After I get to work on this Sync Panel for Nick in Bug 1014994, I would probably have a better framework for scaling these panels across mobile and tablets.

Maybe we can start by extending those to the ends like our newer panels?
Flags: needinfo?(alam)
(Assignee)

Updated

3 years ago
status-firefox32: --- → ?
status-firefox33: --- → ?
status-firefox34: --- → ?
status-firefox35: --- → affected
OS: Linux → Android
Hardware: x86_64 → All
Summary: Hub panels take up full screen width on tablets → Tablet: remove margins on default home screen panels
Version: unspecified → Trunk

Updated

3 years ago
Blocks: 1063058
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1065125
(Assignee)

Updated

3 years ago
Summary: Tablet: remove margins on default home screen panels → Remove margins on default home screen panels
(Assignee)

Updated

3 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Comment 4

3 years ago
Created attachment 8486817 [details] [diff] [review]
Remove margins on default home screen panels. v1

As far as I can tell this bug only applies to History.

Tested this on modern phone, and small and large tablets.

ckitching: could you test this on your S2?
Attachment #8486817 - Flags: review?(wjohnston)
This fixes it for me.

Comment 6

3 years ago
This is probably more fallout from bug 1004850 (similar to bug 1062632).

So hopefully that means this isn't a problem on release, although wesj filed this bug before bug 1004850 landed...

Comment 7

3 years ago
I just saw margins appears on my recent tabs panel on a phone on Nightly (but not on Aurora). Have there been other recent changes here that could have affected this? Maybe my patch for bug 1062632 did something I didn't expect?

In any case, I just tested this patch and confirmed it fixes that problem for me. So +1 :)

(We should probably rename the home_history_list layout, since it's also used in the recent tabs panel, I'll file a separate bug.)
(Assignee)

Comment 8

3 years ago
(In reply to :Margaret Leibovic from comment #7)
> Have there been other recent changes here that could
> have affected this?

Yeah, Bug 1044334. We were deep in undefined behavior land before, so this bug is about making sure the real behavior is what we want :)
(Reporter)

Comment 9

3 years ago
Comment on attachment 8486817 [details] [diff] [review]
Remove margins on default home screen panels. v1

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

This makes history no longer match other panels when in landscape on xlarge devices, for example bookmarks [1]. Do we want to remove the padding on other panels as well? Do we want to keep the margins for this history panel on large tablets? It feels silly to me having it stretch edge to edge (which is why I originally filed this bug, although its been re-purposed since then).

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values-large-land-v11/styles.xml#27
(Assignee)

Comment 10

3 years ago
Discussed on IRC: prior to nailing bug 1063058, we'll just kill padding on bookmarks and history.

Will post updated patch and screenshots.
(Assignee)

Comment 11

3 years ago
Created attachment 8488927 [details] [diff] [review]
Remove margins on default home screen panels. v2

This:

* Lifts stuff out of the bookmark list styles into Top Sites where appropriate to match inheritance lookups.
* Removes padding for the bookmarks and history styles.
* Ensures that the history panel has a top divider line.

Verifies that appearance matches Nightly-with-expected-changes. Verified that visual jank during port/land rotation is no worse than Nightly.

Verified on xlarge-v11; gonna test on phone now.
Attachment #8488927 - Flags: review?(wjohnston)
(Assignee)

Updated

3 years ago
Attachment #8486817 - Attachment is obsolete: true
Attachment #8486817 - Flags: review?(wjohnston)
(Assignee)

Comment 12

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=b7a3ef15334b
Recent regression
tracking-fennec: --- → ?
status-firefox32: ? → unaffected
status-firefox33: ? → unaffected
Keywords: regression
(Reporter)

Comment 14

3 years ago
Comment on attachment 8488927 [details] [diff] [review]
Remove margins on default home screen panels. v2

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

Thanks.
Attachment #8488927 - Flags: review?(wjohnston) → review+
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/e016c34c4d52
Backed out for robocop permafail.
https://hg.mozilla.org/integration/fx-team/rev/37fe74353dce

https://tbpl.mozilla.org/php/getParsedLog.php?id=48114117&tree=Fx-Team
https://hg.mozilla.org/mozilla-central/rev/e016c34c4d52
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(Assignee)

Comment 18

3 years ago
wat
(Assignee)

Comment 19

3 years ago
Created attachment 8489775 [details] [diff] [review]
Follow-up: fix test with incorrect assumptions. v1

Tested locally on Nexus 10. Presumably the forced top divider that we removed counts as a view. Unexpected perf improvement, too!
Attachment #8489775 - Flags: review?(mark.finkle)
Attachment #8489775 - Flags: review?(mark.finkle) → review+
(In reply to Wes Kocher (:KWierso) from comment #17)
> https://hg.mozilla.org/mozilla-central/rev/e016c34c4d52

This got merged to m-c minus the backout. Sorry for the confusion.
Status: RESOLVED → REOPENED
Flags: needinfo?(kwierso)
Resolution: FIXED → ---
Target Milestone: Firefox 35 → ---
(Assignee)

Comment 21

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/de389f79f43e
Sorry, didn't notice the permafail when I did the merge.
Flags: needinfo?(kwierso)
https://hg.mozilla.org/mozilla-central/rev/de389f79f43e
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
tracking-fennec: ? → 35+
status-firefox35: affected → fixed
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.