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): allows shifting the recognition zone of an active GestureSource 🐵 #9526

Merged
merged 8 commits into from
Sep 27, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
// after configuration.

import { InputSample } from "../headless/inputSample.js";
import { Mutable } from "../mutable.js";
import { Nonoptional } from "../nonoptional.js";
import { PaddedZoneSource } from "./paddedZoneSource.js";
import { RecognitionZoneSource } from "./recognitionZoneSource.js";

// For example, customization of a longpress timer's length need not be readonly.
Expand Down Expand Up @@ -81,4 +84,34 @@ export interface GestureRecognizerConfiguration<HoveredItemType> {
* @returns
*/
readonly itemIdentifier?: (coord: Omit<InputSample<any>, 'item'>, target: EventTarget) => HoveredItemType;
}

export function preprocessRecognizerConfig<HoveredItemType>(
config: GestureRecognizerConfiguration<HoveredItemType>
): Nonoptional<GestureRecognizerConfiguration<HoveredItemType>> {
// Allows configuration pre-processing during this method.
let processingConfig: Mutable<Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>> = {...config} as Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>;
Copy link
Member

Choose a reason for hiding this comment

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

That is one heck of a type -- starting to feel downright Java!

Copy link
Contributor Author

@jahorton jahorton Sep 26, 2023

Choose a reason for hiding this comment

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

I always have loved generics a little too much... and yeah, I felt like this was brushing up against "too much". Fits well, though.

FWIW, it can get much worse - it's been noted that TypeScript's type system is Turing-complete. That'd clearly be way too far.

Okay, maybe I don't love them that much - some people clearly had fun writing their analyses in that issue. (Like the guy who wrote a MUD with them!)

processingConfig.mouseEventRoot = processingConfig.mouseEventRoot ?? processingConfig.targetRoot;
processingConfig.touchEventRoot = processingConfig.touchEventRoot ?? processingConfig.targetRoot;

processingConfig.inputStartBounds = processingConfig.inputStartBounds ?? processingConfig.targetRoot;
processingConfig.maxRoamingBounds = processingConfig.maxRoamingBounds ?? processingConfig.targetRoot;
processingConfig.safeBounds = processingConfig.safeBounds ?? new PaddedZoneSource([2]);

processingConfig.itemIdentifier = processingConfig.itemIdentifier ?? (() => null);
Comment on lines +94 to +101
Copy link
Member

Choose a reason for hiding this comment

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

Some comments on why these are being setup this way would be helpful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that the documentation for the fields and their defaults has already been established in JSDoc-style comments on each of them. It shows up via IntelliSense within VS Code.

Granted, why the defaults are set as they are... that is a different question. Is that your concern?

Edit:

And ... now I see[...]

Ah. Got it, the concern would make more sense if the code were appearing without any context.


if(!config.paddedSafeBounds) {
let paddingArray = config.safeBoundPadding;
Comment on lines +103 to +104
Copy link
Member

Choose a reason for hiding this comment

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

These two member names are super conflatable!

if(typeof paddingArray == 'number') {
paddingArray = [ paddingArray ];
}
paddingArray = paddingArray ?? [3];
Copy link
Member

Choose a reason for hiding this comment

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

Why 3? Magic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* If not specified, this will default to a padding of 3px inside the standard safeBounds
* unless `paddedSafeBounds` is defined.

From:

/**
* Used to define a "boundary" slightly more constrained than `safeBounds`. Events that
* start within this pixel range from a safe bound will disable that bound for the duration
* of its corresponding input sequence. May be a number or an array of 1, 2, or 4 numbers,
* as with CSS styling.
*
* If not specified, this will default to a padding of 3px inside the standard safeBounds
* unless `paddedSafeBounds` is defined.
*
* If `paddedSafeBounds` was specified initially, this will be set to `undefined`.
*/
readonly safeBoundPadding?: number | number[];

If memory serves, this default is derived from the corresponding value KMW is already using for the OSK. I've just abstracted its purpose and given it a name and documentation.


processingConfig.paddedSafeBounds = new PaddedZoneSource(processingConfig.safeBounds, paddingArray);
} else {
// processingConfig.paddedSafeBounds is already set via the spread operator above.
delete processingConfig.safeBoundPadding;
}

return processingConfig;
}
36 changes: 2 additions & 34 deletions common/web/gesture-recognizer/src/engine/gestureRecognizer.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { GestureRecognizerConfiguration } from "./configuration/gestureRecognizerConfiguration.js";
import { GestureRecognizerConfiguration, preprocessRecognizerConfig } from "./configuration/gestureRecognizerConfiguration.js";
import { MouseEventEngine } from "./mouseEventEngine.js";
import { Mutable } from "./mutable.js";
import { Nonoptional } from "./nonoptional.js";
import { PaddedZoneSource } from "./configuration/paddedZoneSource.js";
import { TouchEventEngine } from "./touchEventEngine.js";
import { TouchpointCoordinator } from "./headless/touchpointCoordinator.js";

Expand All @@ -12,39 +10,9 @@ export class GestureRecognizer<HoveredItemType> extends TouchpointCoordinator<Ho
private readonly mouseEngine: MouseEventEngine<HoveredItemType>;
private readonly touchEngine: TouchEventEngine<HoveredItemType>;

protected static preprocessConfig<HoveredItemType>(
Copy link
Member

Choose a reason for hiding this comment

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

And ... now I see that my comments relate to pre-existing code. I still think they are good comments though 😁

config: GestureRecognizerConfiguration<HoveredItemType>
): Nonoptional<GestureRecognizerConfiguration<HoveredItemType>> {
// Allows configuration pre-processing during this method.
let processingConfig: Mutable<Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>> = {...config} as Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>;
processingConfig.mouseEventRoot = processingConfig.mouseEventRoot ?? processingConfig.targetRoot;
processingConfig.touchEventRoot = processingConfig.touchEventRoot ?? processingConfig.targetRoot;

processingConfig.inputStartBounds = processingConfig.inputStartBounds ?? processingConfig.targetRoot;
processingConfig.maxRoamingBounds = processingConfig.maxRoamingBounds ?? processingConfig.targetRoot;
processingConfig.safeBounds = processingConfig.safeBounds ?? new PaddedZoneSource([2]);

processingConfig.itemIdentifier = processingConfig.itemIdentifier ?? (() => null);

if(!config.paddedSafeBounds) {
let paddingArray = config.safeBoundPadding;
if(typeof paddingArray == 'number') {
paddingArray = [ paddingArray ];
}
paddingArray = paddingArray ?? [3];

processingConfig.paddedSafeBounds = new PaddedZoneSource(processingConfig.safeBounds, paddingArray);
} else {
// processingConfig.paddedSafeBounds is already set via the spread operator above.
delete processingConfig.safeBoundPadding;
}

return processingConfig;
}

public constructor(config: GestureRecognizerConfiguration<HoveredItemType>) {
super();
this.config = GestureRecognizer.preprocessConfig(config);
this.config = preprocessRecognizerConfig(config);

this.mouseEngine = new MouseEventEngine<HoveredItemType>(this.config);
this.touchEngine = new TouchEventEngine<HoveredItemType>(this.config);
Expand Down
160 changes: 135 additions & 25 deletions common/web/gesture-recognizer/src/engine/headless/gestureSource.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { InputSample } from "./inputSample.js";
import { SerializedGesturePath, GesturePath } from "./gesturePath.js";
import { GestureRecognizerConfiguration, preprocessRecognizerConfig } from "../configuration/gestureRecognizerConfiguration.js";
import { Nonoptional } from "../nonoptional.js";

/**
* Documents the expected typing of serialized versions of the `GestureSource` class.
Expand Down Expand Up @@ -47,6 +49,9 @@ export class GestureSource<HoveredItemType> {

private static _jsonIdSeed: -1;

// Assertion: must always contain an index 0 - the base recognizer config.
protected recognizerConfigStack: Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>[];

/**
* Tracks the coordinates and timestamps of each update for the lifetime of this `GestureSource`.
*/
Expand All @@ -60,10 +65,16 @@ export class GestureSource<HoveredItemType> {
* @param initialHoveredItem The initiating event's original target element
* @param isFromTouch `true` if sourced from a `TouchEvent`; `false` otherwise.
*/
constructor(identifier: number, isFromTouch: boolean) {
constructor(
identifier: number,
recognizerConfig: Nonoptional<GestureRecognizerConfiguration<HoveredItemType>> | Nonoptional<GestureRecognizerConfiguration<HoveredItemType>>[],
isFromTouch: boolean
) {
this.rawIdentifier = identifier;
this.isFromTouch = isFromTouch;
this._path = new GesturePath();

this.recognizerConfigStack = Array.isArray(recognizerConfig) ? recognizerConfig : [recognizerConfig];
}

/**
Expand All @@ -76,7 +87,7 @@ export class GestureSource<HoveredItemType> {
const isFromTouch = jsonObj.isFromTouch;
const path = GesturePath.deserialize(jsonObj.path);

const instance = new GestureSource(id, isFromTouch);
const instance = new GestureSource(id, null, isFromTouch);
instance._path = path;
return instance;
}
Expand Down Expand Up @@ -112,7 +123,7 @@ export class GestureSource<HoveredItemType> {
* @returns
*/
public constructSubview(startAtEnd: boolean, preserveBaseItem: boolean): GestureSourceSubview<HoveredItemType> {
return new GestureSourceSubview(this, startAtEnd, preserveBaseItem);
return new GestureSourceSubview(this, this.recognizerConfigStack, startAtEnd, preserveBaseItem);
}

/**
Expand Down Expand Up @@ -142,22 +153,47 @@ export class GestureSource<HoveredItemType> {
return `${prefix}:${this.rawIdentifier}`;
}

public pushRecognizerConfig(config: Omit<GestureRecognizerConfiguration<HoveredItemType>, 'touchEventRoot'| 'mouseEventRoot'>) {
const configToProcess = {...config,
mouseEventRoot: this.recognizerConfigStack[0].mouseEventRoot,
touchEventRoot: this.recognizerConfigStack[0].touchEventRoot
}
this.recognizerConfigStack.push(preprocessRecognizerConfig(configToProcess));
}

public popRecognizerConfig() {
if(this.recognizerConfigStack.length == 1) {
throw new Error("Cannot 'pop' the original recognizer-configuration for this GestureSource.")
}

return this.recognizerConfigStack.pop();
}

public get currentRecognizerConfig() {
return this.recognizerConfigStack[this.recognizerConfigStack.length-1];
}

/**
* Creates a serialization-friendly version of this instance for use by
* `JSON.stringify`.
*/
/* c8 ignore start */
toJSON(): SerializedGestureSource {
let jsonClone: SerializedGestureSource = {
isFromTouch: this.isFromTouch,
path: this.path.toJSON()
}

return jsonClone;
/* c8 ignore stop */
/* c8 ignore next 2 */
// esbuild or tsc seems to mangle the 'ignore stop' if put outside the ending brace.
}
}

export class GestureSourceSubview<HoveredItemType> extends GestureSource<HoveredItemType> {
private _baseSource: GestureSource<HoveredItemType>
private _baseStartIndex: number;
private subviewDisconnector: () => void;

/**
Expand All @@ -167,25 +203,61 @@ export class GestureSourceSubview<HoveredItemType> extends GestureSource<Hovered
* @param initialHoveredItem The initiating event's original target element
* @param isFromTouch `true` if sourced from a `TouchEvent`; `false` otherwise.
*/
constructor(source: GestureSource<HoveredItemType>, startAtEnd: boolean, preserveBaseItem: boolean) {
super(source.rawIdentifier, source.isFromTouch);
constructor(
source: GestureSource<HoveredItemType>,
configStack: typeof GestureSource.prototype['recognizerConfigStack'],
startAtEnd: boolean,
preserveBaseItem: boolean
) {
let mayUpdate = true;
let start = 0;
let length = source.path.coords.length;
if(source instanceof GestureSourceSubview) {
const expectedLength = source._baseStartIndex + source.path.coords.length;
start = source._baseStartIndex;
jahorton marked this conversation as resolved.
Show resolved Hide resolved
// Check against the full remaining length of the original source; does
// the subview provided to us include its source's most recent point?
const sampleCountSinceStart = source.baseSource.path.coords.length;
if(expectedLength != start + sampleCountSinceStart) {
mayUpdate = false;
}
}

super(source.rawIdentifier, configStack, source.isFromTouch);

const baseSource = this._baseSource = source instanceof GestureSourceSubview ? source._baseSource : source;

/**
* Provides a coordinate-system translation for source subviews.
* The base version still needs to use the original coord system, though.
*/
const translateSample = (sample: InputSample<HoveredItemType>) => {
const translation = this.recognizerTranslation;
// Provide a coordinate-system translation for source subviews.
// The base version still needs to use the original coord system, though.
return {...sample, targetX: sample.targetX - translation.x, targetY: sample.targetY - translation.y};
}

// Note: we don't particularly need subviews to track the actual coords aside from
// tracking related stats data. But... we don't have an "off-switch" for that yet.
let subpath: GesturePath<HoveredItemType>;

// Will hold the last sample _even if_ we don't save every coord that comes through.
const lastSample = source.path.stats.lastSample;

// Are we 'chop'ping off the existing path or preserving it? This sets the sample-copying
// configuration accordingly.
if(startAtEnd) {
subpath = new GesturePath<HoveredItemType>();
if(lastSample) {
subpath.extend(lastSample);
}
this._baseStartIndex = start = Math.max(start + length - 1, 0);
length = length > 0 ? 1 : 0;
} else {
subpath = source.path.clone();
this._baseStartIndex = start;
}

subpath = new GesturePath<HoveredItemType>();
for(let i=0; i < length; i++) {
const index = start + i;
subpath.extend(translateSample(baseSource.path.coords[index]));
jahorton marked this conversation as resolved.
Show resolved Hide resolved
}

this._path = subpath;
Expand All @@ -196,25 +268,51 @@ export class GestureSourceSubview<HoveredItemType> extends GestureSource<Hovered
this._baseItem = lastSample?.item;
}

// Ensure that this 'subview' is updated whenever the "source of truth" is.
const completeHook = () => this.path.terminate(false);
const invalidatedHook = () => this.path.terminate(true);
const stepHook = (sample) => this.update(sample);
baseSource.path.on('complete', completeHook);
baseSource.path.on('invalidated', invalidatedHook);
baseSource.path.on('step', stepHook);

// But make sure we can "disconnect" it later once the gesture being matched
// with the subview has fully matched; it's good to have a snapshot left over.
this.subviewDisconnector = () => {
baseSource.path.off('complete', completeHook);
baseSource.path.off('invalidated', invalidatedHook);
baseSource.path.off('step', stepHook);
if(mayUpdate) {
// Ensure that this 'subview' is updated whenever the "source of truth" is.
const completeHook = () => this.path.terminate(false);
const invalidatedHook = () => this.path.terminate(true);
const stepHook = (sample: InputSample<HoveredItemType>) => {
super.update(translateSample(sample));
};
baseSource.path.on('complete', completeHook);
baseSource.path.on('invalidated', invalidatedHook);
baseSource.path.on('step', stepHook);

// But make sure we can "disconnect" it later once the gesture being matched
// with the subview has fully matched; it's good to have a snapshot left over.
this.subviewDisconnector = () => {
baseSource.path.off('complete', completeHook);
baseSource.path.off('invalidated', invalidatedHook);
baseSource.path.off('step', stepHook);
}
}
}

private get recognizerTranslation() {
// Allowing a 'null' config greatly simplifies many of our unit-test specs.
if(this.recognizerConfigStack.length == 1 || !this.currentRecognizerConfig) {
return {
x: 0,
y: 0
};
}

// Could compute all of this a single time & cache the value whenever a recognizer-config is pushed or popped.
const currentRecognizer = this.currentRecognizerConfig;
const currentClientRect = currentRecognizer.targetRoot.getBoundingClientRect();
const baseClientRect = this.recognizerConfigStack[0].targetRoot.getBoundingClientRect();

return {
x: currentClientRect.x - baseClientRect.x,
y: currentClientRect.y - baseClientRect.y
}
}

/**
* The original GestureSource this subview is based upon.
* The original GestureSource this subview is based upon. Note that the coordinate system may
* differ if a gesture stage/component has occurred that triggered a change to the active
* recognizer configuration. (e.g. a subkey menu is being displayed for a longpress interaction)
*/
public get baseSource() {
return this._baseSource;
Expand All @@ -231,6 +329,18 @@ export class GestureSourceSubview<HoveredItemType> extends GestureSource<Hovered
}
}

public pushRecognizerConfig(config: Omit<GestureRecognizerConfiguration<HoveredItemType>, "touchEventRoot" | "mouseEventRoot">): void {
throw new Error("Pushing and popping of recognizer configurations should only be called on the base GestureSource");
}

public popRecognizerConfig(): Nonoptional<GestureRecognizerConfiguration<HoveredItemType>> {
throw new Error("Pushing and popping of recognizer configurations should only be called on the base GestureSource");
}

public update(sample: InputSample<HoveredItemType>): void {
throw new Error("Updates should be provided through the base GestureSource.")
}

/**
* Like `disconnect`, but this will also terminate the baseSource and prevent further
* updates for the true, original `GestureSource` instance. If the gesture-model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ export abstract class InputEngineBase<HoveredItemType> extends EventEmitter<Even
return this._activeTouchpoints.find((point) => point.rawIdentifier == identifier);
}

/**
* During the lifetime of a GestureSource (a continuous path for a single touchpoint),
* it is possible that the legal area for the path may change. This function allows
* us to find the appropriate set of constraints for the path if any changes have been
* requested - say, for a subkey menu after a longpress.
* @param identifier
* @returns
*/
protected getConfigForId(identifier: number) {
return this.getTouchpointWithId(identifier).currentRecognizerConfig;
}

public dropTouchpointWithId(identifier: number) {
this._activeTouchpoints = this._activeTouchpoints.filter((point) => point.rawIdentifier != identifier);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export abstract class InputEventEngine<HoveredItemType> extends InputEngineBase<
}

protected onInputStart(identifier: number, sample: InputSample<HoveredItemType>, target: EventTarget, isFromTouch: boolean) {
const touchpoint = new GestureSource<HoveredItemType>(identifier, isFromTouch);
const touchpoint = new GestureSource<HoveredItemType>(identifier, this.config, isFromTouch);
touchpoint.update(sample);

this.addTouchpoint(touchpoint);
Expand Down
3 changes: 2 additions & 1 deletion common/web/gesture-recognizer/src/engine/mouseEventEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ export class MouseEventEngine<HoveredItemType> extends InputEventEngine<HoveredI
}

this.preventPropagation(event);
const config = this.getConfigForId(this.activeIdentifier);

if(!ZoneBoundaryChecker.inputMoveCancellationCheck(sample, this.config, this.disabledSafeBounds)) {
if(!ZoneBoundaryChecker.inputMoveCancellationCheck(sample, config, this.disabledSafeBounds)) {
this.onInputMove(this.activeIdentifier, sample, event.target);
} else {
this.onInputMoveCancel(this.activeIdentifier, sample, event.target);
Expand Down
Loading