847241 - Make switching to an app feel faster

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

In the current code, there is a delay between the moment the user tap on a card, and the moment the card is appearing. Also, the user see the animation of removing the card switcher before showing the app.

This is not nice.

Comment 1

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Attachment: patch v1

(see also PR https://github.com/mozilla-b2g/gaia/pull/8422; the PR also has the patch to Bug 846768)

- use the event appopen instead of appwillopen to hide the card switcher; this
  makes the card switcher disappear after showing the app, and thus this is
  nicer.
- disable the transition when we want to remove "immediately" the card switcher
- add some unit tests about the card switcher showing or hiding
---
 apps/system/js/cards_view.js                     |   52 +++++++++-------
 apps/system/js/window_manager.js                 |    3 +-
 apps/system/style/cards_view/cards_view.css      |    6 +-
 apps/system/test/unit/cards_view_test.js         |   72 +++++++++++++++++++++-
 apps/system/test/unit/mock_lock_screen.js        |    3 +
 apps/system/test/unit/mock_popup_manager.js      |    5 ++
 apps/system/test/unit/mock_sleep_menu.js         |    3 +-
 apps/system/test/unit/mock_trusted_ui_manager.js |    2 +
 apps/system/test/unit/mock_window_manager.js     |    6 +-
 9 files changed, 123 insertions(+), 29 deletions(-)
 create mode 100644 apps/system/test/unit/mock_lock_screen.js
 create mode 100644 apps/system/test/unit/mock_popup_manager.js

The patch may need to be applied on top of Bug 846768's patch, I didn't try on plain master.

Comment 2

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

master: https://github.com/mozilla-b2g/gaia/commit/53f9e4f70911d3e083d25730490263a60041f37d

Comment 3

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Attachment: patch v2

carrying r=timdream

This is the patch that landed with the following changes to the previous patch:

- put back the visibility in the CSS transition; otherwise the cards view is hidden as soon as the class is changed and we don't see the transition. I changed the transition easing to step-end though, as visibility doesn't have a numeric value anyway.

- I check for (cardDiff == 0) before calculating the absolute value.

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Comment on attachment 720587 [details] [diff] [review]
patch v2

I'd love this to land on v1-train.

risks: low, it has (small) unit tests to at least test that the card switcher is showing and hiding. So the only risks are on the transitions, but I tested this well.

Advantages: the app switching occurs as soon as the user taps the card, instead of after the card-hiding transition. This feels a lot better.

Tested: yes of course

l10n changes: none

Comment 5

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

asking for tef? too as QC might want this.

Comment 6

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Hum the cardDiff change should have gone in the patch in Bug 846768 and thus introduce a dependency.

So, uplifters, please land the patch in Bug 846768 before this one.

Comment 7

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

My last minute changes introduced a very small regression.

So here is a follow-up to revert one of the (useless) changes :
master: https://github.com/mozilla-b2g/gaia/commit/cb368d63abd8505b7afa9f4c1bb6a5979bb5f537

Comment 8

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Attachment: follow-up patch

Comment 9

[Jason Smith [:jsmith]]

Reopen for bustage patch fix.

Comment 10

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

jason, I already landed it, should this stay opened ?

Comment 11

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> jason, I already landed it, should this stay opened ?

Oh, well, in that case, let's close again. Didn't realize it was already landed.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking, go ahead with uplift to v1.1

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

this was fixed as part of bug 801306 on v1-train and landed there in commit:

8cf4fede0811843b14217caa0619982b7aae49b0

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

follow up landed as master: cb368d63abd8505b7afa9f4c1bb6a5979bb5f537

Comment 15

[Michael Vines [:m1] [:evilmachines]]

(not a core perf usecase we are focusing on for v1.0.1)

Comment 16

[Chris Lord [:cwiiis]]

Contrary to comment #13, I don't think that commit fixed this issue (building current v1-trains still exhibits the issue) - cherry-picking the commit from master does solve this issue though. I think this still needs uplifting to v1-train.

Comment 17

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Comment on attachment 720587 [details] [diff] [review]
patch v2

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 #): -
User impact if declined: some transitions are done faster, making the switch to an app faster
Testing completed: yes, this is in master for 2 months
Risk to taking this patch (and alternatives if risky): very low, and there are unit tests
String or UUID changes made by this patch: none

note: this was supposed to be uplifted a long time ago but it apparently failed.

Comment 18

[Alex Keybl [:akeybl]]

Comment on attachment 720587 [details] [diff] [review]
patch v2

Approving - we'll just back out if it regresses.

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Julien, this patch does not apply cleanly to v1-train.  Could you please take a look at the conflicts?

Comment 20

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

v1-train: 53f9e4f70911d3e083d25730490263a60041f37d -> 790b7c32953ffa5e3078d868b3e8789800e19f77
v1-train: cb368d63abd8505b7afa9f4c1bb6a5979bb5f537 -> fe7d06f44b19e11aa20c45c5df2720bf27d84dd7

I see some visual artefacts when I display the Contacts app from the card switcher, but this is probably unrelated, and possibly because I'm using a m-c build right now. But when verifying this bug, please test this and file a bug if you find the same visual artefacts.