Merge pull request #9893 from keymanapp/fix/web/cancelled-gesture-cleanup

fix(web): better cleanup of cancelled gestures 🐵
This commit is contained in:
Joshua Horton 2023-10-31 09:28:40 +07:00 committed by GitHub
commit 40c67c19d3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 41 additions and 45 deletions

View file

@ -96,6 +96,7 @@ export class GestureSequence<Type> extends EventEmitter<EventMap<Type>> {
private pushedSelector?: MatcherSelector<Type>;
private gestureConfig: GestureModelDefs<Type>;
private markedComplete: boolean = false;
// Note: the first stage will be available under `stageReports` after awaiting a simple Promise.resolve().
constructor(
@ -110,6 +111,16 @@ export class GestureSequence<Type> extends EventEmitter<EventMap<Type>> {
this.selector = selector;
this.selector.on('rejectionwithaction', this.modelResetHandler);
this.once('complete', () => {
if(this.pushedSelector) {
// The `popSelector` method is responsible for triggering cascading cancellations if
// there are nested GestureSequences.
//
// As this tends to affect which gestures are permitted, it's important this is done
// any time the GestureSequence is cancelled or completed, for any reason.
this.touchpointCoordinator?.popSelector(this.pushedSelector);
this.pushedSelector = null;
}
this.selector.off('rejectionwithaction', this.modelResetHandler);
this.selector.dropSourcesWithIds(this.allSourceIds);
@ -193,13 +204,10 @@ export class GestureSequence<Type> extends EventEmitter<EventMap<Type>> {
});
if(!selection.result.matched) {
if(this.pushedSelector) {
// The `popSelector` method is responsible for triggering cascading cancellations if
// there are nested GestureSequences.
this.touchpointCoordinator?.popSelector(this.pushedSelector);
if(!this.markedComplete) {
this.markedComplete = true;
this.emit('complete');
}
this.emit('complete');
return;
}
}
@ -274,13 +282,11 @@ export class GestureSequence<Type> extends EventEmitter<EventMap<Type>> {
}
}
} else {
if(this.pushedSelector) {
this.touchpointCoordinator?.popSelector(this.pushedSelector);
this.pushedSelector = null;
}
// Any extra finalization stuff should go here, before the event, if needed.
this.emit('complete');
if(!this.markedComplete) {
this.markedComplete = true;
this.emit('complete');
}
}
}
@ -305,7 +311,11 @@ export class GestureSequence<Type> extends EventEmitter<EventMap<Type>> {
public cancel() {
const sources = this.stageReports[this.stageReports.length - 1].sources;
sources.forEach((src) => src.terminate(true));
sources.forEach((src) => src.baseSource.isPathComplete || src.baseSource.terminate(true));
if(!this.markedComplete) {
this.markedComplete = true;
this.emit('complete');
}
}
}

View file

@ -325,22 +325,10 @@ export class MatcherSelector<Type> extends EventEmitter<EventMap<Type>> {
return async (result: MatchResult<Type>) => {
// Note: is only called by GestureMatcher Promises that are resolving.
/*
* If we already had a gesture stage match, this will have already been fulfilled;
* bypass all match-handling. Capturing `matchSynchronization` in a closure in this
* manner is important to ensure that the returned handler is "locked" to the
* currently-processing gesture stage.
*/
for(let synchronizer of matchSynchronizers) {
if(synchronizer.isFulfilled) {
return;
}
}
// Do not bypass match handling just because a synchronization promise is fulfilled.
// If a source was force-cancelled, cascading to a call of this handler, we still
// need to perform internal state cleanup.
/* If cancellation was requested but not pre-filtered by the synchronizer setup, replace
* the result object. The matcher's Promise may have resolved simultaneously with the
* winner but 'lost', a scenario that may require careful handling to clean up.
*/
if(matcher.isCancelled) {
result = {
matched: false,
@ -371,11 +359,6 @@ export class MatcherSelector<Type> extends EventEmitter<EventMap<Type>> {
return;
}
if(matcher.isCancelled) {
// Fortunately, the rest of the code will help us recover from the state.
console.warn("Unexpected state: a cancelled GestureMatcher was still listed as a possibility");
}
this.potentialMatchers.splice(matcherIndex, 1);
/*
@ -480,14 +463,6 @@ export class MatcherSelector<Type> extends EventEmitter<EventMap<Type>> {
/*
* Fulfills the contract set by `matchGesture`.
*
* Also, fulfilling the ManagedPromise acts as a synchronizer, partially facilitating the
* guarantee at the start of this closure. It's set synchronously, so other gesture-matchers
* that call into this method will know that a match has already fulfilled for the matched
* source(s). Any further matchers will be silently ignored, effectively cancelling them.
* However, this fails to handle the case where two separate calls to matcherSelectionFilter
* occur for the same matcher due to one source being added at a later point in time;
* this is what the `cancel`
*/
tracker.matchPromise.resolve({matcher, result});
}

View file

@ -173,6 +173,15 @@ export class TouchpointCoordinator<HoveredItemType, StateToken=any> extends Even
touchpoint.path.on('invalidated', () => {
// GestureSequence _should_ handle any other cleanup internally as fallout
// from the path being cancelled.
//
// That said, it's handled asynchronously... but we can give a synchronous signal
// through the next block of code, allowing cleanup to occur earlier during
// recovery states.
const owningSequence = this.activeGestures.find((entry) => entry.allSourceIds.includes(touchpoint.identifier));
if(owningSequence) {
owningSequence.cancel();
}
// To consider: should it specially mark if it 'completed' due to cancellation,
// or is that safe to infer from the tracked GestureSource(s)?
@ -183,8 +192,6 @@ export class TouchpointCoordinator<HoveredItemType, StateToken=any> extends Even
this._activeSources = this._activeSources.splice(i, 1);
});
touchpoint.path.on('complete', () => {
// TODO: on cancellation, is there any other cleanup to be done?
// Also mark the touchpoint as no longer active.
let i = this._activeSources.indexOf(touchpoint);
this._activeSources = this._activeSources.splice(i, 1);

View file

@ -105,7 +105,7 @@ export class TouchEventEngine<HoveredItemType, StateToken = any> extends InputEv
const newTouches = touchListToArray(event.changedTouches);
// Maintain all touches in the `.touches` array that are NOT marked as `.changedTouches` (and therefore, new)
this.maintainTouchpointsWithIds(allTouches
.filter((touch) => (newTouches.indexOf(touch) == -1))
.filter((touch1) => newTouches.findIndex(touch2 => touch1.identifier == touch2.identifier) == -1)
.map((touch) => touch.identifier)
);

View file

@ -217,7 +217,11 @@ export default class VisualKeyboard extends EventEmitter<EventMap> implements Ke
throw new Error(`Keyboard ${this.layoutKeyboard.id} does not have a layer with id ${value}`);
} else {
this._layerId = value;
this.gestureEngine.stateToken = value;
// Does not exist for documentation keyboards!
if(this.gestureEngine) {
this.gestureEngine.stateToken = value;
}
}
if(changedLayer) {