Closed Bug 992085 Opened 6 years ago Closed 5 years ago

[User Story] Edge Gestures When Landscape App is Encountered in Portrait Mode

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S3 (6june)
feature-b2g 2.0
tracking-b2g backlog
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: pdol, Assigned: etienne)

References

Details

(Keywords: feature, Whiteboard: [ucid:System188, ft:systemsfe, 2.0], [p=15])

User Story

As a user I want to be able to remain in portrait mode when I encounter a landscape app while swiping so as to avoid rotating the device multiple times to move between portrait and landscape apps in a sequence.

Acceptance Criteria:
1. Having opened multiple apps with the device in portrait orientation and the current foreground app in portrait mode, swiping from the left edge bring the previous sheet into view.
2. Upon encountering an app (while swiping from the left edge as in AC1) which is in landscape mode while the device is in portrait orientation, the device remains in portrait mode.
3. After pausing on a landscape mode app as in AC2, the device is put into landscape mode and swiping from the left edge no longer works while the device is in portait orientation.
4. Interaction matches UX spec.

Attachments

(2 files, 1 obsolete file)

WIP
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
alive
: review+
Details | Review
No description provided.
User Story: (updated)
Blocks: 993119
Assignee: nobody → alive
Whiteboard: [ucid:System188], [ft:systemsfe] → [ucid:System188], [ft:systemsfe], [p=15]
QA Contact: rafael.marquez
On it from now on.
Component: Gaia → Gaia::System::Window Mgmt
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Target Milestone: --- → 2.0 S2 (23may)
blocking-b2g: --- → backlog
Whiteboard: [ucid:System188], [ft:systemsfe], [p=15] → [ucid:System188, ft:systemsfe, 2.0], [p=15]
Rough idea now:
[Before transition] Rotate and translate the prev/next appWindow to the left side/right side of current window by its orientation settings
[During transition] Maintain a transformation matrix and its inverse matrix in the appWindow. Implement AppWindow.prototype.moveLeft/AppWindow.prototype.moveRight which uses the inverse matrix.
[After transition] Remove the transform of the appWindow and lock screen orientation at the same time.
The states machine for sheet transition:
### States ###
* Center
* Left
* Right
* CtL
* CtR
* RtC
* RtL

### Events ###
* move
* forward (success)
* back (cancel)
feature-b2g: --- → 2.0
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
Attached file WIP
1. Have window_layout.css to store layout rules
2. Have placeholder in appWindow
3. Have sheetTransitionController mixin
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #4)
> Created attachment 8426931 [details] [review]
> WIP
> 
> 1. Have window_layout.css to store layout rules
> 2. Have placeholder in appWindow
> 3. Have sheetTransitionController mixin

The function basically works but:
Need to clean sheet_transition.js
Need to fix some stack error in stack_manager
Need more tests.
Comment on attachment 8426931 [details] [review]
WIP

* Move some logic which is appWindow specific from sheets_transition to sheet_transition_controller within AppWindow.
* Move some logic which is appWindow specific from stack_manager to sheet_transition_controller within AppWindow.
* Rewrite the layout logic of AppWindow: now all window are as same size as the device. The size of the iframe is changing with statusbar and something else.

Known issues
* Unwanted next window is appearing when sheet transitioning.
* Unsmooth transform.
* Fallen tests.
* App without orientation(browser app) will break current logic.

I will try to stabilize this patch tomorrow.
Attachment #8426931 - Flags: feedback?(etienne)
Comment on attachment 8426931 [details] [review]
WIP

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #6)
> Comment on attachment 8426931 [details] [review]
> WIP
> 
> * Move some logic which is appWindow specific from sheets_transition to
> sheet_transition_controller within AppWindow.
> * Move some logic which is appWindow specific from stack_manager to
> sheet_transition_controller within AppWindow.
> * Rewrite the layout logic of AppWindow: now all window are as same size as
> the device. The size of the iframe is changing with statusbar and something
> else.

(left some random comments on github)

The SheetTransition re-organisation with a SheetTransitionController and a SheetTransitionManager looks promising and it's going to be fun to test the getTransformString method :)

But I'm a bit worried about the removal of the broadcastTimeout logic in the StackManager.
How are we going to implement the feature (ie. being able to swipe past an app in a different orientation without stopping) if we're not queuing the swipes and broadcasting only 1 app change?
Attachment #8426931 - Flags: feedback?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #7)
> Comment on attachment 8426931 [details] [review]
> WIP
> 
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #6)
> > Comment on attachment 8426931 [details] [review]
> > WIP
> > 
> > * Move some logic which is appWindow specific from sheets_transition to
> > sheet_transition_controller within AppWindow.
> > * Move some logic which is appWindow specific from stack_manager to
> > sheet_transition_controller within AppWindow.
> > * Rewrite the layout logic of AppWindow: now all window are as same size as
> > the device. The size of the iframe is changing with statusbar and something
> > else.
> 
> (left some random comments on github)
> 
> The SheetTransition re-organisation with a SheetTransitionController and a
> SheetTransitionManager looks promising and it's going to be fun to test the
> getTransformString method :)
> 
> But I'm a bit worried about the removal of the broadcastTimeout logic in the
> StackManager.
> How are we going to implement the feature (ie. being able to swipe past an
> app in a different orientation without stopping) if we're not queuing the
> swipes and broadcasting only 1 app change?

Ya, it's something I missed...
Thanks, I will think about this again and provide fix in next round.
Status today:
Added the delay commit timer back but the complex call route makes me sad. I will consider how to improve this section tomorrow.
Status today:
Rework edgeSwipeDetector and its test - all pass.

Next:
Add test for sheetTransitionManager and sheetTransitionController
Comment on attachment 8426931 [details] [review]
WIP

WIP v2:
Still having some issues but need feedback on architecture change from v1.
* Adding window_layout.css
* Fix orientation-free app by getting rotationg degree from its width/height.
* Move "commit" from stackManager into appWindow.
* Fix some edgeSwipeDetector test issues

Todo
* Keyboard issues seems reoccured.
* Window opacity conflicts
* Sometimes the center sheet returns to center area during swipe.
* Statusbar is annoying during sheet transition
Attachment #8426931 - Flags: feedback?(etienne)
Comment on attachment 8426931 [details] [review]
WIP

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> Comment on attachment 8426931 [details] [review]
> WIP
> 
> WIP v2:
> Still having some issues but need feedback on architecture change from v1.
> * Adding window_layout.css

+1

> * Fix orientation-free app by getting rotationg degree from its width/height.

+1 

> * Move "commit" from stackManager into appWindow.

Mmmhh, AppWindowManager maybe, but the "commiting concept" isn't at the window level but one level above.

> Todo
> * Keyboard issues seems reoccured.
> * Window opacity conflicts
> * Sometimes the center sheet returns to center area during swipe.
> * Statusbar is annoying during sheet transition

There are a lot of refactoring (and other bugs) being addressed in this patch, I'm supportive of pretty much all of them. But I don't think we can get this rewrite landed by tomorrow without regressions.
I'm especially worried about performance regressions that I can already see in this WIP.

I think we should move forward with the approach here, but I'm also preparing a simpler patch for this user story for the 2.0 timeframe.
I have a ~25 lines patch that fits the user story acceptance criteria, going to attach it for feedback.
Attachment #8426931 - Flags: feedback?(etienne)
Attached file POC - without tests for now (obsolete) —
This is a small patch focusing on the user story described here.
We can show it to the UX team in an hour for feedback. If it's accepted I'm pretty sure we can get it landed tomorrow.

What do you think?
Attachment #8434672 - Flags: feedback?(alive)
Comment on attachment 8434672 [details] [review]
POC - without tests for now

Nice one.
Attachment #8434672 - Flags: feedback?(alive) → feedback+
Assignee: alive → etienne
Attached file Gaia PR
Here we go, now with tests!
Attachment #8434672 - Attachment is obsolete: true
Attachment #8434775 - Flags: review?(alive)
Comment on attachment 8434775 [details] [review]
Gaia PR

Quick and good patch. Thank you Etienne.
Attachment #8434775 - Flags: review?(alive) → review+
https://github.com/mozilla-b2g/gaia/commit/cd1550ddfb3b95e1a06037f98779d6eb2423b6a8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 992084
Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.
Blocks: 992084
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.