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

fix(web): longpress shortcut activation should only consider northward part #11306

Merged
merged 5 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions web/src/engine/osk/src/input/gestures/specsForLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,16 @@ export interface GestureParams<Item = any> {
*/
permitsFlick: (item?: Item) => boolean,

/**
* The minimum _net_ distance traveled before a longpress flick-shortcut will cancel any
* conflicting flick models.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be clear, it only does so if there are no actual northish flicks. (nw, n, ne)

*/
flickDistStart: number,

/**
* The minimum _net_ distance traveled before a longpress flick-shortcut will trigger.
*/
flickDist: number,
flickDistFinal: number,

/**
* The maximum amount of raw-distance movement allowed for a longpress before it is
Expand Down Expand Up @@ -104,7 +110,8 @@ export const DEFAULT_GESTURE_PARAMS: GestureParams = {
permitsFlick: () => true,
// Note: actual runtime value is determined at runtime based upon row height.
// See `VisualKeyboard.refreshLayout`, CTRL-F "Step 3".
flickDist: 5,
flickDistStart: 8,
flickDistFinal: 40,
waitLength: 500,
noiseTolerance: 10
},
Expand Down Expand Up @@ -349,11 +356,34 @@ export function instantContactResolutionModel(): ContactModel {
};
}

export function flickStartContactModel(params: GestureParams): ContactModel {
export function flickStartContactModel(params: GestureParams): gestures.specs.ContactModel<KeyElement, any> {
const flickParams = params.flick;

return {
itemPriority: 1,
pathModel: {
evaluate: (path) => path.stats.netDistance > params.flick.startDist ? 'resolve' : null
evaluate: (path, _, item) => {
const stats = path.stats;
const keySpec = item?.key.spec;

if(keySpec && keySpec.sk) {
const flickSpec = keySpec.flick;
const hasUpFlick = flickSpec.nw || flickSpec.n || flickSpec.ne;

if(!hasUpFlick) {
// Check for possible conflict with the longpress up-flick shortcut;
// it's supported on this key, as there is no true northish flick.
const baseDistance = stats.netDistance;
const angle = stats.angle; // from <0, -1> (straight up) going clockwise.
const verticalDistance = baseDistance * Math.cos(angle);
if(verticalDistance > params.longpress.flickDistStart) {
return 'reject';
}
}
}

return stats.netDistance > flickParams.startDist ? 'resolve' : null;
}
},
pathResolutionAction: 'resolve',
pathInheritance: 'partial'
Expand Down Expand Up @@ -464,7 +494,10 @@ export function longpressContactModel(params: GestureParams, enabledFlicks: bool
* each side of due N in total.
*/
if((enabledFlicks && spec.permitsFlick(stats.lastSample.item)) && (stats.cardinalDirection?.indexOf('n') != -1 ?? false)) {
if(stats.netDistance > spec.flickDist) {
const baseDistance = stats.netDistance;
const angle = stats.angle; // from <0, -1> (straight up) going clockwise.
const verticalDistance = baseDistance * Math.cos(angle);
if(verticalDistance > spec.flickDistFinal) {
return 'resolve';
}
} else if(resetForRoaming) {
Expand Down
17 changes: 13 additions & 4 deletions web/src/engine/osk/src/visualKeyboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1298,11 +1298,20 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke
Note: longpress.flickDist needs to be no greater than flick.startDist.
Otherwise, the longpress up-flick shortcut will not work on keys that
support flick gestures. (Such as sil_euro_latin 3.0+)

Since it's also based on the purely northward component, it's best to
have it be slightly lower. 80% of flick.startDist gives a range of
about 37 degrees to each side before a flick-start would win, while
70.7% gives 45 degrees.

(The range _will_ be notably tighter on keys with both longpresses and
flicks as a result.)
*/
this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight;
this.gestureParams.flick.startDist = 0.25 * this.currentLayer.rowHeight;
this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight;
this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight;
this.gestureParams.longpress.flickDistStart = 0.24 * this.currentLayer.rowHeight;
this.gestureParams.flick.startDist = 0.30 * this.currentLayer.rowHeight;
this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight;
this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight;
this.gestureParams.longpress.flickDistFinal = 0.75 * this.currentLayer.rowHeight;
}

// Phase 4: Refresh the layout of the layer-group and active layer.
Expand Down
Loading