Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Web] Implementation of modifier chirality #300

Merged
merged 46 commits into from
Oct 4, 2017
Merged

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Sep 5, 2017

Remaining to-dos:

  • Solid cross-platform compatibility testing. Mobile devices with external keyboards may be particularly in question.
  • New chirality-related API should be properly documented on help.keyman.com.
  • The SpecialOSK font lacks *LALT*, *RALT*, etc keycaps for keyboards with chiral information.
  • Mobile presently lacks extensive default chiral-oriented layout structure.
    • Mobile also lacks state keys (like CAPS LOCK) in its default layout.
    • We merely need to decide on what structure should be default, as the system adapts quite well when adding new subkeys for mobile chiral layers.
    • So far, only variants of the base modifiers (unpaired) have been added; these are in need of font support as noted above.
      • This leaves the bootstrapping chirality.js keyboard without full OSK access when used for testing on Android; not all of its layers have corresponding defaults in kmwlayout.js.
  • Embedded integration with KMEA is being done in PR Android modifier chirality #308.
  • Keyman Developer: compiler integration is being done in PR Chirality support for Keyman Developer compiler #313.
  • Keyman Developer: visual editor integration does not yet have a standing PR.

This is a PR aimed to address #9 and #52. Note that the core implementation is practically complete,. There are a few bells and whistles to be worked out, but they are minor enough to be their own separate PR if necessary.

Other related tasks are noted below.

It's going to be a big one, so to facilitate code review, here's some documentation of the PR thus far.


  • Commit 1 defines numerous new constants related to modifiers and what we're terming 'state keys' (like CAPS). Also performs the initial refactoring and rework of osk.buildDefaultLayout.

  • Commits 2 and 3 refine the work on osk.buildDefaultLayout.

    • As of this point, pre-10.0 keyboards (literally, all of them that currently exist) have their layout data translated into the upcoming 10.0 keyboard spec before being processed. The results have been tested on existing (non-chiral) keyboards for 'legacy' compatibility and everything seems to be in place to preserve pre-10.0 keyboard functionality. (Tested with Web in Firefox and iOS Safari and with Embedded within the iOS app.)
  • Commit 4 creates an initial testing framework for 10.0 keyboard specs.

    • The 'chirality.js' keyboard needs a lot of additional development to be good for testing.
    • However, it resulted in creation of the 'halfDefined.js' keyboard, which provided very useful validation information for the new osk.buildDefaultLayout function.
      • Accordingly, that function got some extra love and now has proper default behavior for the default layers.
  • Commit 5 extends the present implementation of 'chirality.js' with visual layout information for some chiral modifier combinations while correcting some formatting issues. (Tabs vs spaces.)

  • Commit 6 is a small one fixing a bitmask that accidentally got erased somewhere and adding a new one for testing if a keyboard is chiral.

    • The CHIRAL bitmask represents all modifiers to check for chiral-mode keyboards.
    • The IS_CHIRAL bitmasks represents only those modifiers unique to chiral-mode keyboards, for comparison against the KMBM property.
  • Commits 7-9 deal with the chirality constraints for the default keyboard layout's layer generation.

    • We do wish to allow the layers to be made since they provide visual feedback to the user, though they should never have defined keyboard information.
    • The constraints will not be explictly checked by KMW, as per discussions with @mcdurdin - it's enough of an edge case (user-handwritten keyboards) that it's not worth the code space.

  • Commit 10 brings the next phase of the implementation - chiral modifier input support.

    • KMW properly references the KMBM property for managing chiral mode.
      • Comes with the new API function keymanweb.isChiral(), which assumes the active keyboard unless a different keyboard is specified.
      • We could hide it if desired, but I thought I'd expose it like keymanweb.isCJK(). It's a handy function for managing chiral-related control paths.
    • KMW properly manages the OSK's layers.
      • Works properly and responsively with physical chiral and nonchiral keystrokes.
      • Properly manages the layers when under direct OSK control, at least for automatically-managed layouts.
  • Commit 11 merely enhances the documentation for osk.updateLayer.

    • At present, the logical structure (though preserved from the original) seems to accommodate behavior we never designed for; its logic may be possible to simplify. The documentation should help resolve the issue whenever we take a better look at it.

  • Commit 12 brings yet another stage - presenting the first chiral keyboard implementation & operation!

    • The testing keyboard chirality.js now actually processes keystrokes. It's a hand-written keyboard using a few techniques similar to certain compiler changes we're planning for Keyman Desktop.
      • That is, it directly references a few constants defined in kmwosk.js.
    • Both the physical keyboard and the OSK produce nice output. Just... be wary of pre-existing keyboard shortcuts, 'cause the system doesn't block them yet.
      • If SHIFT-ALT-O is a problem, check your Keyman Desktop shortcuts.
  • Commit 13 makes the first tweak to kmwlayout.js - the AltGr mobile subkeys were shifted to the proper kebab-case, as they were broken without that.


  • Commit 14 presents the first implementation of state key handling in KeymanWeb.
    • The state keys now act exactly as modifier keys within the OSK.
      • Activating CAPS LOCK via physical keyboard displays an active CAPS LOCK key.
      • Having a lit CAPS LOCK in the OSK does NOT auto-cap characters typed with the physical keyboard, just as having a lit 'SHIFT' modifier in the OSK does not apply the SHIFT layer to physically-typed key events.
        • It is possible that this behavior of the modifier keys is an error; I simply designed for consistency here.
    • The handwritten example chiral keyboard has been set to globally autocap all typed characters when CAPS LOCK is active. There are no SCROLL_LOCK or NUM_LOCK rules in place at this time.

Commit 15 mostly adds fixes as per @mcdurdin's first code review.

Commits 16-17 perform lots of work to enhance handling of OSK state key feedback, though there's still one edge case under "yet to do" to be resolved.

  • Commented-out code was written toward a first design for AltGr emulation/handling - commented-out due to its instability and complexity at the time.

Commits 18-19 enable AltGr handling/emulation.

  • Included is a bit of a preemptive bugfix - the OSK is unreliable for tracking modifier state data, as it is not modified when using a physical keyboard with mobile devices.
    • It'd have been a fairly nasty bug for Android integration.
    • It required substantial redesign of the modifier handling code, but I feel the result yields overall simpler and more understandable code. I'd been trying to over-optimize with the prior design.

... and then I mucked up the commit structure by attempting to rebase instead of merge for the new history.md file.

The most recent commits:

The system now persistently tracks state key activation and detects when any modifier or state key change occurs after each keypress, ensuring that the OSK never lags behind by more than one keystroke.

  • This undoes the previous approach from commits 16-17.
  • Also added here - a small update for history.md.

Some formatting changes and removal of unnecessary Closure formatting.


Notes:

NUM_LOCK and SCROLL_LOCK do not appear on any keyboard layouts. Rules are in place to treat them similarly to CAPS_LOCK if explicitly added to a layout by a designer.

ALT is sticky in Firefox when used in combinations with other keys. This is a Firefox issue:

Certain keystroke combinations will trigger browser shortcuts instead of outputting the correct keys. This is due to some modern browsers "protecting" shortcuts and cannot be circumvented:

…f modifier chirality. Also defines a lot of nice modifier + state-key constants.
…ns, aiding the readability of osk.buildDefaultLayout.
…rected default behavior of buildDefaultLayout when KLS lacks specifications for default layers.
…nd the physical keyboard and also ensures that `osk.clickKey` operates as expected for existing keyboards.
…nd osk interaction.

Yet to do:  state keys, embedded integration.
…ase - AltGr was broken for mobile due to chirality changes.
… version of state keys acts just like the OSK version of modifiers. Chiral keyboard now auto-caps all keys when CAPS LOCK is active.

// TODO: Ensure the keypress is also the CTRL or the ALT for the e.location != 0 check.
if(e.ctrlKey) {
s.Lmodifiers = s.Lmodifiers | ((e.location != 0 && ctrlEvent) ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find these two lines very hard to grok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two separate conditions that should trigger the CTRL modifier.

  1. The user literally just pressed the CTRL key, so the event has a valid location property we can utilize, but its layer isn't presently activated within the OSK.
  2. CTRL has been held a while, so the OSK layer is valid, but the key event doesn't tell us the chirality of the active CTRL press.

In either case, e.ctrlKey is set to true, telling us that there is an active CTRL modifier - but not if it's new or maintained from a prior _KeyDown.

The e.location != 0 test passing matches condition 1 above, while its failure matches condition two - hence the layered assignment line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm asking if we can rewrite or expand the conditions to make it easier for other codes to grok as well. The documentation that you've just written here would make an excellent code comment.

case 17:
case 18: keymanweb._NotifyKeyboard(Levent.Lcode,Levent.Ltarg,0); return osk._UpdateVKShift(Levent, Levent.Lcode-15, 1); // I2187
case 18:
case 20: //"K_CAPS":20, "K_NUMLOCK":144,"K_SCROLL":145
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these vk codes consistent on Mac and Linux as well? (and all browsers)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be operating under the assumption that by this point in the code, the vk codes have all been unified to those defined toward the top of kmwosk.js. I guess I ought double-check that this assumption is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vk codes are confirmed to be Hackintosh-approved, at the least.

Looking at the MDN keycode table, we can rely on these modifier codes. The one problem would be for Linux when AltGraph is a thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that table is only listing desktop OSes, with nothing for mobile. Android and iOS may not match up; that'll take further testing.

}

if(k) {
if(k['KV']['KMBM']) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if k['KV'] is undefined/null then this will fault

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, good catch.

{ "id": "K_ALTGR", "text": "*AltGr*", "sp": "1", "width": "50", "nextlayer": "ctrlalt" }]
{ "id": "K_LALT", "text": "*LAlt*", "sp": "1", "width": "50", "nextlayer": "leftalt" },
{ "id": "K_RALT", "text": "*RAlt*", "sp": "1", "width": "50", "nextlayer": "rightalt" },
{ "id": "K_ALTGR", "text": "*AltGr*", "sp": "1", "width": "50", "nextlayer": "ctrl-alt" }]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

K_ALTGR is not a valid identifier; I can see what you are wanting to do here but we'll need to find another way to specify that key (can it be implemented by looking for both alt and control selected?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... then our mobile layouts were never valid in the first place? I merely corrected it; it already existed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep in mind these two are for the mobile layouts 'phone' and 'tablet' - neither of which has any actual modifiers or physical keyboard related to them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's discuss this in the office -- I'm not sure what the history behind the use of K_ALTGR in the code is here but it's not a standard Keyman virtual key identifier so we need to be careful with overloads.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If K_ALTGR ends up staying, can we keep the nextlayer name "ctrlalt"? For consistency, none of the other layers have hyphenated names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the chirality design doc (as per @mcdurdin's request) is that we use kebab-case when combining modifiers to obtain a layer name. ctrl-alt is the only nextlayer comprised of multiple modifiers here, hence why it's the only one with a hyphen.

Without the hyphen, the nextlayer operation will break without somewhat significant changes to the layer id management scheme in this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making sure here: existing keyboards don't have kebab-cased layer names. We need to ensure this doesn't break existing keyboards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Existing keyboards that would check for this wouldn't be using the buildDefaultLayout method though, would they?

There is the matter of the default layer-management functions, though, and if a keyboard deliberately chose the name ctrlalt to use said functions this may prove an issue. Know of any good keyboard examples I could use for testing this - ones that provide their own visual layout but prefer to rely on automated layer management?

Then again, that doesn't make sense does it? Touch layouts shouldn't respond to physical keystrokes, so there shouldn't be any automated layer management to speak of here. Desktop devices don't provide their own OSK layout data, thus shouldn't be invoking specific layer names, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A keyboard can target both desktop and touch though. I'm not sure I understand the question: how can a keyboard provide its own visual layout (do you mean .kvk visual keyboard or touch layout?) and also rely on automated layer management?
https://github.com/keymanapp/keyboards/blob/master/fv_xaislakala_kmw/source/fv_xaislakala_kmw.kmn is a sample keyboard that has both touch and ctrl+alt.

{ "id": "K_ALTGR", "text": "*AltGr*", "sp": "1", "width": "50", "nextlayer": "ctrlalt" }]
{ "id": "K_LALT", "text": "*LAlt*", "sp": "1", "width": "50", "nextlayer": "leftalt" },
{ "id": "K_RALT", "text": "*RAlt*", "sp": "1", "width": "50", "nextlayer": "rightalt" },
{ "id": "K_ALTGR", "text": "*AltGr*", "sp": "1", "width": "50", "nextlayer": "ctrl-alt" }]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto on K_ALTGR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've done an initial review. I need to go through it all again and do actual testing once we have the compiler done, but there are a few things here we can dig into.


osk.modifierBitmasks = {
"ALL":0x007F,
"CHIRAL":0x001F, // The default bitmask, for non-chiral keyboards
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the comment here

case 'K_LCTRL':
if(keymanweb.isChiral()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit concerned about the use of K_LCONTROL and K_LCTRL and so on and so forth. I recognise this isn't new code, but why is this happening (and can we fix it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty curious too, but didn't want to rock the boat there. Some of it may have to do with layouts and/or cross-platform compatibility, I'm guessing.

case 'K_RCTRL':
if(keymanweb.isChiral()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

K_LCONTROL/K_LCTRL will drop through to this and give wrong nextLayer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will only drop through if keymanweb.isChiral() == false - note the break in the conditionals above. The fall-through is deliberate for the isChiral() == false case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, missed that. Thanks.

case 'K_RALT':
if(keymanweb.isChiral()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left Alt As per Left Control above (drop through behaviour)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as mentioned for the Control case.

case 'K_LALT':
if(keymanweb.isChiral()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of both K_LMENU and K_LALT concerning, as with Ctrl key

if(layerId.indexOf('ctrl') >= 0) modifier += 2;
if(layerId.indexOf('alt') >= 0) modifier += 4;
if(layerId.indexOf('shift') >= 0) {
modifier += osk.modifierCodes["SHIFT"];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use |= to clarify that these are bitmasks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do. Thought the original was a bit funny, but when in doubt I try to perturb the original style as little as possible.

continue;
}

keys[i]['sp'] = osk._stateKeys[states[i]] ? '2' : '1';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man I'd love to use constants for the 'sp' values

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be arranged. There's a clear definition array in osk.setButtonClass that can be extracted into a set of constants.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, numbers-in-strings ftw but let's not fix that today.


// There must always be at least a plain 'default' layer.
if(typeof KLS['default'] == 'undefined' || ! KLS['default']) {
KLS['default'] = Array.from(''.repeat(65));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does Array.from(''.repeat(65)) do? AFAICT it gives an empty array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. An array of 65 empty strings, corresponding to one empty layer of 65 keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite. The expansion:

var foo=''.repeat(65);  // foo there becomes 65*'' == ''.
Array.from(foo);        // result is therefore empty array

Sadly also String.prototype.repeat is not supported in IE. Polyfill is probably overkill for this single use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to add insult to injury, IE doesn't support Array.from either. Sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stand corrected. I figured out why the code still works regardless and pointed it toward that instead for now, with a comment.


// Clone the default layout object for this device
layout=util.deepCopy(dfltLayout[layoutType]);

var n,layers=layout['layer'],keyLabels=PVK['BK'],key102=PVK['K102'];
var i,j,k,m,row,rows,key,keys;
var kbdBitmask = PVK['KMBM'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this same code elsewhere. Can we refactor into a function getKeyboardModifierBitmask()?

layer.shiftKey=shiftKey;
layer.capsKey=capsKey;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding Caps, Num and Scroll Keys? What do they do given they can't affect physical key state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the properties is to facilitate their OSK feedback. The base modifiers are selectively highlighted based upon the layer on which they appear, so they never needed to be tracked for any special purpose. The state keys, however, don't have their own layer. If we want to say a given state key is toggled, we need a way to forcibly toggle it for whatever layer is presently displayed. See line 1744 for their use in this matter.

Note that toggling a standard modifier within the OSK similarly cannot affect physical key state - it never has within KeymanWeb. The only difference is that the modifier keys have corresponding layers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure I'm following you. Are you saying that these state keys on the OSK can be toggled by the user, or are they purely for visual feedback?

Side note: the difficulty with state keys is that the user will expect their, um, state, to persist across keyboard changes. So if they press Caps Lock on the OSK, they won't anticipate Caps Lock being off once they click in the Address Bar, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be "toggled" (simulated) for OSK input. This does not affect the true key state whatsoever, and any physical keyboard input will instantly reset the state. This reflects how the modifiers are presently handled.

Copy link
Contributor Author

@jahorton jahorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I went through and tried to address the comments as best I could, though there are a few questions I don't really have answers for at this time.

case 'K_RCTRL':
if(keymanweb.isChiral()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will only drop through if keymanweb.isChiral() == false - note the break in the conditionals above. The fall-through is deliberate for the isChiral() == false case.

case 'K_RALT':
if(keymanweb.isChiral()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as mentioned for the Control case.

case 'K_LCTRL':
if(keymanweb.isChiral()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty curious too, but didn't want to rock the boat there. Some of it may have to do with layouts and/or cross-platform compatibility, I'm guessing.

@@ -891,6 +971,12 @@ if(!window['tavultesoft']['keymanweb']['initialized']) {
case 'K_SPACE':
keymanweb.KO(0, Lelem, ' ');
break;
case 'K_CAPS':
case 'K_NUMLOCK':
case 'K_SCROLL':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a definition for K_SCROLLLOCK in the osk.keyCodes array. Should it exist? I looked for any variations when writing this, but the lack of its definition caused me to not consider it.

if(layerId.indexOf('ctrl') >= 0) modifier += 2;
if(layerId.indexOf('alt') >= 0) modifier += 4;
if(layerId.indexOf('shift') >= 0) {
modifier += osk.modifierCodes["SHIFT"];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do. Thought the original was a bit funny, but when in doubt I try to perturb the original style as little as possible.

continue;
}

keys[i]['sp'] = osk._stateKeys[states[i]] ? '2' : '1';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be arranged. There's a clear definition array in osk.setButtonClass that can be extracted into a set of constants.

layer.shiftKey=shiftKey;
layer.capsKey=capsKey;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the properties is to facilitate their OSK feedback. The base modifiers are selectively highlighted based upon the layer on which they appear, so they never needed to be tracked for any special purpose. The state keys, however, don't have their own layer. If we want to say a given state key is toggled, we need a way to forcibly toggle it for whatever layer is presently displayed. See line 1744 for their use in this matter.

Note that toggling a standard modifier within the OSK similarly cannot affect physical key state - it never has within KeymanWeb. The only difference is that the modifier keys have corresponding layers.

… review.

There's still a bit of work to do on supporting proper CAPS_LOCK state upon element refocusing, as with switching tabs.
…ing is out of focus.

Includes some commented-out prelim work on AltGr simulation... it's proving very tricky.
…operly on... actual European keyboards when the AltGr key is pressed. *sigh*
@jahorton jahorton changed the title [WIP] [Web] Implementation of modifier chirality [Web] Implementation of modifier chirality Sep 14, 2017
darcywong00 added a commit that referenced this pull request Sep 15, 2017
This is follow-on to #300 for adding chirality modifiers to KMEA.

Add support for L/R Ctrl, Alt, caps lock, num lock, and scroll lock for external keyboard

- temporarily adding test chriality.js keyboard
darcywong00 added a commit that referenced this pull request Sep 15, 2017
This is follow-on to #300 for adding chirality modifiers to KMEA.

Add support for L/R Ctrl, Alt, caps lock, num lock, and scroll lock for external keyboard

- temporarily adding test chriality.js keyboard
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a regression with the Swedish - EuroLatin2 keyboard on the local test page

Typing these keys: 2345678;' gives the shifted keys @#$%^&*:"

In general, maybe of the key combos seem "shifted" somehow. =c is supposed to generate ɔ but is yielding Ɔ

The other English keyboard and English - English keyboards work fine.

Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More info re: EuroLatin2

@@ -290,13 +290,21 @@ if(!window['tavultesoft']['keymanweb']['initialized']) {
var retVal = 0; // I3318
var keyCode = (e.Lcode == 173 ? 189 : e.Lcode); //I3555 (Firefox hyphen issue)

var bitmask;
if(keymanweb._ActiveKeyboard['KV']) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reduced the EuroLatin2 keyboard to these 2-lines

store(controls) "@"
+ any(controls) > index(controls, 1) dk(1)

Compiles to this

        if (k.KKM(e, 16400, 50)) {
            r = m = 1;
            k.KO(0, t, "@");
            k.KDO(-1, t, 0);
        }

Typing a 2 drops into this statement and outputs @. Maybe something in keymanweb.KKM is incomplete for deadkeys?

Copy link
Contributor

@darcywong00 darcywong00 Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if you need to initialize
var bitmask = osk.modifierBitmasks.NON_CHIRAL;

That seems to fix masking Lruleshift & bitmask and makes EuroLatin2 work for me. (bitmask no longer undefined)

@mcdurdin mcdurdin modified the milestones: P2S4, P2S5 Sep 24, 2017
}
};

var attachType = GetURLParameter("mode");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lines involving attach... seem to be from samples/attachment-api, and can probably be removed from the chirality test page. (simplify)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the dangers of CTRL+C CTRL+V.

darcywong00 added a commit that referenced this pull request Sep 25, 2017
This is follow-on to #300 for adding chirality modifiers to KMEA.

Add support for L/R Ctrl, Alt, caps lock, num lock, and scroll lock for external keyboard

- temporarily adding test chriality.js keyboard
s = (s.length > 0 ? s + '-' : '') + 'alt';
}
return s;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, do we support Caps and NCaps layers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caps and NCaps layers are not default; they'd have to be implemented for Web by keyboard designers. Their visual feedback is managed directly by toggling the OSK keys' visual properties.

@jahorton jahorton merged commit 6b8963b into master Oct 4, 2017
@jahorton
Copy link
Contributor Author

jahorton commented Oct 4, 2017

Remaining issues from the checklist have been moved to #340.

darcywong00 added a commit that referenced this pull request Oct 4, 2017
This is follow-on to #300 for adding chirality modifiers to KMEA.

Add support for L/R Ctrl, Alt, caps lock, num lock, and scroll lock for external keyboard

- temporarily adding test chriality.js keyboard
@jahorton jahorton deleted the web-9-modifier-chirality branch October 13, 2017 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants