Closed Bug 1040598 Opened 10 years ago Closed 10 years ago

Move keyboard states out of keyboard.js

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Keyboard
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2.1 S2 (15aug)

People

(Reporter: timdream, Assigned: timdream)

References

Details

(Whiteboard: [p=5])

Attachments

(1 file)

Github: https://github.com/mozilla-b2g/gaia/pull/22291
(deleted), text/x-github-pull-request
rudyl
: review+
Details 
With bug 1035062, the keyboard.js now contains 600 lines and mostly are logic involving starting the app and loading/switch between different layout and im engines.

The keyboard show/hide state would need yet another manager module, and LayoutManager and InputMethodManager can bear more responsibilities on loading layout/IMEngine. I am not sure about the line to drawn though.

This bug ideally should make keyboard.js contains fewer than 10 lines, basically just

window.app = new KeyboardApp(); app.start();

If the patch turned out to be too big we can always clone another bug and follow-up.
With bug 1041411, the requirement of this bug need to updated to simply removing keyboard.js, since the bootstrap part has moved to it's own script too.
Upon looking into the code a bit, I realized it make more sense for me to take this bug and finish what I have started.

The plan now is to create a StateManager and a LayoutRenderingManager to finally move all non-trivial logic to where they belong, and delete the file.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Whiteboard: [p=5]
Target Milestone: --- → 2.1 S1 (1aug)
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
I will be able to submit for review today or tomorrow once all unit tests are written.
The patch happen to overlap with what Rudy is doing in bug 1020779, so this should wait.
Depends on: 1020779
This patch removes keyboard.js for good with it's remaining responsibility moved to StateManager, LayoutRenderingManager, and elsewhere.
Attachment #8464474 - Flags: review?(rlu)
No longer depends on: 1035619, 1020779
Comment on attachment 8464474 [details]
Github: https://github.com/mozilla-b2g/gaia/pull/22291

This looks pretty good to me with only some nits.
r=me.

Thank you.
Attachment #8464474 - Flags: review?(rlu) → review+
master: https://github.com/mozilla-b2g/gaia/commit/76df34a5c18afd0bb68a37617e1f4adc6ff15d4e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: