removeKeyboards API function validation + enhancement.#395
Conversation
…mponent mentioned in the issue.
web/testing/issue005/header.js
Outdated
| console.log("Keyboard '" + sKbd + "' removal success: " + result); | ||
| } | ||
|
|
||
| // Add keyboard on Enter (as well as pressing button) |
There was a problem hiding this comment.
Yeah, I'm a big fan of CTRL+C CTRL+V at times...
| sKbd=document.getElementById('kbd_id1').value; | ||
| kmw.addKeyboards(sKbd); | ||
| break; | ||
| case 2: |
There was a problem hiding this comment.
It seems your test page only uses addKeyboard(1)
Were you planning to use this switch statement, or can it be removed?
There was a problem hiding this comment.
It can be; that code is lifted straight from the original uncompiled.html sample page. In case we decide later to reinstate the code, I left it in place.
darcywong00
left a comment
There was a problem hiding this comment.
minor cleanup on the test page
web/testing/issue005/header.js
Outdated
| function removeOnEnter(e) | ||
| { | ||
| e = e || window.event; | ||
| removeKeyboard(); |
There was a problem hiding this comment.
When I have the console log open, it seems removeKeyboard() is being called on every keystroke, not just on enter.
There was a problem hiding this comment.
Good catch there; fixed.
| kmw.addKeyboards({id:'lao_2008_basic',name:'Lao Basic', | ||
| languages:{ | ||
| id:'lao',name:'Lao',region:'Asia', | ||
| font:{family:'LaoWeb',source:['../font/saysettha_web.ttf','../font/saysettha_web.woff','../font/saysettha_web.eot']} |
There was a problem hiding this comment.
Do you have a local /testing/fonts folder?
My chrome console is not finding /testing/fonts/saysettha_web.*
There was a problem hiding this comment.
Nope, that issue's been there for quite a while; it's something I've just simply lived with for now. It's straight from the samples-section samplehdr.js file.
|
|
||
| if(success) { | ||
| // Always reset to the first remaining keyboard | ||
| keymanweb._SetActiveKeyboard(ss[0]['KI'],ss[0]['KLC'],true); |
There was a problem hiding this comment.
Surely we should only set active keyboard if the active keyboard was removed?
| for(j=ss.length-1; j>=0; j--) { | ||
| if('Keyboard_'+arguments[i] == ss[j]['KI'] && ss.length > 1) { | ||
| ss.splice(j,1); | ||
| success = true; |
There was a problem hiding this comment.
I think that success should reflect all keyboards being removed successfully.
chore: Use bootstrapping from shared-sites 🦭
Fixes #5.
Includes code style formatting fix - reviewing the KeymanWeb code component on Github may be easier with the
?w=1flag.