Closed Bug 1007600 Opened 6 years ago Closed 5 years ago

[settings] supports inline activity

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S7 (24Oct)

People

(Reporter: arthurcc, Assigned: arthurcc)

References

Details

(Whiteboard: [p=2])

Attachments

(1 file, 1 obsolete file)

STR:
1. Launch message app
2. Click on the option button on the top-right corner.
3. Click on the settings button.

Actual result:
- The message settings panel shows up from the right
- Entering card view, settings app is displayed

Expected result:
- The message settings panel should show up from the bottom (inline activity)
- Settings app is not displayed in card view
Target Milestone: --- → 2.0 S2 (23may)
Status: NEW → ASSIGNED
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
feature-b2g: --- → 2.0
feature-b2g: 2.0 → ---
Attached file WIP v1 (obsolete) —
Comment on attachment 8429089 [details]
WIP v1

EJ, could you help review this patch? As the dependency (bug 973453) is not landed yet, I included the commit in the pull request. Please review the second commit. Thanks.
Attachment #8429089 - Flags: review?(ejchen)
Comment on attachment 8429089 [details]
WIP v1

Arthur, 

just left some comments on Github, and based on our offline discussion, we may have to ask UX and the others about right animations first and let's see how to fix this.

If we are not going to fix the problem about animations, we can still land the patch because the functionality is ready. 

Please ping me if there is any discussion ! Thanks :)
Comment on attachment 8429089 [details]
WIP v1

Hi Arthur,

please ping me again after there is a plan coming out for the decision. 

Thanks :)
Attachment #8429089 - Flags: review?(ejchen)
Comment on attachment 8429089 [details]
WIP v1

Thanks for reviewing, EJ!

I've addressed your comments and added tests for ActivityHandler. Note that I modified ActivityHandler a little bit for writing tests easily. Do you mind take a look at it again?
Attachment #8429089 - Flags: review?(ejchen)
Comment on attachment 8429089 [details]
WIP v1

Alive, the frame for displaying inline activity is under the utility tray. So I hide the utility tray when the wifi button in quick settings triggers an activity. Please help review the change.

Rudy, I modified the keyboard to make it use the new activity for going back to settings app. Please help check it.

Thanks both!
Attachment #8429089 - Flags: review?(rlu)
Attachment #8429089 - Flags: review?(alive)
Comment on attachment 8429089 [details]
WIP v1

See my comment
Attachment #8429089 - Flags: review?(alive)
Comment on attachment 8429089 [details]
WIP v1

Comment addressed.
Attachment #8429089 - Flags: review?(alive)
As keyboard settings needs window position to go back to settings app, we are not able to land this patch until we have another way of doing it. Bug 1005827 is one of the option to be implemented in the system app. Let's block on it.
Depends on: 1005827
With this patch, 3rd party keyboard app will jump into updated `configure` logic and will exit accidentally when clicking on back button on the keyboard. 

Let's postpone this feature until the dependency bug got fixed or any other better way coming.
Comment on attachment 8429089 [details]
WIP v1

I'll clear the review flag here, since we're not going to land this soon and will provide a v2.0 fix for bug 1016280.
Attachment #8429089 - Flags: review?(rlu)
Comment on attachment 8429089 [details]
WIP v1

It seems no review is necessary at this point.
Attachment #8429089 - Flags: review?(alive)
Comment on attachment 8429089 [details]
WIP v1

Because this feature got postponed to 2.1, let me remove the review tag first.
Attachment #8429089 - Flags: review?(ejchen)
Target Milestone: 2.0 S3 (6june) → 2.0 S6 (18july)
Blocks: 969264
No longer blocks: 956210
Not able to finish it within v2.1. Move to the next release.
No longer blocks: settings-2.1
Status: ASSIGNED → NEW
Target Milestone: 2.0 S6 (18july) → 2.1 S5 (26sep)
Blocks: 1071557
Attached file WIP v2
Attachment #8429089 - Attachment is obsolete: true
Duplicate of this bug: 1081592
Target Milestone: 2.1 S5 (26sep) → 2.1 S7 (24Oct)
No longer depends on: 1005827
Comment on attachment 8501574 [details]
WIP v2

It seems bug 1005827 will not be resolved any soon so I would like to land this feature. The new patch was rebased to the current master but what it does is basically the same as the previous patch. 

EJ, alive, would you mind review the patch? Thanks!
Attachment #8501574 - Flags: review?(ejchen)
Attachment #8501574 - Flags: review?(alive)
Attachment #8501574 - Flags: review?(alive) → review+
Comment on attachment 8501574 [details]
WIP v2

Arthur, I just left few comments on Github and please check them ! thanks :P
Attachment #8501574 - Flags: review?(ejchen)
Comment on attachment 8501574 [details]
WIP v2

Comments addressed. Would you mind take a look at it again? Thanks.
Attachment #8501574 - Flags: review?(ejchen)
Comment on attachment 8501574 [details]
WIP v2

thanks for the patch, Arthur ! r+
Attachment #8501574 - Flags: review?(ejchen) → review+
Comment on attachment 8501574 [details]
WIP v2

Zac, would you mind review the modifications on gaia ui tests? Thanks!

The change was due to the switch from window disposition to inline disposition.
Attachment #8501574 - Flags: review?(zcampbell)
Comment on attachment 8501574 [details]
WIP v2

I checked this on my Flame and the test fails with this change.

The original Py test code works fine so just revert the changes to the two Python files and this will be fine.
Attachment #8501574 - Flags: review?(zcampbell) → review-
Comment on attachment 8501574 [details]
WIP v2

Thanks for the information, zac. Clear the review flag as the gaia ui changes have been removed.
Attachment #8501574 - Flags: review-
Treeherder is green: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=c81d89b3e9b2

master: ba10744d64411a8a12ae68f7cf1ec3e3ac897d21
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 957522
Duplicate of this bug: 1071557
Duplicate of this bug: 1084018
Depends on: 1088201
feature-b2g: --- → 2.2+
Duplicate of this bug: 924409
Duplicate of this bug: 1028790
Duplicate of this bug: 1093904
Duplicate of this bug: 1101290
Remove the feature b2g tag to match the new v2.2 scope.
feature-b2g: 2.2+ → ---
You need to log in before you can comment on or make changes to this bug.