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

feat(oem/fv/ios): Dictionary Support #6197

Closed sgschantz closed 2 years ago

sgschantz commented 2 years ago

Adds dictionary/lexical model support to FirstVoices iOS app. Tapping a keyboard in the list causes transition to a detail page where the keyboard is displayed with other settings, including downloadable dictionaries, if available.

keyboard table

image

User Testing

The changes to the app are substantial and tests amount to a regression test

  1. Go to iOS Settings -> General -> Keyboard -> Keyboards -> Add New Keyboard... and choose FirstVoices under the heading Third-Party Keyboards.
  2. Go to the Messages app
  3. Tap in the text edit field to begin typing a new message so that the system keyboard will appear
  4. Tap on the globe icon
  5. Verify that the keyboard has changed to 'EuroLatin (SIL)', available due to the FirstVoices install
  1. Open the FirstVoices app and tap the 'here' button on the main instructions page
  2. Tap the first keyboard in the list which will probably be 'Inuvialuktun' -- tap anywhere but on the information icon
  3. This should cause a transition to the settings page for Inuvialuktun
  4. Tap the uppermost switch to turn on this keyboard
  5. Tap the back arrow in the upper left arrow to return to the keyboards page
  6. On the keyboards page, verify that Inuvialuktun now has a checkmark next to it
  7. Switch to messages and verify that the Inuvialuktun keyboard is available from the globe menu
  1. For any keyboard listed in the keyboards page, tap the information icon
  2. Verify that a web page loads with helpful information about the keyboard that was selected
  1. On the keyboards page, scroll down and tap SENĆOŦEN under the BC Coast region heading
  2. Tap the switch to activate the SENĆOŦEN keyboard
  3. Verify that a switch is now available to activate the dictionary 'SENĆOŦEN (Saanich Dialect)'
  4. Tap the switch and choose Install on the resulting confirmation dialog
  5. Verify that related dictionary settings Suggest Predictions and Suggest Corrections have defaulted to the On position
  6. Open Messages and after switching to the SENĆOŦEN keyboard, verify that words are suggested while typing image
  1. Make changes to various settings within the app, such as turning on certain keyboards and specifying different dictionary settings for the SENĆOŦEN keyboard
  2. Quit the app and restart it
  3. Verify that the previous settings are preserved
  1. On the keyboards page, scroll down and tap SENĆOŦEN under the BC Coast region heading
  2. When the keyboard switch is in the Off position, verify that all other switches are Off and disabled (cannot be toggled)
  3. Tap the 'SENĆOŦEN keyboard switch so that it is in the On position and verify that the 'SENĆOŦEN (Saanich Dialect)' switch is now enabled
  4. Tap the 'SENĆOŦEN (Saanich Dialect)' switch so that it is now in the On position and verify (after dictionary download) that the Suggest Predictions and Suggest Corrections switches automatically default to the On position
  5. Tap the Suggest Predictions switch to the off position and verify that Suggest Corrections also moves to the off position where it is now disabled
keymanapp-test-bot[bot] commented 2 years ago

User Test Results

Test specification and instructions

Results Template ``` # Test Results * **TEST_TURN_ON_KEYBOARD (OPEN):** notes * **TEST_INSTALL_DICTIONARY (OPEN):** notes ```

Test Artifacts

darcywong00 commented 2 years ago

UX question: Are the defaults on "Suggest Predictions" and "Suggest Corrections" off on Keyman for iPhone and iPad?

I think on Android, we default to "On" (less clicking after the user installs a lexical-model)

sgschantz commented 2 years ago

In Keyman for iOS, I installed a new keyboard, and the switches for Predictions and Corrections were active and enabled by default. That was even with no dictionary installed.

That doesn't make much sense as those options should not be available until there is a dictionary, right? After a dictionary is installed, we can activate them and default them as enabled. Until then they should be disabled and inactive.

sgschantz commented 2 years ago

A few notes on the details page:

  1. I labeled the switches Suggest Predictions and Suggest Corrections, using 'Suggest' instead of 'Enable' to try to be a little more descriptive. Switches always enable and disable things, so instead I wanted to describe what was being enabled or disabled. If there is a better word than 'suggest', let me know.
  2. After changing the enabling/graying out of controls, I think the Downloadable Dictionaries section should be moved up above the Suggest switches because the Suggest switches are dependent upon whether there is an active dictionary.
  3. Now we are calling the Suggest switches 'Language Settings' -- should they be Dictionary Settings instead? I think that would reinforce their purpose and make it more obvious why they are disabled when there is no dictionary.

The above changes differ from what we have in Keyman, but I think it would improve clarity of the UI.

As for how the state of the controls depend on each other, there is a pretty clear hierarchy:

  1. If the keyboard switch is off, then all other controls are off and disabled.
  2. If there is no dictionary downloaded or there is a dictionary and it is turned off, then both Suggest switches are off and disabled.
  3. If the Suggest Predictions switch is off, then the Suggest Corrections switch is off and disabled. (This is currently enforced in Keyman. Keyman also hides the switch, but that is not necessary.)
darcywong00 commented 2 years ago

iOS is using an old script to download fv_all.kmp https://github.com/keymanapp/keyman/blob/7e43e3782dce134dcfe0b69b09541898203773cb/oem/firstvoices/ios/build.sh#L9-L10

We can remove build_keyboards.sh if you include the following

. "$KEYMAN_ROOT/resources/build/build-download-resources.sh"

and then do https://github.com/keymanapp/keyman/blob/7e43e3782dce134dcfe0b69b09541898203773cb/oem/firstvoices/android/build.sh#L95-L101

MakaraSok commented 2 years ago

Test Results

Tested with the build retrieved from the link provided in the Test Results above.

Note: At one point, iMessage crashes when pressing on the FV globe key > Emoji key:

sgschantz commented 2 years ago

@MakaraSok I haven't been able to reproduce your trouble installing a keyboard, but I made one change to at least display an alert if it fails. Also, the switch should not change state now if the install fails. Please try again and let me know if the behavior is any different. Is FV Settings set to full access?

@mcdurdin What should I do with any error information that I get back from KeymanEngine so that we can investigate this? Without Sentry in FV, I'm not sure where to send it. Is there any place that I can retrieve error text when Makara encounters an issue? I could pass it back and display in this new alert, but that doesn't seem too user friendly.

mcdurdin commented 2 years ago

What should I do with any error information that I get back from KeymanEngine so that we can investigate this? Without Sentry in FV, I'm not sure where to send it. Is there any place that I can retrieve error text when Makara encounters an issue? I could pass it back and display in this new alert, but that doesn't seem too user friendly.

I think we aim to add Sentry to FV (I have created the fv-ios Sentry project at https://sentry.io/keyman/fv-ios/getting-started/apple-ios/). Otherwise, it is difficult, and I am certainly not keen on creating a new error-reporting mechanism for this.

If adding Sentry is too big a task for now, then we can temporarily add an alert while diagnosing these issues and then remove it again?

darcywong00 commented 2 years ago

Worth considering feeding the same changes to Keyman for Android

Yeah, we can consider that. I had a question on the SENCOTEN settings: "Keyboard Information" section. If there's multiple keyboards available for a language, which one would "Version" correspond to?

sgschantz commented 2 years ago

@MakaraSok I've added some code temporarily for troubleshooting the issue you are have activating the keyboard. This new version will display an alert with error information. Please retest and tell me what you see. I cannot reproduce this, so let me know if you have any more details that will isolate the issue.

@keymanapp-test-bot retest TEST_TURN_ON_KEYBOARD

sgschantz commented 2 years ago

@darcywong00 Good question on the version number. I think it's broken. We could display the version of the active dictionary if there is one, but that's kind of weak and still not obvious. I'm not sure what the best solution is.

mcdurdin commented 2 years ago

Good question on the version number. I think it's broken.

This was obviously designed way back in the day with a one-keyboard-per-language pattern. Do we have any languages with more than one keyboard in the list? If not, then this is a brokenness that we can just ignore for now :grin: If we do, then I suggest for now just adding a version row for each keyboard and differentiating with the keyboard name in the caption (e.g. SENCOTEN version -> 9.1.1).

The detail is of course important but we also need to balance that with addressing more serious issues... so I am comfortable with a less than 100% elegant answer for this edge case.

darcywong00 commented 2 years ago

Do we have any languages with more than one keyboard in the list?

Looking at keyboards.csv, I see "ikt-Latn: Inuinnuaqtun (Latin)" is used by fv_inuvialuktun and fv_uummartmitutun.

But I'm thinking this should be fine because the FV apps separate Region --> Keyboard -->

so each keyboard has its own row...

MakaraSok commented 2 years ago

Test Results

https://user-images.githubusercontent.com/28331388/158724337-44331887-0b15-45fb-be31-fcf34641cc96.mov

sgschantz commented 2 years ago

@mcdurdin So Makara is not seeing the alert that I added which implies that Keyman Engine is not throwing an error and it looks like a success to FV. Do you know of any other way to troubleshoot this in Makara's environment? @MakaraSok can you describe what environment you're running this in or provide any more info that might help me reproduce it?

mcdurdin commented 2 years ago

@mcdurdin So Makara is not seeing the alert that I added which implies that Keyman Engine is not throwing an error and it looks like a success to FV. Do you know of any other way to troubleshoot this in Makara's environment?

I guess running Console on his machine and looking at logs from the simulator may provide some enlightenment?

sgschantz commented 2 years ago

@MakaraSok @darcywong00 @mcdurdin I just realized that this problem Makara is seeing is the same as #6226 which appeared with the old FV iOS code. The change of Darcy's should not have broken the iOS FV app, should it? Or maybe that's why there was an iOS test case on #6226?

mcdurdin commented 2 years ago

I just realized that this problem Makara is seeing is the same as #6226 which appeared with the old FV iOS code. The change of Darcy's should not have broken the iOS FV app, should it? Or maybe that's why there was an iOS test case on #6226?

Yes, I think that this problem is not new to this PR and is specific to the simulator.

MakaraSok commented 2 years ago

Retest all failed/blocked

I've retested this again today and it doesn't work as expected like what was experienced prior. Note that the test was done on a simulator with its "all content and settings" erased being installing the .

https://user-images.githubusercontent.com/28331388/159197472-9e930029-c823-4024-94db-f73566fe8dff.mov

https://user-images.githubusercontent.com/28331388/159198672-6246bc9e-a291-4459-864d-c3bde4df96ad.mov

The second attempt to enable the dictionary got through without bouncing back, but it still doesn't show up when summoned. Please see the screen recording below.

https://user-images.githubusercontent.com/28331388/159199192-e3482bb9-e753-4d52-8003-5c0b3c6ca467.mov

The blocked ones stay "Blocked".

MakaraSok commented 2 years ago

FYI: The build from here was used https://build.palaso.org/buildConfiguration/Keyman_iOS_TestPullRequests/310924?buildTab=artifacts#%2Fupload.

sgschantz commented 2 years ago

Ready for retest, but need to have a way to either test a release version of FV on the simulator or a real device. Debug FV on the simulator works well from Xcode. @mcdurdin any ideas on how to change the relase build so that it will behave the same on simulator and device?

@keymanapp-test-bot retest TEST_TURN_ON_KEYBOARD TEST_INSTALL_DICTIONARY

mcdurdin commented 2 years ago

ny ideas on how to change the relase build so that it will behave the same on simulator and device?

It's really hard. The simulator does not appear to behave identically to devices in all circumstances -- particularly that boundary between apps and app extensions seems different.

The best solution may be to merge to beta and do a beta release build which is then available in TestFlight. Otherwise, unless you are in the same room (where you can plug the device into your computer and do a build and run from XCode), I don't know how it can be done.

keyman-server commented 2 years ago

Changes in this pull request will be available for download in Keyman version 15.0.224-beta

MakaraSok commented 2 years ago

Test Results

Tested on a physical device, iPad mini 6 running iOS 15.4.

Note that when removing a keyboard from the FV app, every keyboard was removed with it leaving only EuroLatin keyboard to choose from.

MakaraSok commented 2 years ago

Test Results