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.

Sign up for GitHub

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): fixes up-flick shortcut issue for longpress when keys support downflicks #11216

Merged
merged 1 commit into from
Apr 12, 2024
+8 −2
Merged

fix(web): fixes up-flick shortcut issue for longpress when keys support downflicks #11216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion web/src/engine/osk/src/input/gestures/specsForLayout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,8 @@ export function specialKeyEndModel(params: GestureParams): GestureModel<any> {
export function longpressModel(params: GestureParams, allowShortcut: boolean, allowRoaming: boolean): GestureModel<any> {
const base: GestureModel<any> = {
id: 'longpress',
resolutionPriority: 0,
// Needs to beat flick-start priority.
resolutionPriority: 4,
Comment on lines +635 to +636
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, flick-start is specified at priority 3.

keyman/web/src/engine/osk/src/input/gestures/specsForLayout.ts

Lines 731 to 734 in b4ed832

export function flickStartModel(params: GestureParams): GestureModel<any> {
return {
id: 'flick-start',
resolutionPriority: 3,

contacts: [
{
model: {
Expand Down
7 changes: 6 additions & 1 deletion web/src/engine/osk/src/visualKeyboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,8 +1280,13 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke
const paddingZone = this.gestureEngine.config.maxRoamingBounds as PaddedZoneSource;
paddingZone.updatePadding([-0.333 * this.currentLayer.rowHeight]);

/*
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+)
*/
this.gestureParams.longpress.flickDist = 0.25 * this.currentLayer.rowHeight;
this.gestureParams.flick.startDist = 0.15 * this.currentLayer.rowHeight;
this.gestureParams.flick.startDist = 0.25 * this.currentLayer.rowHeight;
Copy link
Contributor Author

@jahorton jahorton Apr 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I opted to increase the minimum distance for a flick start due to #10647 - if the existing threshold for quick longpress triggers is already too permissive, reducing it would only make things worse. Thus, increasing the flick-start threshold to match is the safer way forward.

Additionally, #10647 probably indicates that the flick-start threshold needed to increase anyway - if 25% of key height is too permissive for one gesture, it likely is too permissive (as a minimum) for all of them.

this.gestureParams.flick.dirLockDist = 0.35 * this.currentLayer.rowHeight;
this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight;
}
Expand Down
Loading