1007600 - [settings] supports inline activity

Status: RESOLVED FIXED

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

Description

[Arthur Chen [:arthurcc][inactive after 6/30]]

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

Comment 1

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: WIP v1

Comment 2

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.

Comment 3

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 4

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8429089 [details]
WIP v1

Hi Arthur,

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

Thanks :)

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

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?

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

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!

Comment 7

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

Comment on attachment 8429089 [details]
WIP v1

See my comment

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8429089 [details]
WIP v1

Comment addressed.

Comment 9

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.

Comment 10

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 11

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

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.

Comment 12

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

Comment on attachment 8429089 [details]
WIP v1

It seems no review is necessary at this point.

Comment 13

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8429089 [details]
WIP v1

Because this feature got postponed to 2.1, let me remove the review tag first.

Comment 14

[Arthur Chen [:arthurcc][inactive after 6/30]]

Not able to finish it within v2.1. Move to the next release.

Comment 15

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: WIP v2

Comment 17

[Arthur Chen [:arthurcc][inactive after 6/30]]

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!

Comment 18

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8501574 [details]
WIP v2

Arthur, I just left few comments on Github and please check them ! thanks :P

Comment 19

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8501574 [details]
WIP v2

Comments addressed. Would you mind take a look at it again? Thanks.

Comment 20

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8501574 [details]
WIP v2

thanks for the patch, Arthur ! r+

Comment 21

[Arthur Chen [:arthurcc][inactive after 6/30]]

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.

Comment 22

[Zac C (:zac)]

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.

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8501574 [details]
WIP v2

Thanks for the information, zac. Clear the review flag as the gaia ui changes have been removed.

Comment 24

[Arthur Chen [:arthurcc][inactive after 6/30]]

Treeherder is green: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=c81d89b3e9b2

master: ba10744d64411a8a12ae68f7cf1ec3e3ac897d21

Comment 32

[howie [:howie]]

Remove the feature b2g tag to match the new v2.2 scope.