Closed Bug 1054716 Opened 10 years ago Closed 10 years ago

[Soft Home Button] Marketplace toast covers up software home button

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.1 verified)

VERIFIED FIXED
2.1 S4 (12sep)
Tracking Status
b2g-v2.1 --- verified

People

(Reporter: mikehenrty, Assigned: Eli)

References

Details

(Whiteboard: [systemsfe])

Attachments

(2 files)

Attached image toasty.png
STR:

1.) Enable soft home button
2.) Open marketplace
3.) Install app

Actual Results:
App installed toast notification covers up soft home button

Expected Results:
Soft home button should always render on top of app content.
Assignee: nobody → eperelman
Status: NEW → ASSIGNED
Attachment #8475499 - Flags: review?(etienne)
Attachment #8475499 - Flags: review?(alive)
Attachment #8475499 - Flags: feedback?(mhenretty)
Comment on attachment 8475499 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23052

I'm no expert here, but seems reasonable to me. I would say you don't even need to put the bug # in your comments, the explanation should be enough.
Attachment #8475499 - Flags: feedback?(mhenretty) → feedback+
Comment on attachment 8475499 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23052

Small comment on github.
Can you explain why you need the |zindex.css| change?
Attachment #8475499 - Flags: review?(etienne)
Attachment #8475499 - Flags: review?(alive)
@etienne Sure! So the original value of the z-index of the software home button is 4094. The system notification banners have a z-index of 65536. While I implemented a change to shift the location of the notification to account for the height of the software home button, this element also has an opening and closing animation which slide and fade on top of the software home button. By adding giving the software home button a higher z-index priority, we can keep the notifications from overlaying the software home button not only during its primary display, but also during its transitioning animation.
Keywords: checkin-needed
Doesn't have r+ yet AFAICT.
Keywords: checkin-needed
Attachment #8475499 - Flags: review?(etienne)
Comment on attachment 8475499 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23052

Thanks, the explanation makes perfect sense, we definitely don't want the banner to overlay the software home button during the transition.

But the current approach breaks the utility tray you can't close it anymore since it's behind the software home button :)

It's tricky because I think the banner should go on top of the utility tray and below the softhome but the utility tray should go on top of the soft home.

We can:
- check with UX if we can compromise on the "banner being on top of the utility tray", and make it part of the |Level 7: dialog module|, just making sure they go on top of dialogs

- get creative with the transition ie. transitioning a clip property on top of the opacity+translate we already have, it might just work.

Also, you are making a solid long term fix, not a workaround so feel free to remove the bug references in your comments!
Attachment #8475499 - Flags: review?(etienne) → review-
Depends on: 1056907
Etienne, since bug 1047115 is solving the issue of the utility tray overlapping the SHB, do we need to make any changes to this once that lands?
Depends on: 1047115
Flags: needinfo?(etienne)
Comment on attachment 8475499 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23052

Looking, let's just make sure the first part lands and we give it a manual check before landing.
Attachment #8475499 - Flags: review- → review+
Flags: needinfo?(etienne)
Absolutely. I will not land this before that is resolved, and I will spot check this to make sure the fix is compatible with that patch.
Per IRC with Eli, this work will be tracked in bug 1048620.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
I misspoke on IRC, this bug isn't a duplicate of bug 1048620.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
I have just tested the patch locally after the landing of bug 1048620 and can confirm that the utility tray is no longer overlapped by the SHB and the notification works as expected.
(In reply to :Eli Perelman from comment #12)
> I have just tested the patch locally after the landing of bug 1048620 and
> can confirm that the utility tray is no longer overlapped by the SHB and the
> notification works as expected.

The Marketplace toast notification works as expected? Can we close this then?
Flags: needinfo?(eperelman)
I am waiting to land until try is green:

https://tbpl.mozilla.org/?rev=9c07adc884a347d5f7e00f79c874da2abb194fac&tree=Gaia-Try
Flags: needinfo?(eperelman)
Sorry :mhenretty, I think I misunderstood your question. I meant that this patch will fix the toast notifications without causing an overlap with the utility tray since that was resolved in another bug.
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/27fd427b459d35e9c590f9d33dd368f1129a33f3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S4 (12sep)
Integration test added in bug 1077579.
Flags: in-testsuite+
This issue is verified fixed on Flame 2.1.

Result: The notification appears above the SHB.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031001201
Gaia: f89c7b12c36572262c9ea76058694a139b1a8634
Gecko: 50d48f8a04c7
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: