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

chore(ios): remove dead Swift-side keyboard gesture code #11672

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jun 4, 2024

This PR removes what is essentially dead code.

While working on a different PR this sprint, I remembered that way back in the past, with #2959, we changed the iOS keyboard engine to use the Web-based subkey menu. At the time, we hoped that what we saw as a bug would be fixed and that we'd be able to reverse the decision. A lot has changed since then, and we're now fully committed to handling all keyboard gestures fully on the JS side, within the WebView... so there's no longer any real reason to keep this code around.

Note that the Swift-side gesture handlers actually are still connected... but the functionality they exist to facilitate never utilizes them.

I originally thought this might be related to #11650, but it turned out to have a different underlying cause. Still... being able to delete over 800 lines of code is pretty significant.

User Testing

TEST_GENERAL_USE: do some general testing of the Keyman app for iOS and its system keyboard; things should work the same as usual.

@jahorton jahorton added this to the A18S3 milestone Jun 4, 2024
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jun 4, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jun 4, 2024

User Test Results

Test specification and instructions

  • TEST_GENERAL_USE (PASSED): I tested this issue with the attached "Keyman 18.0.48-alpha-test-11672" build on an iOS mobile(physical device) environment: Here is my observation. (notes)

Test Artifacts

@jahorton jahorton changed the base branch from master to chore/ios/enable-webview-debugging June 4, 2024 02:32
@jahorton jahorton force-pushed the chore/ios/remove-dead-gesture-code branch from 708fe8a to 105de3b Compare June 4, 2024 02:35
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test 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 Jun 6, 2024
@jahorton jahorton marked this pull request as ready for review June 6, 2024 01:57
@jahorton jahorton requested a review from sgschantz as a code owner June 6, 2024 01:57
Base automatically changed from chore/ios/enable-webview-debugging to master June 6, 2024 02:15
@dinakaranr
Copy link

Test Results

  • TEST_GENERAL_USE (PASSED): I tested this issue with the attached "Keyman 18.0.48-alpha-test-11672" build on an iOS mobile(physical device) environment: Here is my observation.
  1. Installed the "Keyman 18.0.47.apk" file and gave all permissions to the application.
  2. Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
  3. Open the Keyman app.
  4. Used various gesture features( "long press,"  "flick," and "multitap")

I tested the gesture features, such as "long press,"  "flick," and "multitap". I can hold the key and select a letter from the submenu. Caplock is enabled after pressing the "shift" key multiple times. "Long press" and "Multitap" are working fine. Flick is working when you slide down the key. It works well. Thank you.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jun 6, 2024
@jahorton jahorton merged commit 2c2815b into master Jun 7, 2024
5 checks passed
@jahorton jahorton deleted the chore/ios/remove-dead-gesture-code branch June 7, 2024 06:44
@keyman-server
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants