[Vertical Homescreen] Last divider is visible in normal mode sometimes

VERIFIED FIXED in 2.0 S5 (4july)

Status

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: crdlc, Assigned: crdlc)

Tracking

unspecified
2.0 S5 (4july)
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(b2g-v2.0 verified, b2g-v2.1 verified)

Details

(Whiteboard: ux-tracking, visual design, visual-tracking, [ft:systemsfe][systemsfe])

Attachments

(2 attachments)

The last divider is visible in normal mode:

* After adding a smart collection
* Sometimes existing from edit mode
* ....
Assignee: nobody → crdlc
Blocks: 1015336
Status: NEW → ASSIGNED
Whiteboard: [systemsfe]
Posted file Github pull request
Attachment #8444309 - Flags: review?(kgrandon)
Duplicate of this bug: 1028702
Blocks: vertical-home-next
No longer blocks: 1015336
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-]
Whiteboard: [systemsfe] → ux-tracking, visual design, visual-tracking, [ft:systemsfe][systemsfe]
Comment on attachment 8444309 [details]
Github pull request

Hmm, I really thought that we'd need to do this via javascript, but potentially we can still leverage last-of-type.

Chris - could you take a look and review this for me? Thanks!
Attachment #8444309 - Flags: review?(kgrandon) → review?(chrislord.net)
I do agree with you Kevin.
I think with this patch we fix a side effect of the problem.

Let me describe the issue that I saw on the phone:

When we start the phone or we restart verticalhome app, apps are added in the grid with a last divider at bottom as below:

A1 A2 A3
--------
A4 A5  
--------
A6
--------

Ax are apps and -- divider

After we switch to edit mode and we come back to the normal mode we have that:

A1 A2 A3
--------
A4 A5  
--------
A6
--------
X  X  X

Where X are placeholders

In my opinion we should fix the issue by removing the placeholders below the last divider in normal mode.
In normal mode we should be like the initial state if apps are not moved.
If the apps moved we should have a last divier with nothing below.
Flags: needinfo?(jsmith)
I'm not really the right person to ask here.
Flags: needinfo?(jsmith)
With this and bug 1028702, we have two different approaches. I believe Chris may also have another patch which accomplishes the same thing. We definitely need to find ways to avoid duplicating work in the future.

I was experiencing a regression in creating new groups in bug 1028702, but other people seemed to not be - so it could've just been my device/setup. The patch here does seem pretty simple and risk-free as well. Let's see what the review on this one comes back as.
With my patch dividers are another kind of DOM element so no problem if this is actually the last element in the children list or not. CSS will hide the last <section> element which is only the last divider although you have three placeholders like latest children
Flags: needinfo?(kgrandon)
Comment on attachment 8444309 [details]
Github pull request

This is a nice way to fix it :)
Attachment #8444309 - Flags: review?(chrislord.net) → review+
Merged in master:

https://github.com/crdlc/gaia/commit/7601815d1a14e0ebb13192de546d62a456ad2092
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8444309 [details]
Github pull request

This is needed for the vertical homescreen.
Attachment #8444309 - Flags: approval-gaia-v2.0?(bbajaj)
Flags: needinfo?(kgrandon)
Target Milestone: --- → 2.0 S5 (4july)
Attachment #8444309 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Going to block 989848 so we can track uplifts easier.
Blocks: vertical-homescreen
No longer blocks: vertical-home-next
 Hi Cristian Rodriguez;
There is some discription of this bug,I can get known that the behavior of this bug mainly.But I am not sure the detail reproduce steps.Would you like to provide the detail reproduce step for tester to verify this bug?Thanks.
Flags: needinfo?(crdlc)
Sometimes users enter in edit move and move some icons around but when they leave the edit move, the last divider for last group was visible and this behavior was wrong
Flags: needinfo?(crdlc)
This issue has been successfully verified on Flame 2.0:
Gaia-Rev        3aa0797c111a40e36f722722309668de3d469181
Gecko-Rev       93efc8b4155f0a4a50eaad19acbb95ec24139e63
Build-ID        20141204050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2

This issue has been successfully verified on Flame 2.1:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.