906598 - Move Gaia Keyboard settings from Settings app into Keyboard app itself

Status: RESOLVED FIXED

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

Description

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

After Bug 858383 is landed for v1.2, we are going to support 3rd-party IME and the settings for each IME app should reside in the app itself.

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

This brings back the settings screen, but now as a child of the keyboard application itself. I think we should not rely on global settings for this in the future but rather go to asyncStorage but that'll breaking customization scripts, FTU, language settings, at the moment.

I've moved the en-US localization strings out of settings and into this app. Guess that's enough?

This PR includes RudyLu/3rdParty_keyboard_support_commit, and two patches of mine to allow it to work in FF (yay, easier development). Rebasing against master will now break because RudyLu/3rdParty_keyboard_support_commit has been backed out. So:

Land RudyLu/3rdParty_keyboard_support_commit
Land this (Other two patches are not required for this patch to work)

Comment 3

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

Comment on attachment 796004 [details] [diff] [review]
Patch

Hi Jan,

Thanks for working on this.

We should move "vibration", "click sound", "word suggestion", and "auto correction" to keyboard app while keep the layout listing in settings app.

This is based on the new keyboard UX spec,
https://bug891678.bugzilla.mozilla.org/attachment.cgi?id=776309

Sorry that I did not make this point clear at the beginning.
Please let me know if you need me or others to do the follow-up for you.

I'll clear the review flag first and please set it back if you're going to have an updated patch.

Comment 4

[Jan Jongboom [:janjongboom] (inactive)]

Ah, my bad, didn't check the design docs.

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

(In reply to Rudy Lu [:rudyl] from comment #3)
> Comment on attachment 796004 [details] [diff] [review]
> We should move "vibration", "click sound", "word suggestion", and "auto
> correction" to keyboard app while keep the layout listing in settings app.

The layout listing is related to the default keyboard app. 3rd party keyboards probably won't have all the layouts that we have or introduce new ones themself. How does it make sense to have a global layout listing then??

Comment 6

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

(In reply to Jan Jongboom [:janjongboom] from comment #5)

> The layout listing is related to the default keyboard app. 3rd party
> keyboards probably won't have all the layouts that we have or introduce new
> ones themself. How does it make sense to have a global layout listing then??

The layout listing should be handled by settings app to include all the layouts exported by each keyboard app whether it is 3rd-party app or built-in.

The listing should look like the 3rd screenshot on page 6. of the UX spec.
(https://bug891678.bugzilla.mozilla.org/attachment.cgi?id=776309)

Built-in Keyboard
=================
English
-----------------
Chinese
-----------------

Swype (just for example)
=================
English
-----------------
German
-----------------


Thank you.

Comment 7

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

Jan,

BTW, our latest branch for 3rd-party IME can be found here,
https://github.com/RudyLu/gaia/tree/3rdParty_keyboard_support_commit2

You may checkout the code from there to see how we implemented as comment 6.
Could you help check how to integrate your work here with that?

Cheers!

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 796004 [details] [diff] [review]
Patch

As discussed with alive on IRC we moved from launching the keyboard app to (ab)using activities. We'll need to do some manifest verification that the filter matches the app name otherwise we'll break this.

Comment 9

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

Comment on attachment 796004 [details] [diff] [review]
Patch

Sorry, but this break our consensus to use launch path to access the settings of each keyboard app,
https://wiki.mozilla.org/WebAPI/KeboardIME#Proposed_Manifest_of_a_3rd-Party_IME

As you mentioned, this depend on the app team do mannifest verification for us, however, I don't think it is good idea since I am sure that's going to happen in app verification process.

If possible, I would like to see we go the original proposal, i.e. using launch_path, unless we do find this way cannot work.

Thanks.

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Yeah, I had that before, but with launch path we cannot go back from 3rd party keyboard settings into the main keyboard settings page; and that means we cannot implement UX as proposed. It's either not having a way to go back or abusing activities.

Or we should add a method in mozApps.getSelf() to close an app and get back to the previous app on the display stack or something. But could get hairy.

Comment 11

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

I am going to discuss this issue with our IME UX, Carrie.
Please stay tuned.

Thanks.

Comment 12

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

Hi Jan,

I've discussed this with our UX Carrie and Juwei.
The conclusion is we should follow the original proposal to use launch_path for settings app to launch the 3rd-party keyboard app's setting page and no go back.

We all agree this is not perfect, but this is because: 
 1. We cannot enforce the 3rd-party settings page to implement [x] or [Finish] button to go back.
 2. (Engineering) I don't think it is a good idea to use "filter" field for this purpose, our marketplace review process may not be able to ensure this.

Hope I explain this clear enough.
Thanks.

Comment 13

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 796004 [details] [diff] [review]
Patch

Reverted the changes regarding activities, please try again.

Comment 14

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

Comment on attachment 796004 [details] [diff] [review]
Patch

Jan,

Great job, r+ with only some comments/css style that may need to be slightly modified.

Besides, About the settings page,  visually I think the text is a little mis-aligned with the header "Keyboard Settings".
I know we don't have a clear visual spec for this, so we can adjust this as a follow-up.

Thanks.

Comment 16

[Jan Jongboom [:janjongboom] (inactive)]

Landed in https://github.com/mozilla-b2g/gaia/commit/76f40264007bd0216bda3aba392b532f69de273a

Comment 17

[Ivan Tsay (:ITsay)]

Correct dependency with 914093

Comment 23

[William Hsu [:whsu]]

Hi, all,

Thanks for your help.
These settings are added back to built-in keyboard settings.

* Test build:
 - Gaia:     9cb0bf07dd6b8c23ee4c4db3d866464077f81e45
 - Gecko:    afc0d277a978e95a9c74d1daa41af0bd15c4d6e6
 - BuildID   20130916060051
 - Version   26.0a1

Attaching screenshot.

Comment 24

[William Hsu [:whsu]]

Attachment: Built-in keyboard settings

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Landed before branching.