feat(web): browser-KMW support for default subkeys by jahorton · Pull Request #9496 · keymanapp/keyman

jahorton · 2023-08-21T03:58:06Z

Building on the work from #9432, this adds support for default subkeys to KMW (and to the iOS app).
See also #9416.

The Android version of Keyman will require extra handling due to how it has special subkey handling of its own.

Note: most of the new changes are for the new manual testing page and its keyboard - I don't know of an existing keyboard resource with default subkeys yet, so I drafted one from scratch.

User Testing

TEST_SPECIAL_DEFAULT: if a subkey exists with an explicitly-marked 'default' subkey, it should be auto-selected when the subkey menu is first displayed.

On a mobile device (or when emulating one in a desktop browser), visit the testing page marked "Tests handling of new default-subkey feature (feat(developer): define long-press default key 🐵 #9430)."
Longpress the d key.
When the subkey menu is displayed, verify that the fifth subkey - "default" - is selected. The text may be cropped.
Without moving the touchpoint/finger, release the touchpoint / lift your finger. This should output the text default.
- Note: if significant movement occurs, the text will not be output. (A small "fudge factor" is allowed before this occurs.)

TEST_MATCHING_ID_DEFAULT: if a subkey exists with an ID perfectly matching its base key, it should be auto-selected in the absence of a specified subkey.

On a mobile device (or when emulating one in a desktop browser), visit the testing page marked "Tests handling of new default-subkey feature (feat(developer): define long-press default key 🐵 #9430)."
Longpress the a key.
When the subkey menu is displayed, verify that the third subkey - "a" - is selected.
Without moving the touchpoint/finger, release the touchpoint / lift your finger. This should output the text a.

TEST_IOS_SPECIAL_DEFAULT: the same as the first test above, but this time within the iOS app.

Using Safari, download and then install the default-subkey keyboard package: default_subkey.kmp
- I'm hosting this under my GitHub pages; it's the same link that can be found here: https://jahorton.github.io/
Within the iOS app, longpress the d key.
When the subkey menu is displayed, verify that the fifth subkey - "default" - is selected. The text may be cropped.
Without moving the touchpoint/finger, release the touchpoint / lift your finger. This should output the text default.
- Note: if significant movement occurs, the text will not be output. (A small "fudge factor" is allowed before this occurs.)

keymanapp-test-bot · 2023-08-21T03:58:25Z

User Test Results

Test specification and instructions

✅ TEST_SPECIAL_DEFAULT (PASSED): Tested with the given testing page in a desktop browser and here is my observation: 1. Longpress the d key and verified that the fifth subkey -"default" - is selected. And the text was cropped. 2. Without moving the touchpoint, I released the touchpoint and verified that the text 'default' appeared on the screen. Seems to be working fine. (notes)
✅ TEST_MATCHING_ID_DEFAULT (PASSED): Tested with the given testing page in a desktop browser and here is my observation: 1. Longpress the 'a' key and verified that the third subkey 'a' - is selected. 2. Without moving the touchpoint, I released the touchpoint and verified that the text 'a' appeared on the screen. Seems to be working fine. (notes)
✅ TEST_IOS_SPECIAL_DEFAULT (PASSED): Tested with the attached PR build (Keyman 17.0.181-alpha-test-9496) in iPhone 13 Pro Max / iOS 15.0 Simulator and here is my observation: 1. Installed the default_subkey.kmp file in the Keyman In-App. 2. Long press the d key. 2. Able to see that the 5th key appeared in blue color with the name 'default'. The 'default' name is cropped. 3. Release the button and verified that the name 'default' appeared in the input text screen. (notes)

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 17.0.181 (0.9496.9274)
Keyboards
- Test Keyboards
Web
- KeymanWeb Test Home
Windows

jahorton · 2023-08-21T05:33:01Z

common/web/keyboard-processor/src/keyboards/defaultLayouts.ts

  "pad"?: string | number,
  "sk"?: LayoutKey[]
+  "default"?: boolean
 }


For Android: this (mixed with ActiveKey, seen in the file above) is the type of the subkeys provided by Web to the oskCreatePopup function found in the keyboard-host page:

keyman/android/KMEA/app/src/main/assets/android-host.js

Lines 261 to 282 in 8ba1989

function oskCreatePopup(obj,x,y) {

if(obj != null) {

var i;

var s = '';

var shift = false;

var keyPos = x.toString() + ',' + y.toString();

for(i=0; i<obj.length; i++)

{

// elementID contains the layer and coreID

s=s+obj[i].elementID;

if(obj[i].sp == 1 || obj[i].sp == 2) shift = true;

if(typeof(obj[i].text) != 'undefined' && obj[i].text != null && obj[i].text != '') s=s+':'+toHex(obj[i].text);

if(i < (obj.length -1)) s=s+';'

}

fragmentToggle=(fragmentToggle+1) % 100;

var hash = 'showMore-' + fragmentToggle + '+keyPos=' + keyPos + '+keys=' + s;

if(shift) {

hash = hash + '+font=' + 'SpecialOSK';

}

window.location.hash = hash;

}

}

The obj parameter is an array of these objects. Currently, oskCreatePopup only references elementID (from ActiveKey) and text - so we'll need something to annotate a 'default' key when calling out from KMW into Android code.

(elementID is a mix of the layer ID for the layer hosting the base key, the key's ID [id], and the layer ID to use for any relevant key modifiers [layer].)

darcywong00 · 2023-08-21T06:29:40Z

web/src/test/manual/web/default-subkey/index.html

+      kmw.init({
+		    attachType:'auto'
+      }).then(function() {
+        kmw.addKeyboards({id:'default_subkey',name:'English',languages:{id:'en',name:'English'}, filename:'./default_subkey.js'});


Did you manually move ./build/default_subkey.js to this folder?
I see the .kpj and .kps compile to ./build/default_subkey.js so I wonder if the test page should just use that

I did. Keep in mind, our .gitignore settings omit anything within build folders; that's why I copied it.

I guess I could mark it as an exception in a local .gitignore?

darcywong00 · 2023-08-21T08:06:58Z

common/web/keyboard-processor/src/keyboards/defaultLayouts.ts

  "nextlayer"?: string,
  "pad"?: string | number,
  "sk"?: LayoutKey[]
+  "default"?: boolean


Is default just needed for ActiveKey? (wondering if LayoutKey and OSKKeySpec need it)

keyman/common/web/keyboard-processor/src/keyboards/activeLayout.ts

Line 23 in 909331d

export class ActiveKey implements LayoutKey {

keyman/web/src/engine/osk/src/keyboard-layout/oskKey.ts

Line 13 in 909331d

export class OSKKeySpec implements LayoutKey {

When loaded, the touch-layout spec within a keyboard is generally an instance of type LayoutKey with all the properties and methods of ActiveKey copy-pasted onto it.

As for OSKKeySpec, after checking, it appears to only ever be constructed when a default is needed. Otherwise, the LayoutKey is typecast to OSKKeySpec, and it works.

darcywong00

lgtm
Hopefully the browserstack fail is a flaky timeout?

jahorton · 2023-08-23T08:43:26Z

For the LMLayer tests: the macOS-Monterrey configuration failed to connect properly. That's all.

For the Web ones, it seems like the Android ones are the ones failing to connect properly. That's actually kind of concerning, since this PR's changes are most focused in that direction... but a local run of the automated tests passes flawlessly under Chrome's remote device emulation. And it's not unheard of for there to be stability issues with the Android target for our BrowserStack runs.

... except the latest Web run? Android ran fine. It was macOS-Monterrey.

Sounds like BrowserStack may just be having a bad day/week?

mcdurdin

Also LGTM. When the full gestures suite lands, it would be fantastic to have a demo of all the new functionality 😁

bharanidharanj · 2023-09-28T08:45:58Z

TEST_IOS_SPECIAL_DEFAULT (PASSED): Tested with the attached PR build (Keyman 17.0.181-alpha-test-9496) in iPhone 13 Pro Max / iOS 15.0 Simulator and here is my observation: 1. Installed the default_subkey.kmp file in the Keyman In-App. 2. Long press the d key. 2. Able to see that the 5th key appeared in blue color with the name 'default'. The 'default' name is cropped. 3. Release the button and verified that the name 'default' appeared in the input text screen.

..long press d key

..After releasing the d key the name default is displayed on the screen

bharanidharanj · 2023-09-28T14:17:14Z

TEST_SPECIAL_DEFAULT (PASSED): Tested with the given testing page in a desktop browser and here is my observation: 1. Longpress the d key and verified that the fifth subkey -"default" - is selected. And the text was cropped. 2. Without moving the touchpoint, I released the touchpoint and verified that the text 'default' appeared on the screen. Seems to be working fine.

bharanidharanj · 2023-09-28T14:22:21Z

Test Results

TEST_MATCHING_ID_DEFAULT (PASSED): Tested with the given testing page in a desktop browser and here is my observation: 1. Longpress the 'a' key and verified that the third subkey 'a' - is selected. 2. Without moving the touchpoint, I released the touchpoint and verified that the text 'a' appeared on the screen. Seems to be working fine.

keyman-server · 2023-09-29T18:02:56Z

Changes in this pull request will be available for download in Keyman version 17.0.183-alpha

jahorton added 2 commits August 21, 2023 10:42

feat(web): browser-KMW support for default subkeys

d488f22

docs(android): markers for sites to add default subkey handling code

dc033b9

jahorton added this to the A17S20 milestone Aug 21, 2023

keymanapp-test-bot bot added the has-user-test label Aug 21, 2023

github-actions bot added web/ android/ common/ android/engine/ common/web/ feat labels Aug 21, 2023

keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Aug 21, 2023

fix(web): stuck base-key highlighting upon default selection

3adf4dc

keymanapp-test-bot bot added user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Aug 21, 2023

jahorton added 2 commits August 21, 2023 12:16

fix(web): test resource package compilation

bf713d1

feat(web): allows small fudge-factor before cancelling default

909331d

jahorton commented Aug 21, 2023

View reviewed changes

jahorton marked this pull request as ready for review August 21, 2023 05:33

jahorton requested review from darcywong00, mcdurdin and sgschantz as code owners August 21, 2023 05:33

darcywong00 reviewed Aug 21, 2023

View reviewed changes

darcywong00 approved these changes Aug 21, 2023

View reviewed changes

darcywong00 mentioned this pull request Aug 28, 2023

feat: Consider long-press default key (i.e. when user long-presses but doesn't swipe) as dev-configurable 🐵 #9416

Closed

2 tasks

mcdurdin modified the milestones: A17S20, A17S21 Sep 1, 2023

mcdurdin approved these changes Sep 2, 2023

View reviewed changes

mcdurdin modified the milestones: A17S21, A17S22 Sep 15, 2023

mcdurdin added the m:osk On Screen Keyboard or Touch Keyboard, all platforms label Sep 16, 2023

chore(web): Merge branch 'master' into feat/web/default-subkeys-9430

05b4de3

github-actions bot added android/ android/engine/ and removed android/ android/engine/ labels Sep 26, 2023

fix(web): test page path on the index page

a5cbf6a

github-actions bot added android/ android/engine/ and removed android/ android/engine/ labels Sep 28, 2023

keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Sep 28, 2023

jahorton merged commit 4a4c759 into master Sep 29, 2023

jahorton deleted the feat/web/default-subkeys-9430 branch September 29, 2023 08:46

This was referenced Oct 9, 2023

feat(web): longpress restoration, enhancements 🐵 #9698

Merged

chore(web): master-merge for feature-gestures (mid A17S23) 🐵 #9729

Merged

	function oskCreatePopup(obj,x,y) {
	if(obj != null) {
	var i;
	var s = '';
	var shift = false;
	var keyPos = x.toString() + ',' + y.toString();
	for(i=0; i<obj.length; i++)
	{
	// elementID contains the layer and coreID
	s=s+obj[i].elementID;
	if(obj[i].sp == 1 \|\| obj[i].sp == 2) shift = true;
	if(typeof(obj[i].text) != 'undefined' && obj[i].text != null && obj[i].text != '') s=s+':'+toHex(obj[i].text);
	if(i < (obj.length -1)) s=s+';'
	}
	fragmentToggle=(fragmentToggle+1) % 100;
	var hash = 'showMore-' + fragmentToggle + '+keyPos=' + keyPos + '+keys=' + s;
	if(shift) {
	hash = hash + '+font=' + 'SpecialOSK';
	}
	window.location.hash = hash;
	}
	}

Uh oh!

Conversation

jahorton commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Testing

Uh oh!

keymanapp-test-bot bot commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Test Artifacts

Uh oh!

jahorton Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

darcywong00 Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

jahorton Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darcywong00 Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

jahorton Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

darcywong00 left a comment

Choose a reason for hiding this comment

Uh oh!

jahorton commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

bharanidharanj commented Sep 28, 2023

Uh oh!

bharanidharanj commented Sep 28, 2023

Uh oh!

bharanidharanj commented Sep 28, 2023

Test Results

Uh oh!

keyman-server commented Sep 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jahorton commented Aug 21, 2023 •

edited

Loading

keymanapp-test-bot bot commented Aug 21, 2023 •

edited

Loading

jahorton Aug 21, 2023 •

edited

Loading

jahorton commented Aug 23, 2023 •

edited

Loading