1047837 - Allow each layout define more symbol pages and the relations for switching

Status: RESOLVED FIXED

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

Description

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

+++ This bug was initially created as a clone of Bug #1046061 +++

[Background]
with bug 1046061 comment 0, we would like to define a generic way to handle layout page switching, especially for the case that there are more than 2 symbol pages, e.g. pinyin input method.
However, I tried to land a partial fix for this problem to unlock other layout related issues.

[Intent to implement]
This bug should be implement the following things, 
 1. To allow each layout define several layout page (or sub-layout), not only alternateLayout and symbolLayout.
    (The above sub layouts should be loaded by layout loader module.)
 2. Can specify a key which is capable of switching between these sub layouts.
    e.g. { value: 'Alt', pageSwitching: 'sub-layout2'}.
    With this, we could probably make layoutManager.currentLayoutPage as a string instead of ENUM.

Comment 1

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

Taking as way to move forward to implement Emoji keyboard.

Comment 2

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

Attachment: Github: https://github.com/mozilla-b2g/gaia/pull/23936

This patch do the following:

-- Officially separate the idea of layout and layout page, where pages are put into an array under "pages" property of the layout
-- A normalize step in the loader will try to create the 0th page from properties of the layout (that should really go to the page under the new concept)
-- alternateLayout is now the 1st page in the array
-- symbolLayout is now the 2nd page in the array
-- forced layout is removed. Zhuyin, Pinyin, and Japanese layouts now use the 3rd and 4th page of the layout definition.
-- All switching keys now comes with keyCode=ALT, with a "targetPage" property to tell the which page it is going to.
-- currentLayoutPage is now currentPageIndex.
-- currentModifiedLayout is now currentPage (and expose the information of the page, not the layout)
-- however, some layout information is copy over from layout to the page.
-- currentLayout should not be used by others and is removed.

Apologies for not being able to split the patch into nicer chunks. I wasn't sure the scope of this patch until I actually work on it. I have a few bugs already filed that is actually done in this patch -- I will dup them over here.

Comment 5

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8487698 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23936

Overall this great work looks good to me! (Y).
What I'm a little bit concerned about is the use of |undefined| in place of |pages[0]| in the majority of the layout definition files, which may seem little self-explanatory for other people who desire to add new layouts in the future, without studying your patch or any comments in the layout definition file. We could add comments there, but maybe another property, with a descriptive name, in the definition file would be more robust.

Also some keyboard tests in Gip are failing, and you might want to check if your patch has not broke gaiatest/apps/keyboard/app.py -- there is a function called |_switch_to_correct_layout| that could smell fishy.

> -- All switching keys now comes with keyCode=ALT, with a "targetPage"
> property to tell the which page it is going to.

Yappie! I was going to normalize that in bug 1044525. Glad I haven't touched that change.

Comment 6

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

keyboard/app.py updated.

Comment 7

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

Comment on attachment 8487698 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23936

Zac, would you might checking the keyboard/app.py changes? I updated the DOM of switching keys so I update the test accordingly.

Comment 8

[Zac C (:zac)]

Comment on attachment 8487698 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23936

r+, tested on device and it's good.

Comment 9

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

Comment on attachment 8487698 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23936

Looks good to me as well.
Thanks for taking over this change for me.

Comment 10

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

master: https://github.com/mozilla-b2g/gaia/commit/2f5a1aff6b39f6e8e03e3a69c05d57d5b0987409