Closed Bug 1255127 Opened 4 years ago Closed 4 years ago

Decrease height of bookmark folders to match remote client item height

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: liuche, Assigned: liuche)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Blocks: 1232868
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38981/diff/1-2/
Attachment #8728562 - Attachment description: MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=mcomella → MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha
Attachment #8728562 - Flags: review?(michael.l.comella) → review?(gkruglov)
Grisha, this is a tiny review (just dimension changes) that mcomella said he probably wouldn't get to for a while, so I thought I'd send it to you so you can learn a bit about reviewboard :)
Blocks: bookmark-folders
No longer blocks: 1232868
Depends on: 1232868
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

https://reviewboard.mozilla.org/r/38981/#review35985

page_group_height value for landscape mode is actually the same as page_row_height (from what you were changing away). should we be doing something else for landscape here? otherwise "decrease height" is a bit of a misnomer.
Attachment #8728562 - Flags: review?(gkruglov)
Attachment #8728562 - Flags: review?(gkruglov)
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38981/diff/2-3/
Looks like that page_group_height was added mistakenly to the wrong file in bug 1129171 - that line should have been added to values-large-land instead just values-land/ so that it would affect just large tablets in landscape mode, not all devices in landscape mode.

Since we have three kinds of height situations (one smaller height for interleaved headers in single-pane phone/portrait mode, one for the the side headers of the dual-pane landscape tablet mode, and one for the page rows which should remain the same in all situations), I added another dimen to clarify that.

Good catch, Grisha!
Attachment #8728562 - Flags: review?(gkruglov) → review+
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

https://reviewboard.mozilla.org/r/38981/#review36683

As far as I can tell, it's doing what you've intended!
https://hg.mozilla.org/mozilla-central/rev/d62999417a47
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

Approval Request Comment
[Feature/regressing bug #]: Home panels restyling
[User impact if declined]: Less differentiation between groups and links
[Describe test coverage new/current, TreeHerder]: local
[Risks and why]: low, updating dimensions of views, fixes a bug for landscape mode
[String/UUID change made/needed]: none
Attachment #8728562 - Flags: approval-mozilla-aurora?
Comment on attachment 8728562 [details]
MozReview Request: Bug 1255127 - Decrease height of bookmark folders to match remote client item height. r=grisha

Seems like a row risk fix, taking it. Aurora47+
Attachment #8728562 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flagging for QE verification around bookmark folder height for mid-Aurora sign offs.
Flags: qe-verify+
STR:
On a phone, or in portrait mode of a tablet (there should be a single column of items, not two), open Bookmarks home panel

Expected: Bookmark folders* should be shorter than the link/history items (and be the same height as the Device lines in the "Remote Tabs" panel). On a phone, rotating to landscape should not cause the height of the folder items to grow. (When clicking into a bookmark folder, the "back" item should also be the same height as the folders, not the history/link items.)

* In order to get bookmark folders, you need to have synced bookmarks with a Desktop Firefox that has bookmarks
Assignee: nobody → liuche
Hi,

I was not able to reproduce the issue.

Tested with:
Browser / Version: Firefox Mobile Nightly 63.0a1 (2018-08-19)
Operating System: Oneplus Two (Android 6.0.1)

Thank you!

Also, taking into consideration Comment 14, I will remove the qe-verify+ flag.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.