search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
372 stars 102 forks source link

fix(web): key tip constraint logic requires .bottom CSS #6784

Closed jahorton closed 1 year ago

jahorton commented 1 year ago

Fixes #6783.

Key tips should properly remain within the OSK's bounds again after this fix.

Screen Shot 2022-06-16 at 10 50 57 AM

As this is a top-row key preview, note that its base key is completely obscured by the preview.

This fixes a regression introduced in #6383. (Gotta test the top row without predictive-text!)

User Testing

These tests are derived from those of #6383, but they're a bit more precise and targeted than before.

SUITE_1: no predictive text

Test environments

In-app groups:

In-browser groups:

Tests

SUITE_2: predictive text active

This is only written as a separate suite because the predictive-text test can't be done with the (same) in-browser Web test page.

Test environments

Test

keymanapp-test-bot[bot] commented 1 year ago

User Test Results

Test specification and instructions

✅ SUITE_1: no predictive text

24 tests in 6 groups PASSED * ✅ GROUP_IPHONE_13_APP: - use the Keyman app with a (simulated or real) iPhone 13.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157337731))**: Tested Keyman 16.0.13 alpha-6784 build in iOS 15.0 / iPhone 13 along with Tamil99 Keyboard (ekwtami199uni), and it seems that the key preview is uncropped. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157337731)) - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157635353))**: Retested with the updated Keyman 16.0.13-alpha build in iOS 15.0 / iPhone 13 Simulator along with Tamil99 Keyboard and it seems to be working fine. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157635353)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157337731))**: Tested Keyman 16.0.13 alpha-6784 build in iOS 15.0 / iPhone 13 along with Cherokee Nation (SIL) Keyboard (sil_cherokee_nation) and it seems that the key preview is uncropped. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157337731)) - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157635353))**: Retested with the updated Keyman 16.0.13-alpha build in iOS 15.0 / iPhone 13 Simulator along with Cherokee Keyboard and it seems to be working fine. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157635353))
* ✅ GROUP_IPHONE_SE_2_APP: - use the Keyman app with a (simulated or real) iPhone SE 2nd gen.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157562815))**: Tested Keyman 16.0.13 alpha-6784 in iOS 15.0 / iPhone 2nd Gen Simulator along with Tamil99 keyboard and it seems to be working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157562815)) - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1160116497))**: Retested this in iOS 14.4 / iPhone SE 2nd gen Simulator and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1160116497)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157562815))**: Tested Keyman 16.0.13 alpha-6784 in iOS 15.0 / iPhone 2nd Gen Simulator along with Cherokee_SIL keyboard and it seems to be working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157562815)) - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1160116497))**: Retested this in iOS 14.4 / iPhone SE 2nd gen Simulator and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1160116497))
* ✅ GROUP_ANDROID_APP: - use the Keyman app with a (simulated or real) Android device - your pick.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100))**: Tested Keyman 16.0.13 alpha-6784 build in API 29 / Android 10 Simulator along with Tamil99 Keyboard (ekwtami199uni), and it seems that the key preview is uncropped. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100)) - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100))**: There is no gap between the Key preview's bottm and the bottom of its corresponding key. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100))**: Tested Keyman 16.0.13 alpha-6784 build in API 29 / Android 10 Simulator along with Cherokee Nation (SIL) Keyboard (sil_cherokee_nation) and it seems that the key preview is uncropped. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100)) - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100))**: There is not gap between the Key preview's bottom and the bottom of its corresponding key. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157376100))
* ✅ GROUP_IPHONE_13_BROWSER: - use Safari on a (simulated or real) iPhone 13.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139))**: Tested this in iOS 15.0 / iPhone 13 Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139))**: Tested this in iOS 15.0 / iPhone 13 Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139))**: Tested this in iOS 15.0 / iPhone 13 Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139))**: Tested this in iOS 15.0 / iPhone 13 Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157674139))
* ✅ GROUP_IPHONE_SE_2_BROWSER: - use Safari on a (simulated or real) iPhone SE 2nd gen.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517))**: Tested this in iOS 15.0 / iPhone SE 2nd gen Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517))**: Tested this in iOS 15.0 / iPhone SE 2nd gen Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517))**: Tested this in iOS 15.0 / iPhone SE 2nd gen Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517))**: Tested this in iOS 15.0 / iPhone SE 2nd gen Simulator in Test unminified Keymanweb" Web test page using Safari Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157687517))
* ✅ GROUP_ANDROID_BROWSER: - use Chrome on a (simulated or real) Android device - your pick.
4 tests PASSED - ✅ **TEST_TAMIL_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187))**: Retested this in API 29 / Android 10 Simulator in Test unminified Keymanweb" Web test page using Chrome Browser and it is working as expected. - ✅ **TEST_TAMIL_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187))**: Retested this in API 29 / Android 10 Simulator in Test unminified Keymanweb" Web test page using Chrome Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187)) - ✅ **TEST_CHEROKEE_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187))**: Retested this in API 29 / Android 10 Simulator in Test unminified Keymanweb" Web test page using Chrome Browser and it is working as expected. - ✅ **TEST_CHEROKEE_NORMAL_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187))**: Retested this in API 29 / Android 10 Simulator in Test unminified Keymanweb" Web test page using Chrome Browser and it is working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1158582187))

✅ SUITE_2: predictive text active

3 tests in 3 groups PASSED * ✅ GROUP_IPHONE_13_APP: - use the Keyman app with a (simulated or real) iPhone 13.
1 tests PASSED - ✅ **TEST_EUROLATIN_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157746123))**: Tested in Keyman 16.0.13 alpha build in iOS 15.0 / iPhone 13 Simulator and it is working as expected.
* ✅ GROUP_IPHONE_SE_2_APP: - use the Keyman app with a (simulated or real) iPhone SE 2nd gen.
1 tests PASSED - ✅ **TEST_EUROLATIN_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157746123))**: Tested in Keyman 16.0.13 alpha build in iOS 15.0 / SE 2nd gen Simulator and it is working as expected.
* ✅ GROUP_ANDROID_APP: - use the Keyman app with a (simulated or real) Android device - your pick.
1 tests PASSED - ✅ **TEST_EUROLATIN_TOP_PREVIEW ([PASSED](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157584245))**: Tested in Keyman 16.0.13 alpha test-6784 in API 29 / Android 10 Simulator and it seems to be working as expected. ([notes](https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157584245))

Test Artifacts

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_13_APP: - use the Keyman app with a (simulated or real) iPhone 13.

jahorton commented 1 year ago

... wherein we gain yet another case of cleanup-mode "oh, I can just remove this one extra line I threw in during development" turning out to be a "bad idea."

You can see that the line originally existed there before #6383 in its changelog; turns out there actually was a reason for it.

SUITE_1 GROUP_IPHONE_13_APP TEST_TAMIL_NORMAL_PREVIEW OPEN TEST_CHEROKEE_NORMAL_PREVIEW OPEN

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_ANDROID_APP: - use the Keyman app with a (simulated or real) Android device - your pick.

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_SE_2_APP: - use the Keyman app with a (simulated or real) iPhone SE 2nd gen.

bharanidharanj commented 1 year ago

SUITE_2: predictive text active

GROUP_ANDROID_APP: - use the Keyman app with a (simulated or real) Android device - your pick.

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_13_APP: - use the Keyman app with a (simulated or real) iPhone 13.

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_13_BROWSER: - use Safari on a (simulated or real) iPhone 13.

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_SE_2_BROWSER: - use Safari on a (simulated or real) iPhone SE 2nd gen.

bharanidharanj commented 1 year ago

SUITE_2: predictive text active

GROUP_IPHONE_13_APP: - use the Keyman app with a (simulated or real) iPhone 13.

GROUP_IPHONE_SE_2_APP: - use the Keyman app with a (simulated or real) iPhone SE 2nd gen.

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_ANDROID_BROWSER: - use Chrome on a (simulated or real) Android device - your pick.

jahorton commented 1 year ago

:facepalm: Silly me forgot to change up the group names for in-app vs browser tests.

Guess I'll patch that up manually...

jahorton commented 1 year ago

So... when the keytip got redone, apparently the 'cap' - the popup-part below - didn't actually get any styling in kmwosk.css, unlike the iOS version. Without it, we get an invisible one. Simple enough to fix; patch coming.

Stuff like this is why I didn't pre-build a 🍒 version; it's rougher to update a pre-built one than to just make it right on the first go.

jahorton commented 1 year ago

@keymanapp-test-bot retest SUITE_1 GROUP_ANDROID_BROWSER all

jahorton commented 1 year ago

OK... might have pulled the trigger a little too quickly on the Android key-tip bit. It definitely needs some extra styling work, so I'll need to figure out how to fix that up. Root cause was right; it's just that there's extra work beyond it that needs doing.

image

So, uh...

Test Results

SUITE_1: no predictive text

GROUP_ANDROID_BROWSER: - use Chrome on a (simulated or real) Android device - your pick.

jahorton commented 1 year ago

Okay, all better now.

image

@keymanapp-test-bot retest SUITE_1 GROUP_ANDROID_BROWSER all

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_ANDROID_BROWSER: - use Chrome on a (simulated or real) Android device - your pick.

jahorton commented 1 year ago

The 🍒 is up at #6795.

mcdurdin commented 1 year ago

I'm not 100% sure of the test results. https://github.com/keymanapp/keyman/pull/6784#issuecomment-1157562815 shows "PASSED" with screenshots that look clearly wrong to me, e.g. TEST_TAMIL_NORMAL_PREVIEW which has the following associated screenshot, annotated here:

image

I'll wait to review the #6795 until these tests are confirmed okay.

jahorton commented 1 year ago

Given @mcdurdin's note above...

SUITE_1

GROUP_IPHONE_SE_2_APP

TEST_TAMIL_NORMAL_PREVIEW: OPEN TEST_CHEROKEE_NORMAL_PREVIEW: OPEN

They still pass on my local machine via local build, but the attached screenshots, as noted, should not have been marked as PASSing. So, we should double-check that they work properly for our testers, too. (It shouldn't be related, but note that my local tests are targeting iOS 14.4, rather than iOS 15.x.)

image

image

bharanidharanj commented 1 year ago

SUITE_1: no predictive text

GROUP_IPHONE_SE_2_APP: - use the Keyman app with a (simulated or real) iPhone SE 2nd gen.

keyman-server commented 1 year ago

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