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): suggestion banner UI enhancements - suggestion expanding + scrollable banner #7934

Merged
merged 30 commits into from
Jan 22, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Dec 21, 2022

Fixes #9371.

This PR adds the following features to the predictive-text suggestion banner:

  1. The suggestion banner itself scrolls, rather than individual suggestions.
  2. Up to eight total suggestions are available via scrolling (if available).
    • Depending on context and suggestion availability, they may be hidden due to unavailability.
  3. Long suggestions start off in a "collapsed" state, but pressing down on one will expand it to display the full suggestion.
    • A "fade out" effect will be applied near the start of the word as an indicator of the "collapsed" state.
    • The "fade out" effect will be removed when the suggestion is expanded (no longer collapsed) and return when it collapses again.
  4. If a suggestion's display text is short, the suggestion is allotted less total width.
  5. If all suggestions are short (i.e, the sum width is less than that of the full banner width), padding will be added evenly to each to space them out nicely.

So, what does this look like in practice?

Under a super-low screen resolution (drop-down)

low-res-understanding

Note the initial position of the touch: when things seem "sticky" once the word understanding is selected. We believe the most natural interaction is to expect the suggestion to generally stay at the same place (when reasonable) underneath your finger. That's the part you originally saw before the touch started, so that part is sure to have already been visible; if the suggestion was forced / clamped into view, a different part you hadn't seen could end up underneath your finger instead. We want to ensure you have a chance to see that previously-hidden part by moving your finger during the clamped-movement state, as you would have been able to see it without issue if you touched the suggestion when it wasn't near screen borders.

The suggestion wants to stay at the same position underneath the touchpoint, but it wants to show itself initially even more so that you have as much info as possible. As the touchpoint is returned to the same initial position over the suggestion's word - the last n - the "sticky" aspect is progressively dropped.

At iPhone SE resolution (drop-down)

high-res-understanding

Examples

(from an older version of this PR, with less aggressive fading effects)

Note that these examples were derived from Chrome's mobile-device emulation mode, set to Repsonsive 200x400. (While not directly matching an existing device, this allows comparatively shorter words to overrun their boundaries within the banner.)

All suggestions collapsed:

image

Upon pressing down on one:

image

And with a little scrolling in the mix (with a different 'selection'):

image

User Testing

TEST_ANDROID_LTR: Using the Android app on a mobile device - preferably one with low resolution - test the banner to ensure it works properly.

  • Using sil_euro_latin as the keyboard, here are some useful word roots to experiment with:
    • roller... (rollercoaster is high frequency and long; others will be short)
      • rollerco... is generally enough to narrow the suggestions down to just two or three; they'll have an interesting layout.
    • interna... (international, internationally, etc)
    • circu... (circumstance, circumstances, etc)
    • extrao... (extraordinary, extraordinarily, etc)
    • relati... (relationship, relationships, etc)
    • commi... (commis... / commit... both have many long words)
    • specifications is long enough, but lacks other close, also-long entries
    • under... (understanding, understandable, etc)
      • has a LOT of good, long neighbors.
  • Report if anything seems "wrong" with the banner.

TEST_ANDROID_RTL: Using the Android app on a mobile device - preferably one with low resolution - test the banner to ensure it works properly.

  • Using balochi_phonetic as the keyboard, play with the banner and report if anything seems "wrong" with the banner.
  • Note: this is an RTL keyboard/model pair!
    • One comparatively long word I found in the lexical model: اِسراییلیانی
    • Corresponding US-layout keys: a v s r a y y l y a n y
  • Try to find something that leaves only one or two visible suggestions - report if anything seems "wrong". (The suggestion separators have acted "funny" here before.)

TEST_LOW_RES_EMULATION: Using Chrome mobile-device emulation and the "Predictive text - robust testing" Web testing page, test the banner at a low resolution to ensure it works properly.

  • Aim for the following settings:
    image
    • The 92% part can be ignored, but do aim for "Responsive" and "200 x 400".
  • This should make it far easier to test collapsing and expanding suggestions.
  • Report if anything seems "wrong" with the banner.

Personal testing notes

Some of the ways I've played / tested the banner myself:

  • ensure that the banner scrolls well and behaves 'nicely' at its borders
  • ensure that the new styling applies properly for both 'phone' and 'tablet' layouts for both mobile apps (Android, iOS)
    • Text fades out when touching borders
    • The "fade out" is clean - it fades to the suggestion's background color
    • The "fade out" does not affect text for an 'expanded' (highlighted) suggestion once related animations are complete.
  • behavior / stability when a second tap is received while a first tap is active
    • Highlighting might "stick" for a moment when lifting all fingers, but it should clear itself on the next interaction with the banner.

other things to test:

  • left-most option: attempt to scroll left
  • left-most option: properly padded
  • middle option in need of expansion: position the option near (but not on or past) left visible border, then expand it.
    • scroll left should not 'jitter'
    • whole thing should be visible
    • there may be clamping if you scroll the banner in the direction that the suggestion expanded.
      • This clamping should be removed if you move the touchpoint over the part of the word you originally touched.
  • middle option in need of expansion: does not expand past border
    • should not suffer 'clamping' effects when crossing visibility
      border / becoming cropped
  • middle option in need of expansion: position slightly/significantly overflowing left visible border, then expand it
    • scroll left should not 'jitter'
    • before scrolling, there should be no additional overflow; should have the net effect of expanding by sliding the suggestion right.
  • letting go of a suggestion that's not immediately accepted should not cause that suggestion / suggestion slot to "auto-complete" upon the next spacebar use! (This happened at one point.)

@jahorton jahorton added this to the A17S2 milestone Dec 21, 2022
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Dec 21, 2022
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Dec 21, 2022

User Test Results

Test specification and instructions

  • TEST_ANDROID_LTR (PASSED): Tested with the attached PR build (keyman 17.0.244-alpha-test-7934) in the Android Mobile (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Typing 'interna' showed the left part of the word 'international and internationally on the suggestion banner which are partially hidden (ie., fade out effect). 2. Pressing down on that word showed the full word 'international' or 'internationally'. 3. Tried with all the other words given in the user testing section, and they are working fine.
  • TEST_ANDROID_RTL (PASSED): Tested with the attached PR build (keyman 17.0.244-alpha-test-7934) in the Android emulator (Ver 5.0 / API 21) and here is my observation: 1. Installed balochi_phonetic keyboard. 2. Typed 'avsrayylyany' from the physical keyboard showed 'اِسراییلیانی' on the text input screen (appeared with 3 suggestions). 3. Tried with some other letters to see one or two visible suggestions on the banner and I succeeded. (notes)
  • TEST_LOW_RES_EMULATION (PASSED): Retested as per Josh's comments and here is my observation: 1. Created custom mobile device with low resolution. 2. Able to see the suggestions are still visible in the banner. 3. Able to scroll (from left to right or either direction) the predictive texts in the suggestion banner. 4. However, the text on the far left is cropped at the border. 5. When the leftmost suggestion is pressed and held for an extended period (fade-out text), the word is displayed in its entirety on the banner. 6. Tested with iPhone SE, iPad Mini and Android Mobile and Tablet emulators and it is working fine. (notes)

Test Artifacts

@jahorton
Copy link
Contributor Author

jahorton commented Dec 21, 2022

Notes from demo today: would be nice for suggestions not at the left of screen to hold position and have the banner correspondingly 'scroll left'. (Unless it's the left-most option, of course.)

Variable-width options would be nice, too. (Personal note: would likely be as a child PR.)

@jahorton
Copy link
Contributor Author

jahorton commented Dec 22, 2022

The new commit adds two new features (4 and 5 above).

In short, it adds initial support for variable-width suggestions on the banner along with some polish for cases where too few suggestions are available to cover the full banner width.

At 200x400:

image

At iPhone SE resolution:

image

Contrast "and" with "I" - "and" is clearly wider. The padding amounts are equal, though.

The banner is scrollable for neither; the suggestion are padded just enough to fill with width, but no further.


Alternate cases:

image

The banner above is not scrollable; there are only two selectable options. (The excess separators are also hidden.)

image

Words that are short enough use only as much space as needed, while overly long words are collapsed. Max width is set to 1/3 the banner's width, so "completely" is partially obscured. This banner scrolls, since additional suggestions are available past the margin.

Comment on lines 514 to 520
let optionFormat: OptionFormatSpec = {
paddingWidth: textLeftPad.val + textRightPad.val, // Assumes fixed px padding.
emSize: emSize,
styleForFont: fontStyle,
collapsedWidth: targetWidth,
minWidth: 0,
}
Copy link
Contributor Author

@jahorton jahorton Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tested with:

  • collapsedWidth: targetWidth, minWidth: targetWidth/2
  • minWidth: targetWidth (no collapsedWidth - so all suggestions got full length)
  • collapsedWidth: targetWidth, minWidth: targetWidth (maintains the original banner's formatting)

Comment on lines 177 to 186
// TODO: finalize + document
interface OptionFormatSpec {
minWidth?: number;
paddingWidth: number,
emSize: number,
styleForFont: CSSStyleDeclaration

collapsedWidth?: number
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously a significant TODO; see other comment in this grouping for example uses.

@jahorton
Copy link
Contributor Author

jahorton commented Dec 23, 2022

Some notes toward further work:

While modern browsers support 'transitionstart' and 'transitionend' events we could try to tap, those events aren't supported on some of the older devices we aim to support:

https://developer.mozilla.org/en-US/docs/Web/API/Element/transitionstart_event#browser_compatibility

Note: Chrome for Android @ v74, Safari on iOS @ 13.4.

Also, there's no related event nor event property I can find that would actually let us inspect the progress of the transition to ensure any, uh, "counter scrolling" on the banner would be perfectly synchronized.

Just in case, for clarity: "counter scrolling" here = scrolling the banner left exactly as much as the suggestion would "slide" right when expanding. One of the ideas we floated in design meetings was to keep the touchpoint stationary, but the option-expansion technique we're using requires expanding to the right; we'd need that counter scroll to offset the location change.

A better way forward, even though it has a higher up-front cost: refer to https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame#examples.

requestAnimationFrame is an older, more thoroughly supported method designed to facilitate transitions and animations from JS. Since we'd have to use this anyway for the banner's "counter scroll", we may as well use it to handle suggestion expanding & collapsing too. That would give us direct access to the state of the transition, which we a CSS-only implementation does not make available, which means we'd have the exact data needed to make a perfect "counter scroll."

@mcdurdin mcdurdin modified the milestones: A17S2, A17S3 Jan 1, 2023
@jahorton
Copy link
Contributor Author

jahorton commented Jan 6, 2023

After a lot of work + experimentation, I think I've finally gotten something nice working for offsetting scrolls to promote expanded option visibility. The logic seems in place for LTR scripts, but I've yet to do it for RTL ones. I'm quite happy with its current form, but it took way longer to get all of the details and polish right than I'd like to admit.

Newly-added effects:

  • when an option that is fully visible is expanded, an offset to the scroll will be progressively applied as necessary to prevent the option from being clipped by the border being expanded toward.
  • if the option is already partially obscured by the border that would be expanded toward, the already-clipped part will remain clipped, with a scroll offset progressively applied that has a net effect making the option appear to "expand" in the opposite direction, making the newly-added part of the option visible.
  • In both cases, once the option is fully visible (for the latter case above, after user scrolling), attempts to "scroll further past it" will be temporarily "sticky" until the touchpoint's original location on the option is restored, progressively removing scroll offset effects until zeroed out. They will no longer be applied for as long as the option is actively held.
  • Corresponding animations have been made as smooth as possible, though it's worth noting that there may be sub-pixel level adjustments due to .scrollLeft limitations, as .scrollLeft typically avoids sub-pixel values.
    • Dropping said adjustments results in a "jittery" animation that is far less ideal.

@jahorton
Copy link
Contributor Author

jahorton commented Jan 6, 2023

RTL scroll offsetting is now in place.

And, partly because I can't leave well enough alone, I've followed up by allowing variable-width suggestion functionality to interact with everything else.

A few highlights when using a highly-restricted screen resolution:

image

image

In both cases, only three suggestions were available, so the variable-width subsystem allocated width where it'd be needed most. That got split across both longer "butterfly" suggestions, while the "rollercoaster" suggestion was the only long one in its set, allowing it to hog the extra width. The "collapsed" suggestions are still expandable, and scrolling is possible while they're expanded.

As is already noticeable above, this facilitates narrowing the space allocated to short suggestions. For a more extreme case:

image

That's right... seven default suggestions fit in the banner in this highly-restrictive resolution! Since most devices have markedly more space available, I've upped the max number of suggestions to display up to 8:

image

Also...

image

When suggestions run long enough that all require "collapsing", three will display, with the rest available via scrolling.

image

image

Of course, there are definitely in-between states.

image

image


Admittedly, allowing suggestions to be narrowed significantly reduces the average amount of whitespace, so a little extra padding around the text of each suggestion may be in order.

@mcdurdin mcdurdin modified the milestones: A17S3, A17S4 Jan 7, 2023
@jahorton
Copy link
Contributor Author

jahorton commented Jan 9, 2023

Notable bugs when experimenting on a real device:

File (2)

Huh, that second option's acting really funny, though I didn't do any notable setup to trigger that scenario.

Highlighting it triggers the usual animations, which will start from a properly-collapsed state. This causes a "snapping" / "jumping" behavior because it's showing up expanded before selection (unlike the suggestion to its left) instead, and the animation start forces the implied jump.

If one suggestion is held, then another one tapped, the banner's state management kinda breaks; multi-tap will definitely need testing.

@darcywong00 darcywong00 modified the milestones: A17S4, A17S5 Jan 23, 2023
Base automatically changed from chore/web/ci-step-extraction to master February 2, 2023 02:20
@darcywong00 darcywong00 modified the milestones: A17S5, A17S6 Feb 6, 2023
@mcdurdin mcdurdin modified the milestones: A17S6, A17S7 Feb 18, 2023
@mcdurdin mcdurdin modified the milestones: A17S7, A17S8 Mar 6, 2023
@mcdurdin mcdurdin modified the milestones: A17S8, A17S9 Mar 18, 2023
@mcdurdin mcdurdin modified the milestones: A17S9, A17S10 Apr 4, 2023
@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 Jan 16, 2024
@github-actions github-actions bot added the user-test-missing User tests have not yet been defined for the PR label Jan 16, 2024
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jan 16, 2024
@jahorton
Copy link
Contributor Author

jahorton commented Jan 16, 2024

The last commit fixes something subtle I noticed in the current screen-recordings - the banner's scroll position wasn't being reset when a later suggestion (that required scrolling to reach) was selected.

Granted, this only matters if the screen resolution is so low that the default suggestion set overflows screen width; if it doesn't, that fact in itself will force a scroll-position reset.

@jahorton jahorton linked an issue Jan 17, 2024 that may be closed by this pull request
@bharanidharanj
Copy link

Test Results

  • TEST_ANDROID_LTR (PASSED): Tested with the attached PR build (keyman 17.0.244-alpha-test-7934) in the Android Mobile (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Typing 'interna' showed the left part of the word 'international and internationally on the suggestion banner which are partially hidden (ie., fade out effect). 2. Pressing down on that word showed the full word 'international' or 'internationally'. 3. Tried with all the other words given in the user testing section, and they are working fine.

@bharanidharanj
Copy link

Test Results

  • TEST_ANDROID_RTL (PASSED): Tested with the attached PR build (keyman 17.0.244-alpha-test-7934) in the Android emulator (Ver 5.0 / API 21) and here is my observation: 1. Installed balochi_phonetic keyboard. 2. Typed 'avsrayylyany' from the physical keyboard showed 'اِسراییلیانی' on the text input screen (appeared with 3 suggestions). 3. Tried with some other letters to see one or two visible suggestions on the banner and I succeeded.

..suggestion banner 1

..1 or 2 suggestions on the banner

@bharanidharanj
Copy link

Test Results

  • TEST_LOW_RES_EMULATION (FAILED): Tested using KeymanWeb testing page in the chrome emulation page and here is my observation: 1. Created the custom mobile device with a resolution "200x400". 2. Typed "comp" in the text area box and I noticed that there is no predictive text appearing in the suggestion banner. Seems to be an issue.

@keymanapp-test-bot keymanapp-test-bot bot added user-test-failed and removed user-test-required User tests have not been completed labels Jan 18, 2024
@jahorton
Copy link
Contributor Author

jahorton commented Jan 19, 2024

Test Results

  • TEST_LOW_RES_EMULATION (FAILED): Tested using KeymanWeb testing page in the chrome emulation page and here is my observation: 1. Created the custom mobile device with a resolution "200x400". 2. Typed "comp" in the text area box and I noticed that there is no predictive text appearing in the suggestion banner. Seems to be an issue.

Error in the JS console:

GET https://build.palaso.org/repository/download/Keymanweb_TestPullRequests/435384:id/src/test/manual/web/prediction-mtnt/index.htmlnrc.en.mtnt.model.js
    Status: 404

Since the bad part's near the end...

prediction-mtnt/index.htmlnrc.en.mtnt.model.js

Looks like the actual page's URL got mashed into the model's URL for some reason.

It appears a related change happened with #9953. Not sure why we'd have only caught this now, but at least it is caught now.

@jahorton
Copy link
Contributor Author

@keymanapp-test-bot retest TEST_LOW_RES_EMULATION

I've fixed the model-linkage issue for the testing page; you should get suggestions now.

@keymanapp-test-bot keymanapp-test-bot bot added user-test-required User tests have not been completed and removed user-test-failed labels Jan 19, 2024
@bharanidharanj
Copy link

Test Results

  • TEST_LOW_RES_EMULATION (PASSED): Retested as per Josh's comments and here is my observation: 1. Created custom mobile device with low resolution. 2. Able to see the suggestions are still visible in the banner. 3. Able to scroll (from left to right or either direction) the predictive texts in the suggestion banner. 4. However, the text on the far left is cropped at the border. 5. When the leftmost suggestion is pressed and held for an extended period (fade-out text), the word is displayed in its entirety on the banner. 6. Tested with iPhone SE, iPad Mini and Android Mobile and Tablet emulators and it is working fine.

..Android Mobile with custom resolution

..iPhone SE

..iPad Mini

..Android Mobile

..Android Tablet

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 19, 2024
@mcdurdin mcdurdin modified the milestones: A17S30, A17S31 Jan 20, 2024
@jahorton jahorton merged commit e00dbe8 into master Jan 22, 2024
14 of 15 checks passed
@jahorton jahorton deleted the feat/web/fancy-suggestion-banner branch January 22, 2024 01:17
@keyman-server
Copy link
Collaborator

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

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