[Dialer. UX] Apply correct transition to overlays.

RESOLVED FIXED

Status

Firefox OS
Gaia::Dialer
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicky, Assigned: gtorodelvalle)

Tracking

unspecified
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(b2g18 fixed)

Details

(Whiteboard: interaction, UX-P2, [TEF_REQ])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Transitions specs live here: 
https://www.dropbox.com/sh/0yq0akt8v3k6l40/APF8QfVTYV

Check page 25.
(Reporter)

Updated

5 years ago
Assignee: nobody → gtorodelvalle
Keywords: polish
Priority: -- → P3
Whiteboard: interaction
(Reporter)

Updated

5 years ago
Summary: [Dialer. UX] Apply correct transition to Airplane mode overlay. → [Dialer. UX] Apply correct transition to overlays.
Whiteboard: interaction → interaction, UX-P2, [TEF_REQ]
(Reporter)

Updated

5 years ago
Keywords: polish
(Reporter)

Comment 1

5 years ago
As per Josh's request, transition for overlays and prompts has changed to a fade in behaviour, see: http://buildingfirefoxos.com/transitions.html#prompts
Created attachment 716558 [details]
Associated PR.
Attachment #716558 - Flags: review?(alberto.pastor)
Attachment #716558 - Flags: review?(crdlc)
Attachment #716558 - Flags: review?(crdlc) → review+

Updated

5 years ago
Attachment #716558 - Flags: review?(alberto.pastor) → review?(igonzaleznicolas)
Attachment #716558 - Flags: review?(igonzaleznicolas) → review+
Thank you very much, guys, for your review! It was highly useful ;-) Merging it ;-)
Comment on attachment 716558 [details]
Associated PR.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bugs applies the new transition for overlays and prompts (http://buildingfirefoxos.com/transitions.html#prompts) in the Communications app.
User impact if declined: Very little.
Testing completed: Heavy testing in device including 5 people reviewing the patch.
Risk to taking this patch (and alternatives if risky): Low. Many eyes have tested it.
String or UUID changes made by this patch:
Additional info: https://github.com/gtorodelvalle/gaia/commit/e92d92de1839bd905f710da92cf2260725051837
15 changed files with 85 additions and 34 deletions.
Attachment #716558 - Flags: approval-gaia-v1?
I think this patch causes the very serious regression described in bug 846530.  I can reproduce that bug on gaia master today, but if I git revert e92d92 it goes away.

Marking that bug as blocking this one.
Blocks: 846530
Comment on attachment 716558 [details]
Associated PR.

Cancelling the approval-gaia-v1 request so this doesn't get uplifted before Germain is able to confirm or deny that it caused bug 846530.
Attachment #716558 - Flags: approval-gaia-v1?
Hi guys! Although I am still running some tests, it seems that the lazy loading of the DOM elements and the JS libraries are provoking this problem. In fact, I have just seen that "sometimes" the lazy loading of https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/confirm_dialog.js goes before the lazy loading of the DOM (please see  https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/index.html#L156 and https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L413), and, right now, the code in confirm_dialog.js is not ready for this. I am making it safe in front of this kind of lazy loading issues.
Hi guys, again! It seems we have found the problem, or a least a solution to it. I'll send a new PR and include Etienne as the reviewer. Thanks!
Created attachment 719973 [details]
Follow up PR.
Attachment #719973 - Flags: review?(etienne)
Attachment #719973 - Flags: review?(etienne) → review+
https://github.com/mozilla-b2g/gaia/commit/b80b3763b32d753030894eee2294d414d0fd4177
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 716558 [details]
Associated PR.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bugs applies the new transition for overlays and prompts (http://buildingfirefoxos.com/transitions.html#prompts) in the Communications app.
User impact if declined: Very little.
Testing completed: Heavy testing in device including 5 people reviewing the patch.
Risk to taking this patch (and alternatives if risky): Low. Many eyes have tested it.
String or UUID changes made by this patch:
Additional info: https://github.com/gtorodelvalle/gaia/commit/e92d92de1839bd905f710da92cf2260725051837
15 changed files with 85 additions and 34 deletions.
Attachment #716558 - Flags: approval-gaia-v1?
Comment on attachment 719973 [details]
Follow up PR.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This is a follow up to the previous PR since we noticed some misbehavior in the latest builds (latest 2-3 days). 
User impact if declined: The dialer becomes unusable. Must be accepted if https://github.com/mozilla-b2g/gaia/pull/8241 (the other PR included in this bug is also accepted).
Testing completed: Tested in the device since it is a visual change.
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch: None
Additional info: 1 changed file with 5 additions and 0 deletions.
Attachment #719973 - Flags: approval-gaia-v1?
Comment on attachment 716558 [details]
Associated PR.

Approving since this bring us closer to desired spec, has been on master for a couple of days, and is considered low risk.
Attachment #716558 - Flags: approval-gaia-v1? → approval-gaia-v1+

Updated

5 years ago
Attachment #719973 - Flags: approval-gaia-v1? → approval-gaia-v1+
I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 b80b3763b32d753030894eee2294d414d0fd4177
  <RESOLVE MERGE CONFLICTS>
  git commit
  git checkout v1-train
  git cherry-pick -x -m1 f95e978df1abdaf3c61f779ac04c15c6cf9af4ea
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(gtorodelvalle)
Blocks: 847866
Solving the conflicts in v1-train ;-)
Flags: needinfo?(gtorodelvalle)
Hi John, again ;-) Conflicts should be solved and successfully landed in v1-train :-) Would you be so kind to confirm it? Thanks!
Flags: needinfo?(jhford)
same here, please put the commit hash for v1-train, thanks !
status-b2g18: --- → fixed
Flags: needinfo?(gtorodelvalle)
Sorry guys! I had some issues around here ;-999

After cherry-picking: 
1. Commit f95e978df1abdaf3c61f779ac04c15c6cf9af4ea -> https://github.com/mozilla-b2g/gaia/commit/80bd09e404b9e266fc75d08abfede82556255770
2. Commit b80b3763b32d753030894eee2294d414d0fd4177-> https://github.com/mozilla-b2g/gaia/commit/076df0443deb651a36586de884f028c010f54ef4
Flags: needinfo?(gtorodelvalle)
Flags: needinfo?(jhford)
You need to log in before you can comment on or make changes to this bug.