1135123 - Add transition to private window overlay in task manager / tab view

Status: VERIFIED FIXED

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

Description

[Sam Foster [:sfoster] (he/him)]

Follow-up from bug 1133826, :epang asks "When accessing the task manager/tab view is it possible to fade in the overlay?  At the moment it instantly switches on which looks a little janky"

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

Comment 2

[Chris Lord [:cwiiis]]

Comment on attachment 8583125 [details] [review]
[gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

Sam isn't coming up in suggested reviewers, so tagging Alive too - whoever gets there first, feel free to remove the other as appropriate (although I guess autolander is going to balk without a suggested reviewer...)

Anyway, this patch adds a transition for the private card overlay. I've also added a transition out of the cards view to maintain symmetry, and I shortened the fade-in slightly, as 1s felt a bit excessive.

ui-review?epang for feedback on whether it's better like this, and if there are any small tweaks (there is a tiny amount of jank that I think will be hard to get rid of without it becoming a larger change, so tiny changes = small timing changes and not much else)

Comment 3

[Chris Lord [:cwiiis]]

So the jank is because of bug 771367, which I wasn't aware of until now :( This explains a lot...

I'd still like the review, but I'm very tempted to augment this and remove the use of pseudo-elements...

Comment 4

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8583125 [details] [review]
[gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

Looks like there's a unit test failure: TEST-UNEXPECTED-FAIL | null | [system-test/unit/task_manager_test.js] system/TaskManager > tapping on an app > displays the new app before dismissing the task manager. Presumably because that state change is now async in all cases with the transition. 

I'm not a system module peer which is why I wasnt coming up on the list. Etienne has been doing most of my recent reviews here as he and Vivien worked on getting the horizontal task strip working ~v2.1, but he or Alive can review this. 

The exit animation in particular seems really ponderous (not changed in your patch) I don't know what happened there - the .appWindow.from-cardview rule says it should be 0.3s, but it seems longer. 

I used the :after pseudo-element for the private window bits for no particularly good reason except I didnt want to modify the template and carry those extra elements when the normal case was non-private and they would never be seen or used. But having them transition seems like reason enough to change that implementation.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8583125 [details] [review]
[gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

Since Sam thinks this is good let's take it. (with green try)

Comment 6

[Chris Lord [:cwiiis]]

Fixed unit test and moved the private overlay to its own element to remove jank. Much nicer :) Will carry r=alive, but will wait on ui-review=epang before checking in.

Comment 7

[Eric Pang [:epang] UX]

Comment on attachment 8583125 [details] [review]
[gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

This already looks 100x better!  It's so much smoother, any less jank from resolving the other bug will be a bonus :).  Thanks for working on this Chris!!

Comment 8

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/b56548c3b5df2a1634b8b4f0628ff100469c83ea

Comment 9

[Chris Lord [:cwiiis]]

Comment on attachment 8583125 [details] [review]
[gaia] Cwiiis:bug1135123-cardsview-private-transition > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Janky transition when opening card view with private windows opened (also, very slightly janky transition when picking an app from card view, regardless of private windows)
[Testing completed]: Manual testing, also covered by automated testing
[Risk to taking this patch] (and alternatives if risky): Low risk.
[String changes made]: None

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.2 uplift.

Comment 11

[Chris Lord [:cwiiis]]

This requires that bug 1133826 land in 2.2 first - That bug is a 2.2 feature, so seems a bit weird it hasn't been uplifted yet... I'll needinfo on that bug.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/19436397211b615a9d51fb6aa77eb6f81b6cc67f

Comment 13

[Jamie Charlton (:Jamie_)]

verified fixed
3.0
Device: Flame 3.0
Build ID: 20150501010203
Gaia: 759a1f935a6a81c32ad66e39a6353b334dfa4f91
Gecko: 7723b15ea695
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 40.0a1 (3.0)
Firmware Version: v18D_nightly_v2
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

2.2
Device: Flame 2.2
Build ID: 20150501162500
Gaia: 8d14361337e608c8cdf165ea5034db5eda23b618
Gecko: 8a284a9251e8
Gonk: I couldnt pull the gonk.  Did you shallow Flash again?
Version: 37.0 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0