834709 - [Transitions. UX] Implement "invoke an app" transition as specified.

Status: RESOLVED FIXED

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

Description

[Victoria Gerchinhoren [:vicky]]

Please refer to: 
http://buildingfirefoxos.com/transitions.html#invoke

https://www.dropbox.com/sh/ekzrrz7um4uj6y0/NVzqB1D4hx/03%20Transitions
TransitionsVXX.pdf (latest version)

Lack of Transitions affect the experience. Required for TEF build.
Transitions a key feature to give the user a clue of the architecture of the OS and how to navigate it.

Comment 1

[Cristian Rodriguez (:crdlc)]

Attachment: Video of the current animation implemented in this patch

Comment 2

[Cristian Rodriguez (:crdlc)]

I have the current implementation here [1]:

[1] https://github.com/crdlc/gaia/tree/bug-aia

It is based on bug 804270

Comment 3

[Cristian Rodriguez (:crdlc)]

We are trying to install a new homescreen with a lot of logs in order to know what happens when it fails and reverts icons. Thanks

Comment 4

[Cristian Rodriguez (:crdlc)]

sorry, the last message was wrong. I should write in other bug :(

Comment 5

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

The last animation ;)

Comment 6

[Victoria Gerchinhoren [:vicky]]

Looks great. 
Thanks Cristian!

Comment 7

[Cristian Rodriguez (:crdlc)]

please Vivien, do you need some help here to review the patch? tell me if you need something, thanks a lot

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Cristian Rodriguez (a remontar!) from comment #7)
> please Vivien, do you need some help here to review the patch? tell me if
> you need something, thanks a lot

Sorry I'm taking forever because I have seen some cases where the homescreen is zoomed with the previous patch and I would like to fix it before landing more changes to the window manager.

See bug 866643

Comment 9

[jachen]

Once this is reviewed, we can push this through triage according to Daniel's request.

Just keep me posted when its landed to master. 

Thanks!
Jaime

Comment 10

[Cristian Rodriguez (:crdlc)]

When Vivien review the bug 866643, he said that he will start reviewing this one, thanks

Comment 11

[Cristian Rodriguez (:crdlc)]

Hi Vivien, the bug 866643 was closed as worksforme due to your bug 866174 fixed the problem. I rebased this pull request with the master and then tell me when you can review it, thanks a lot!

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Cristian Rodriguez de la Cruz (:crdlc) from comment #11)
> Hi Vivien, the bug 866643 was closed as worksforme due to your bug 866174
> fixed the problem. I rebased this pull request with the master and then tell
> me when you can review it, thanks a lot!

Sorry for the incredibly long delay to review this PR. I keep procrastinating about it and push it back into my list of stuff to do. I will try to review it at the beginning of next week. Thanks for being always patient and nice Cristian, really appreciated :)

Comment 13

[Cristian Rodriguez (:crdlc)]

OK, I know that you are busy, don't worry. Please tell me when you start reviewing it because I've resolved conflicts a few times here :) but maybe for that moment the branch couldn't be merged, thanks a lot my friend.

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 739114 [details]
Patch v1

The app-ready mechanism seems a way to bypass all the existing code inside the window manager. I don't really like it.

Also I wish some of the code paths can be unified.

r- for those 2 points.

I like that you remove 3 classes!

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Also the new app that is opened is often not visible in the transition. Can we add at least a white background so something will be visible?

Comment 16

[Cristian Rodriguez (:crdlc)]

Comment on attachment 739114 [details]
Patch v1

I've implemented your suggestions, thanks for the feedback Vivien

Comment 17

[Cristian Rodriguez (:crdlc)]

https://github.com/mozilla-b2g/gaia/commit/b8161872fb3f01e6df7b7457994af3e378b7f8e1

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 739114 [details]
Patch v1

Could Vivien r+ the patch just for the record? There were no r= on the comment of the patch too...

Comment 19

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 739114 [details]
Patch v1

Post mortem r+ i believe...