feat(web): introduction of flicks 🐵#9909
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
mcdurdin
left a comment
There was a problem hiding this comment.
LGTM apart from minor niggles
| const TRIGGER_DIST = this.gestureParams.flick.triggerDist; | ||
| const baseDist = Math.min(TRIGGER_DIST, pathStats.netDistance); | ||
|
|
||
| // The 1.001: makes the opposite direction ever-so-slightly less likely than the base key. |
There was a problem hiding this comment.
This comment may belong somewhere else now?
| } | ||
| } | ||
|
|
||
| export function flickStartContactModel(params: GestureParams): ContactModel { |
There was a problem hiding this comment.
| export function flickStartContactModel(params: GestureParams): ContactModel { | |
| export function FlickStartContactModel(params: GestureParams): ContactModel { |
I am not actually sure which direction you want the casing to go because it looks like there are existing inconsistencies in this file. Fixup in the direction you think is best!
There was a problem hiding this comment.
Lowercase: it's configured by a function.
Uppercase: it's a statically-defined object with no configuration of any sort.
There was a problem hiding this comment.
OK, that's a little obtuse 😁 BasicLongpressContactModel also doesn't seem to fit?
| } | ||
| } | ||
|
|
||
| export function flickEndContactModel(params: GestureParams): ContactModel { |
There was a problem hiding this comment.
| export function flickEndContactModel(params: GestureParams): ContactModel { | |
| export function FlickEndContactModel(params: GestureParams): ContactModel { |
| } | ||
| } | ||
|
|
||
| export function flickStartModel(params: GestureParams): GestureModel<any> { |
There was a problem hiding this comment.
| export function flickStartModel(params: GestureParams): GestureModel<any> { | |
| export function FlickStartModel(params: GestureParams): GestureModel<any> { |
| } | ||
| } | ||
|
|
||
| export function flickEndModel(params: GestureParams): GestureModel<any> { |
There was a problem hiding this comment.
| export function flickEndModel(params: GestureParams): GestureModel<any> { | |
| export function FlickEndModel(params: GestureParams): GestureModel<any> { |
| this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight; | ||
| this.gestureParams.flick.startDist = 0.25 * this.currentLayer.rowHeight; | ||
| this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; |
There was a problem hiding this comment.
Should these numbers be shifted into constants somewhere?
Addresses the primary remaining concern of #5029 - the flick gesture.
Note that they don't "feel great" at this stage in development. The key-preview system cuts off quickly during the flick, giving the feeling that "something's off", even though the flicks do work. That'll be addressed in a followup.
It'd probably be wise to make some sort of keyboard to better test the correction-distribution aspect - with actual 'base letters' as flicks, since that would actually show up. Haven't actually done that yet.
User testing will be requested on a follow-up, as key previews are pretty important for getting the feel of flicks right.
@keymanapp-test-bot skip