Bug 816912 - 3rd party keyboard apps management, r=djf, Tim #9030

Closed
wants to merge 2 commits into
from

Projects

None yet

2 participants

@evelynhung
Mozilla-B2G member
  1. manage running keyboard layouts in System app
  2. list installed keyboard layouts in Settings app
@evelynhung evelynhung Bug 816912 - 3rd party keyboard apps management, r=djf, Tim
1. manage running keyboard layouts in System app
2. list installed keyboard layouts in Settings app
60f1207
@timdream timdream commented on an outdated diff Apr 8, 2013
apps/settings/js/keyboard.js
+const TYPE_GROUP = {
+ 'text': true,
+ 'number': true,
+ 'option': true
+}
+
+const SETTINGS_KEY = 'keyboard.enabled-layouts';
+*/
+
+var KeyboardLayout = {
+ keyboardLayouts: {},
+
+ init: function kl_init() {
+ this.getAllElements();
+ KeyboardHelper.getAllLayouts(this.makeLayoutList.bind(this));
+ //Settings.mozSettings.addObserver('keyboard.enabled-layouts', this.updateList.bind(this));
@timdream
timdream Apr 8, 2013

Is this intentional comment out?

@timdream
Mozilla-B2G member

This is lint error on this file.

@timdream timdream commented on an outdated diff Apr 8, 2013
apps/settings/js/settings.js
@@ -580,6 +581,7 @@ window.addEventListener('load', function loadSettings() {
];
scripts.forEach(function attachScripts(src) {
var script = document.createElement('script');
+ script.type = 'application/javascript';
@timdream
timdream Apr 8, 2013

text/javascript?

@timdream timdream commented on an outdated diff Apr 8, 2013
apps/system/js/keyboard_manager.js
+ var initIndex = self.showingLayout.index;
+ self.launchLayoutFrame(self.keyboardLayouts[initType][initIndex]);
+ }
+ if (evt) {
+ // update because of observing settings change
+ var allLayouts = JSON.parse(evt.settingValue);
+ resetLayoutList(allLayouts);
+ } else {
+ // update because of initializing
+ KeyboardHelper.getAllLayouts(resetLayoutList);
+ }
+ },
+
+ inputFocusChange: function km_inputFocusChange(evt) {
+ // let value selector notice the event
+ dispatchEvent(new CustomEvent('inputfocuschange', evt));
@timdream
timdream Apr 8, 2013

window.dispatchEvent ?

@timdream
timdream Apr 8, 2013

Is evt being reused transparently from another event?

@timdream timdream commented on the diff Apr 8, 2013
apps/system/js/keyboard_manager.js
- // Listen for mozbrowserlocationchange of keyboard iframe.
- var previousHash = '';
+ handleKeyboardRequest: function km_handleKeyboardRequest(evt) {
+ var url = evt.detail.url;
+ this._debug('handleKeyboardRequest: ' + url);
+ // everything is hack here! will be removed after having real platform API
+ if (url.lastIndexOf('keyboard-test') < 0)
@timdream
timdream Apr 8, 2013

Is the hack doing anything special with keyboard-test app?

@evelynhung
evelynhung Apr 9, 2013

it's not an app name, just a keyword to distinguish the window.open url is a special request.

@timdream timdream commented on an outdated diff Apr 8, 2013
apps/system/js/keyboard_manager.js
+ }, FOCUS_CHANGE_DELAY);
+ break;
+ case 'showlayoutlist':
+ clearTimeout(this.switchChangeTimeout);
+ this.switchChangeTimeout = setTimeout(function keyboardLayoutList() {
+ var items = [];
+ self.keyboardLayouts[showed.type].forEach(function(layout, index) {
+ var label = layout.appName + ' ' + layout.name;
+ items.push({
+ label: label,
+ value: index
+ });
+ });
+ self.hideKeyboard();
+ ListMenu.request(items, 'Layout selection', function(selectedIndex) {
+ //XXX the type of selectedIndex is string
@timdream
timdream Apr 8, 2013

Let's fix ListMenu instead of XXX here?

@timdream timdream commented on the diff Apr 8, 2013
apps/system/js/keyboard_manager.js
return;
- previousHash = urlparser.hash;
-
- var type = urlparser.hash.split('=');
- switch (type[0]) {
- case '#show':
- var updateHeight = function updateHeight() {
- container.removeEventListener('transitionend', updateHeight);
- if (container.classList.contains('hide')) {
+
+ var urlparser = document.createElement('a');
+ urlparser.href = url;
+ var keyword = urlparser.hash.split('=')[1];
+
+ var self = this;
+ switch (keyword) {
@timdream
timdream Apr 8, 2013

The 3 code block in this switch block should be in their own function.

@timdream
timdream Apr 8, 2013

(this can be done as follow-up)

@evelynhung
evelynhung Apr 9, 2013

I will do this when I try to add test case.

@timdream timdream commented on the diff Apr 8, 2013
apps/system/js/keyboard_manager.js
+ this.showingLayout.index = index;
+ var layout = this.keyboardLayouts[group][index];
+ this.showingLayout.frame = this.launchLayoutFrame(layout);
+ this.showingLayout.frame.hidden = false;
+ this.showingLayout.frame.setVisible(true);
+ this.showingLayout.frame.addEventListener(
+ 'mozbrowseropenwindow', this, true);
+ },
+
+ showKeyboard: function km_showKeyboard() {
+ this.keyboardFrameContainer.classList.remove('hide');
+
+ // XXX Keyboard transition may be affected by window.open event,
+ // and thus the keyboard looks like jump into screen.
+ // It may because window.open blocks main thread?
+ // Monitor transitionend time here.
@timdream
timdream Apr 8, 2013

I would like to find out what really happen before check-in the code.

@evelynhung
evelynhung Apr 9, 2013

Let's test two cases:
1. Does it happen on desktop?
2. Does it happen in OOP case?

@evelynhung
Mozilla-B2G member

close this PR and will request afterwards.

@evelynhung evelynhung closed this May 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment