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

feat(web): Start of Sentence support - part 1 🕊 #5963

Merged
merged 18 commits into from Jan 17, 2022

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Nov 23, 2021

Relates to #3621.

  • Adds basic support for begin newContext and begin postKeystroke to compiler, including gn and gpk entry points.
  • Adds basic support for begin newContext to KeymanWeb, calling into gn entry point, for mobile, touch and embedded contexts.

TODO in this PR:

  • Compiler:
    • readonly semantics
    • usage constraints
    • &layerChanged system store tests
  • KeymanWeb:
    • begin postKeystroke support'
    • verify that postKeystroke fires after model events (fix(web): call postKeystroke on banner touch 🕊 #6004)
    • banner should be updated after newcontext, not before. (note: this may be an unrelated issue, as the banner does not currently respect layer selection for its hints.)

For follow-up pull requests:

Optional for v15:

  • KeymanWeb
    • Automated tests for start-of-sentence
  • Keyman Core
    • begin newContext, begin postKeystroke support

User Testing

SUITE_BASELINE: Ensure nothing is broken!

  • TEST_BASELINE: Run through a set of basic keyboard usage tests with SIL Eurolatin, Khmer Angkor, and other keyboards of your choice, to ensure that existing functionality has not changed. Focus on the touch keyboard interactions. Do this on Android and iOS (simulators okay), and KeymanWeb (unminified test page).

SUITE_START_OF_SENTENCE: Test start-of-sentence detection

This PR includes a simple English keyboard that should detect start of sentence, called start_of_sentence_3621.kmn.

start_of_sentence_3621.kmp

  • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).
  • It should drop back to the base layer after typing a single capital letter.
  • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.
  • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).
  • It should continue to allow the user to switch layers at any time.
  • It should not switch layers automatically apart from in these cases.

Test that the keyboard behaves as expected in text fields, when you move the cursor, when you switch between fields with and without text, and as you type.

  • TEST_SOS_IOS: Run the above tests in Keyman for iOS (simulator okay) - system keyboard and in-app
  • TEST_SOS_ANDROID: Run the above tests in Keyman for Android (virtual okay) - system keyboard and in-app
  • TEST_SOS_WEB: Run the above tests in KeymanWeb, Prediction UI testing page - on any touch platform (emulation in Chrome okay). Check that there are no script errors.

Relates to #3621.

Initial draft of keyboard to test start-of-sentence detection.
Relates to #3621.

This adds basic compiler support for the `begin newContext` and
`begin postKeystroke` statements, along with the corresponding `gn` and
`gpk` entry points in the compiled JavaScript keyboard.

TODO: `readonly` semantics, usage constraints, `&layerChanged`.
Relates to #3621.

Adds basic support for `begin newContext`.
@mcdurdin mcdurdin added this to the A15S18 milestone Nov 23, 2021
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Nov 23, 2021
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Nov 23, 2021

User Test Results

Test specification and instructions

✅ SUITE_BASELINE: Ensure nothing is broken!

  • TEST_BASELINE (PASSED): no significant issue experienced. The only issue is the different rendering of shift layers on iOS and Android. iOS rendered the layer nicely, unlike Android. See below. (notes)

🟥 SUITE_START_OF_SENTENCE: Test start-of-sentence detection

  • 🟥 TEST_SOS_IOS (FAILED): Tested with iOS 15.0. mostly (notes)
  • 🟥 TEST_SOS_ANDROID (FAILED): Longpress not working. (notes)
  • TEST_SOS_WEB (PASSED): Tested on Windows 10, Chrome Version 96.0.4664.93 (Official Build) (64-bit) (notes)

Test Artifacts

Relates to #3621.

This adds support for `begin postKeystroke` to KeymanWeb. This is now
close to final, apart from support for `&layerChanged`.
Relates to #3621.

Add support for `&layerChanged` system store. This store is set to `1`
before a `begin postKeystroke`, if the keystroke it follows resulted in
a layer change, either programatically through a keyboard rule, or
through a `nextlayer` property of the touched key.
Relates to #3621.

Adds `readonly` group support and verifies that keyboards meet
constraints around usage of these groups.

(Note: fixes test_valid.kps and test.bat which seemed to have some
invalid tests?)
mcdurdin added a commit that referenced this pull request Nov 25, 2021
Picked up in development of #5963.
Picked up in development of #5963.
mcdurdin added a commit that referenced this pull request Nov 25, 2021
…ature

Picked up during #5963 development.

If you ran `kmcomp <file.kmn> <out.js>`, compiler messages were passed
to a function with an incorrect signature, resulting in an exception.
This was a mode that was not widely used, which is why we haven't
picked it up earlier.
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Dec 9, 2021
@mcdurdin
Copy link
Member Author

mcdurdin commented Dec 9, 2021

SUITE_START_OF_SENTENCE

  • TEST_SOS_WEB: OPEN I've updated the instructions and fixed a link in the KeymanWeb test page; please retest 😁

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-required User tests have not been completed label Dec 9, 2021
@mcdurdin mcdurdin changed the title feat(web): Start of Sentence support - part 1 🍆 feat(web): Start of Sentence support - part 1 🕊 Dec 9, 2021
@mcdurdin
Copy link
Member Author

mcdurdin commented Dec 9, 2021

I'm very sorry 😭, I uploaded an old version of the start_of_sentence_3621.kmp package. I've updated the package, and tested it myself with good outcomes, so please retest with the new package.

SUITE_START_OF_SENTENCE

  • TEST_SOS_IOS: OPEN please retest
  • TEST_SOS_ANDROID: OPEN please retest

@MakaraSok
Copy link
Collaborator

SUITE_START_OF_SENTENCE: Test start-of-sentence detection

  • TEST_SOS_IOS: FAILED Tested with iOS 15.0. mostly
    • In-app

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        Not working! In iOS 15.0, there are two tabs audibly heard right after opening the Keyman app before the "Get Started" window appears. Click on "Done" to close the window to see the default layer of the keyboard instead of the shift layer. See the screenshot below.

        The shift layer appears only after tapping in the text area.

        Here is a screen recording for a better visual.

        Screen.Recording.2021-12-13.at.2.06.58.PM.mov

        In iOS 13.7, still hear the two taps, but no "Get Started" page is shown. The keyboard layout does show the shift layer right off.

      • It should drop back to the base layer after typing a single capital letter.

        Working!

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Not working! Pressing any key on the numeric layer moves the keyboard back to the default layer.

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Not working! The keyboard doesn't stay on shift layer after typing two capital letters in a row.

      • It should continue to allow the user to switch layers at any time.

        Working!

      • It should not switch layers automatically apart from in these cases.

        Working!

      Note that the default layer is shown when the Keyman app is opened from standby. It is expected to have shift layer shown instead.

      Screen.Recording.2021-12-13.at.1.07.16.PM.mov
    • As a system keyboard:

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        Working.

      • It should drop back to the base layer after typing a single capital letter.

        Working.

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Working.

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Working.

      • It should continue to allow the user to switch layers at any time.

        Working.

      • It should not switch layers automatically apart from in these cases.

        OK.

      After a line break (pressing Enter key), the shift layer should be shown, but it still stays in the default layer. Please consider adding this as a new functionality.

@MakaraSok
Copy link
Collaborator

SUITE_START_OF_SENTENCE: Test start-of-sentence detection

  • TEST_SOS_ANDROID: FAILED
    • In-app:

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        The shift layer does show up after closing the "Get Started" page, but the longpress keys are not working. The subkeys are not popped up.

        Screen.Recording.2021-12-13.at.3.26.19.PM.mov
      • It should drop back to the base layer after typing a single capital letter.

        Working as expected.

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Working as expected. Note also that the longpress keys on this layer are not working. Subkeys don't pop up!

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Working as expected.

      • It should continue to allow the user to switch layers at any time.

        Working as expected.

      • It should not switch layers automatically apart from in these cases.

        OK

    • As a system keyboard:

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        The shift layer does show up after closing the "Get Started" page, but the longpress keys are not working. The subkeys are not popped up.

        Screen.Recording.2021-12-13.at.3.39.19.PM.mov
      • It should drop back to the base layer after typing a single capital letter.

        Working as expected.

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Working as expected.

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Working as expected.

      • It should continue to allow the user to switch layers at any time.

        OK

      • It should not switch layers automatically apart from in these cases.

        OK

  • TEST_SOS_WEB: FAILED? Tested on Windows 10, Chrome Version 96.0.4664.93 (Official Build) (64-bit)
    • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

      Not working. At the start of the text field, the default (unshift) layer is shown.

      Here is what I see in the console:

    • It should drop back to the base layer after typing a single capital letter.

      Not working.

    • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

      Not verifiable as the is no numeric layer. In anyway, no auto-switch of keyboard layer at any point though.

    • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

      Not verifiable.

    • It should continue to allow the user to switch layers at any time.

      OK

    • It should not switch layers automatically apart from in these cases.

      OK

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Dec 13, 2021
@mcdurdin
Copy link
Member Author

The web test should be run with the correct keyboard, which is the start_of_sentence_3621 keyboard available in the Prediction UI test page. For the current build, that's https://build.palaso.org/repository/download/Keymanweb_TestPullRequests/300375:id/testing/prediction-ui/ The keyboard can be found under language "English...", "Start of Sentence test"

@keymanapp-test-bot retest SUITE_START_OF_SENTENCE TEST_SOS_WEB

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-required User tests have not been completed label Dec 17, 2021
@mcdurdin
Copy link
Member Author

  • [TEST_SOS_ANDROID] The shift layer does show up after closing the "Get Started" page, but the longpress keys are not working. The subkeys are not popped up.

@MakaraSok I am not able to reproduce this problem with the keyboard, after testing both on my device and on the emulator. Can you check again, and if you can reproduce it, perhaps we can look more closely at the difference between your test and mine?

@keymanapp-test-bot retest SUITE_START_OF_SENTENCE TEST_SOS_ANDROID

@MakaraSok
Copy link
Collaborator

SUITE_START_OF_SENTENCE: Test start-of-sentence detection

  • TEST_SOS_ANDROID: FAILED Longpress not working.

    • In-app:

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        OK, but the longpress keys are not working. The subkeys are not popped up.

      Screen.Recording.2021-12-20.at.9.10.52.AM.mov
      • It should drop back to the base layer after typing a single capital letter.

        Working as expected.

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Working as expected. Note also that the longpress keys on this layer are not working. Subkeys don't pop up!

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Working as expected.

      • It should continue to allow the user to switch layers at any time.

        Working as expected.

      • It should not switch layers automatically apart from in these cases.

        While on numeric layer, pressing the spacebar after a digit switch the keyboard layout to default.

    • As a system keyboard:

      • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

        The same behavior as in-app. The long press doesn't work either.

      Screen.Recording.2021-12-20.at.9.21.25.AM.mov
      • It should drop back to the base layer after typing a single capital letter.

        Working as expected.

      • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

        Working as expected.

      • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

        Working as expected.

      • It should continue to allow the user to switch layers at any time.

        OK

      • It should not switch layers automatically apart from in these cases.

        It switches to the default layer while on the numeric layer after pressing the spacebar after a digit.

  • TEST_SOS_WEB: PASSED Tested on Windows 10, Chrome Version 96.0.4664.93 (Official Build) (64-bit)

    • It should set the keyboard to the Shift layer at the start of a text field, after full stop (.), exclamation (!) or question mark (?) and one or spaces (e.g. . ).

      Working OK.

      I don't know what to look out for, but here is what I see:
      image

    • It should drop back to the base layer after typing a single capital letter.

      Working OK.

    • It should keep the keyboard in the numeric layer after typing a digit, but move back to base layer for all other symbols.

      Working OK.

    • It should keep the keyboard in the shift layer after typing two capital letters (note that this is a temporary testing behaviour until we support a CAPS layer in an upcoming PR).

      Working OK.

    • It should continue to allow the user to switch layers at any time.

      OK

    • It should not switch layers automatically apart from in these cases.

      On the numeric layer, press a space after a digit switches the keyboard layer to default.

@mcdurdin
Copy link
Member Author

Tracking user testing failures in two separate issues and will go ahead and merge this and the entire 🕊 start-of-sentence chain, given I think these two issues can probably be resolved in beta:

Note: I have managed to reproduce #6112, but have not reproduced #6113.

@mcdurdin mcdurdin merged commit 0c0bb42 into master Jan 17, 2022
@mcdurdin mcdurdin deleted the feat/web/3621-newcontext-group branch January 17, 2022 01:04
@keyman-server
Copy link
Collaborator

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

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

Successfully merging this pull request may close these issues.

None yet

4 participants