-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Conversation
User Test ResultsTest specification and instructions
Test Artifacts
|
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.) |
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: At iPhone SE resolution: 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: The banner above is not scrollable; there are only two selectable options. (The excess separators are also hidden.) 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. |
web/src/engine/main/osk/banner.ts
Outdated
let optionFormat: OptionFormatSpec = { | ||
paddingWidth: textLeftPad.val + textRightPad.val, // Assumes fixed px padding. | ||
emSize: emSize, | ||
styleForFont: fontStyle, | ||
collapsedWidth: targetWidth, | ||
minWidth: 0, | ||
} |
There was a problem hiding this comment.
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
(nocollapsedWidth
- so all suggestions got full length)collapsedWidth: targetWidth, minWidth: targetWidth
(maintains the original banner's formatting)
web/src/engine/main/osk/banner.ts
Outdated
// TODO: finalize + document | ||
interface OptionFormatSpec { | ||
minWidth?: number; | ||
paddingWidth: number, | ||
emSize: number, | ||
styleForFont: CSSStyleDeclaration | ||
|
||
collapsedWidth?: number | ||
} | ||
|
There was a problem hiding this comment.
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.
Some notes toward further work: While modern browsers 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.
|
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:
|
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: 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: 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: Also... When suggestions run long enough that all require "collapsing", three will display, with the rest available via scrolling. Of course, there are definitely in-between states. 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. |
Notable bugs when experimenting on a real device: 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. |
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. |
Test Results
|
Error in the JS console:
Since the bad part's near the end...
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. |
@keymanapp-test-bot retest TEST_LOW_RES_EMULATION I've fixed the model-linkage issue for the testing page; you should get suggestions now. |
Changes in this pull request will be available for download in Keyman version 17.0.249-alpha |
Fixes #9371.
This PR adds the following features to the predictive-text suggestion banner:
Examples for points 4 and 5 may be found in this comment below.
One example (drop-down)
So, what does this look like in practice?
Under a super-low screen resolution (drop-down)
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)
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:
Upon pressing down on one:
And with a little scrolling in the mix (with a different 'selection'):
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.
sil_euro_latin
as the keyboard, here are some useful word roots to experiment with:TEST_ANDROID_RTL: Using the Android app on a mobile device - preferably one with low resolution - test the banner to ensure it works properly.
balochi_phonetic
as the keyboard, play with the banner and report if anything seems "wrong" with the banner.اِسراییلیانی
a
v
s
r
a
y
y
l
y
a
n
y
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.
Personal testing notes
Some of the ways I've played / tested the banner myself:
other things to test:
border / becoming cropped