Closed Bug 939809 Opened 6 years ago Closed 6 years ago

[User Story] Update Task Manager to Match Edge Gesture Navigation

Categories

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

x86
macOS
defect
Not set

Tracking

(tracking-b2g:backlog, b2g-master verified)

VERIFIED FIXED
1.4 S2 (28feb)
tracking-b2g backlog
Tracking Status
b2g-master --- verified

People

(Reporter: pdol, Assigned: aus)

References

Details

(Whiteboard: [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5])

Attachments

(1 file)

User Story:
As a user, I want my task manager's app ordering to match the ordering with edge gesture app navigation so that there is consistency between the experiences.

Acceptance Criteria:
1. I open task manager and see that the cards match the order of the apps in the edge gesture stack.
Flags: in-moztrap?(jhammink)
Note: UX will provide a more detailed explanation regarding how task manager should function.
Flags: needinfo?(rmacdonald)
From the UX perspective, we would target the following prior to enabling the edge gesture:

1) We can have a direct mapping between the ordering of sheets in the task manager and by navigating between sheets using the swipe from edge gestures
2) The current sheet appears in the foreground, regardless of its position within the stack.
3) A general improvement to the performance of the animation.
Flags: needinfo?(rmacdonald)
Assignee: nobody → aus
Whiteboard: [ucid:System111, 1.3:P2, ft:systems-fe] → [ucid:System111, 1.3:P2, ft:systems-fe][systemsfe]
Target Milestone: --- → 1.3 Sprint 5 - 11/22
(In reply to Rob MacDonald [:robmac] from comment #2)
> From the UX perspective, we would target the following prior to enabling the
> edge gesture:
> 
> 1) We can have a direct mapping between the ordering of sheets in the task
> manager and by navigating between sheets using the swipe from edge gestures
> 2) The current sheet appears in the foreground, regardless of its position
> within the stack.
> 3) A general improvement to the performance of the animation.

A couple of UX clarifications that came up during a call this morning with Etienne... 

1) The task manager should show the same items that are in the sheet stack. Since the sheet stack includes killed apps, this means that killed apps will also need to be shown in the task manager. Guidelines on how they're displayed are included on page 9 of the 1.3 spec for edge swipe gestures. (https://mozilla.box.com/s/cxsuctcrdsm4aou5983u)

2) If a user closes an item in the task manager, this item should also be removed from the sheet stack.

Best regards,
Rob
Status: NEW → ASSIGNED
No longer blocks: 1.3-systems-fe
Whiteboard: [ucid:System111, 1.3:P2, ft:systems-fe][systemsfe] → [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe]
Flags: in-moztrap?(jhammink)
Aus, right now, based on Comment 3, this story covers both the ordering update AND the implementation of including killed apps in the task manager view.  Should that second piece be broken out into a different bug?
Flags: needinfo?(aus)
It would be simpler to two it two phased by splitting out the killed apps requirement. The new task would depend on this one.
Flags: needinfo?(aus)
Whiteboard: [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe] → [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5]
Blocks: 957473
(In reply to Ghislain 'Aus' Lacroix from comment #5)
> It would be simpler to two it two phased by splitting out the killed apps
> requirement. The new task would depend on this one.

I broke out the killed app piece into bug 957473.
Depends on: 959235
Whiteboard: [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5] → [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5][systemsfe]
Target Milestone: 1.3 Sprint 5 - 11/22 → 1.3 C3/1.4 S3(31jan)
Attachment #8369458 - Flags: review?(etienne)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

Not quite there yet :)

Some comments on github, but to sum up:

* Looks like the apps still get sorted by launch date (and not using the stack order)

* You still can't use edge gesture after using the card view to switch apps, I commented about this on one of Kevin's bug [1], all the details should be there.

* And the tests need some love :)

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=962355#c9
Attachment #8369458 - Flags: review?(etienne) → review-
Etienne, I updated the pull request to fix everything *except* the edge gesture not working after the rocket bar / task manager has been displayed.
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

I've updated the pull request and I believe we should be ready to push to master once Travis confirms that it's green. Patch has been rebased already.
Attachment #8369458 - Flags: review- → review?(etienne)
Just noticed I forgot to remove the constant at the top of stack_manager.js. I've removed it now and pushed the change to the pull request. Sorry about that.
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

Getting there :)
Some comments on github.

But I *really* think we shouldn't land/close this bug without:

* making sure you can still use edge gestures after entering the card view
* making sure that when you open the cardview the selected app reflects your position in the stack

Peter can you confirm?
Attachment #8369458 - Flags: review?(etienne)
Flags: needinfo?(pdolanjski)
> But I *really* think we shouldn't land/close this bug without:
> 
> * making sure you can still use edge gestures after entering the card view

Yes, totally! I'll be flashing my phone with the changes present in this patch to make sure they work.

> * making sure that when you open the cardview the selected app reflects your
> position in the stack

I'm a fan of this. I think we should definitely open the cards view with the current active app being selected.
(In reply to Etienne Segonzac (:etienne) from comment #12)
> But I *really* think we shouldn't land/close this bug without:
> * making sure you can still use edge gestures after entering the card view
> * making sure that when you open the cardview the selected app reflects your position in the stack
> 
> Peter can you confirm?

This makes sense to me.  I'd leave it open until this is validated.
Flags: needinfo?(pdolanjski)
OK. Just one thing left... when displaying the cards view, we should select the the card for the current active application.
Target Milestone: 1.3 C3/1.4 S3(31jan) → 1.4 S1 (14feb)
Flags: in-moztrap?(jhammink)
Whiteboard: [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5][systemsfe] → [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][systemsfe]
Target Milestone: 1.4 S1 (14feb) → ---
Whiteboard: [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][systemsfe] → [ucid:System111, 1.4:P2, ft:systems-fe][systemsfe][p=5]
Target Milestone: --- → 1.4 S1 (14feb)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

There it is, last issue sorted out. :) After landing this, I will tackle having killed apps in the stack manager and cards view.
Attachment #8369458 - Flags: review?(etienne)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

Made comments on github, ping me if anything is unclear.
Attachment #8369458 - Flags: review?(etienne)
Blocks: 962355
Attachment #8369458 - Flags: review?(etienne)
OK, hopefully, it's all to your liking. :)
Attachment #8369458 - Flags: review?(alive)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

See github
Attachment #8369458 - Flags: review?(alive)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

Hey Aus, sorry this is taking so long, my first reviews probably weren't exhaustive enough hence the back and forth.

We're getting pretty close though, this last patch is already running on my dogfooding phone :)

I've added to Alive and Zac's comments on github, quite a bit of work still but I'm confident it's an exhaustive list.
Attachment #8369458 - Flags: review?(etienne)
https://github.com/mozilla-b2g/gaia/pull/16045#r9783361
Suggestion for cardsView launching app: we should change this._current but not change the order.
Target Milestone: 1.4 S1 (14feb) → 1.4 S2 (28feb)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

OK, we're in pretty good shape now. All tests pass, all the features work as expected. I'm hoping this is it. :)
Attachment #8369458 - Flags: review?(etienne)
Attachment #8369458 - Flags: review?(alive)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

So close...
2 nits and one very important but very simple fix :)

The unit-tests are failing but looks easy to fix it's probably because bug 962649 landed (make sure to rebase the patch but it should not conflict).

The finish line (which in our case is a world-class multi tasking system) in literally in sight :)
Attachment #8369458 - Flags: review?(etienne)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

I opted for using the setter/getter for position everywhere. It's cleaner and it was an efficient way to enforce _current never turning into a string. Other goodies include full passing tests and no more forgotten dump statement.
Attachment #8369458 - Flags: review?(alive) → review?(etienne)
Comment on attachment 8369458 [details] [review]
Pull Request - Cards View should use Stack Manager

**** yeah!
Attachment #8369458 - Flags: review?(etienne) → review+
Blocks: 931514
Landed!!!

Commit: https://github.com/mozilla-b2g/gaia/commit/91a572e0fb84a12e7499a718dd7e50192a62a442

Turn on edge gestures, and experience an amazing multi-tasking mobile OS!!!
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 959235
No longer depends on: 959235
(In reply to Ghislain Aus Lacroix [:aus] from comment #26)
> Landed!!!
> 
> Commit:
> https://github.com/mozilla-b2g/gaia/commit/
> 91a572e0fb84a12e7499a718dd7e50192a62a442
> 
> Turn on edge gestures, and experience an amazing multi-tasking mobile OS!!!

This is too risky to let this land for 1.4 at this point given our release criteria - this is a high regression risk for breaking the task manager. I need to see a mitigation plan in place to make sure this lands safely before this commit can land (e.g. pre-landing testing). Please backout.
Flags: needinfo?(jhford)
Aus & I talked in person about this - he mentioned he'll back this out on first sign of regressions & thinks there's sufficient test coverage here to catch regressions. On that regard, I'm okay with leaving this in. I'll be sure to have some people regression test to make sure there's no fallouts.
Flags: needinfo?(jhford)
No longer blocks: 957473
Duplicate of this bug: 957473
Depends on: 976960
set 1.3T? to consider if we able to uplift for Tarako.
blocking-b2g: --- → 1.3T?
ni? assignee on the risk involved
thanks
Flags: needinfo?(aus)
(In reply to Marvin Khoo [:Marvin_Khoo] from comment #30)
> set 1.3T? to consider if we able to uplift for Tarako.

This is not a blocker - there's no inherent strong value to consider this for Tarako, as much of the value of this feature comes from when edge gestures are enabled on Tarako. Edge gestures is not enabled on 1.3T, so there's low value to include this feature.
blocking-b2g: 1.3T? → backlog
Flags: needinfo?(aus)
Hi Jason,
i thought this patch be able to fix the recent app to show/display a recently open app that was killed by system.

consider Tarako platform limitation, it help user easier to launch recently open app.

for instance, I'm in mail app, and the mail app is killed after received a call. as a user, s/he be able to navigate back to mail app by using Recent app instead of going back to home and find the mail app icon.

for me, this is very helpful from a user stand point, especially for low end devices. thought?

BR,
Marvin
(In reply to Marvin Khoo [:Marvin_Khoo] from comment #33)
> Hi Jason,
> i thought this patch be able to fix the recent app to show/display a
> recently open app that was killed by system.
> 
> consider Tarako platform limitation, it help user easier to launch recently
> open app.
> 
> for instance, I'm in mail app, and the mail app is killed after received a
> call. as a user, s/he be able to navigate back to mail app by using Recent
> app instead of going back to home and find the mail app icon.
> 
> for me, this is very helpful from a user stand point, especially for low end
> devices. thought?
> 
> BR,
> Marvin

That wasn't my understanding of what this feature does. This feature in this bug just implements support for ordering of the cards based on the last app(s) launched vs. apps recently used.
This one DUP Bug 957473 "Update Task Manager to include apps that were recently open but whose process was killed" reading the title and user stories, it should be the same thing right?
or should i nominate Bug 957473 instead of this one?
(In reply to Marvin Khoo [:Marvin_Khoo] from comment #35)
> This one DUP Bug 957473 "Update Task Manager to include apps that were
> recently open but whose process was killed" reading the title and user
> stories, it should be the same thing right?

Oh right. The implementation here fixed both the user story in this bug & the user story in bug 957473. However, bug 957473 is currently preffed off in all branches. It's currently targeted as a 2.0 feature.

Anyways, I'll discuss this with Peter in the System FE planning meeting on Monday.
Thanks for your prompt feedback!
From Tarako stand point this solution provide ways better experiences. let me catch up with you tomorrow.
A pain point was encountered when attempting to verify this [user story] due to bug 1068345, which causes apps to black screen or open blank when attempting multiple edge gestures, however the integrity of the stack seems stable while opening in separate parts of task manager and attempting multiple slides.

Verifying fix on flame 3.0 devices.
Results: Apps held in Task Manager hold same logical order when observed in Manager (order= order opened from earliest[left] to latest[right]), and as when navigating between them via edge gestures.

--------------------------------------------------
Environmental Variables:
Device: Flame 3.0
BuildID: 20150210010523
Gaia: 0cf517083f7eb5fc269e1236edba50534f65e3cd
Gecko: 2cb22c058add
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
--------------------------------------------------
Repro: 7/7
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.