Skip to content

feat(web): start of support for multi-touchpoint tracking 🐵#6914

Merged
jahorton merged 26 commits intofeat/web/gesture-recognizer-mainfrom
feat/web/recognizer-multipoint-tracking
Jul 29, 2022
Merged

feat(web): start of support for multi-touchpoint tracking 🐵#6914
jahorton merged 26 commits intofeat/web/gesture-recognizer-mainfrom
feat/web/recognizer-multipoint-tracking

Conversation

@jahorton
Copy link
Copy Markdown
Contributor

@jahorton jahorton commented Jul 8, 2022

Continues from #6878.

This PR introduces object-orientation designed to facilitate the tracking and handling of multiple active touchpoints and their related input(s). On touchstart or mousedown, a new TrackedPoint is initialized that will receive data for all further events during the lifetime of that touchpoint (or touchpoint-emulating mouse interaction) until it is considered ended or canceled.

The structure, as currently implemented, also records all incoming coordinate-timestamp pairs over the touchpoint's lifetime. This will greatly facilitate the ability to create unit tests for the module. In practical use, the plan is for these pairs to be progressively cleared once their role in a gesture's lifetime has been fully determined - unless an option is set to preserve them for "recording" mode.


This branch aims to establish a specific object-oriented design for each 'input' received by this module. That statement needs some unpacking, so here goes:

TrackedInput

This will be the top-level object for any single ongoing input event as determined by the gesture-recognition engine, even once completed. Note: as determined by the gesture-recognition engine. The goal is that this will model the expected user-experience of what "a single input" is, rather than the programmer interpretation.

For now, this class mostly acts as a wrapper for a single touchpoint. If/when we start supporting multi-touchpoint gestures - say, a two-finger swipe-like gesture (perhaps to fine-tune the caret's position in KMW) - both touchpoints should be considered as equal components of "the same" input.

As a result of this, the object offers an array of touchpoints, though the common case will feature only one. It is also intended for the input's corresponding gesture (or potential gesture states?), once determined, to be accessible here as a property.

TrackedPoint

This is the top-level object for any metadata particular to a single active (or once-active) touchpoint (or cursor-based emulation thereof). At present, its main role as a distinct class from TrackedPath is how it serves to match incoming events against the touchpoint, but there will likely be useful properties added later for ease of use by consuming objects that make less sense on a path property.

TrackedPath

This class will be responsible for tracking and analyzing the path of the touchpoint over its lifetime. While currently only supporting an array of coordinates, the current plan is for path segmentation to occur either within this class or from the view of this class, without the need for any of its parents or their properties. There will be a not-insignificant amount of code for this goal, so it's best to keep it isolated away from pretty much everything else if we can help it - that's part of the reason for keeping it separate from TrackedPoint.

There's also some nice interface-semantics design reasoning behind this:

touchpoint.path.on('step', ...); // Event generated whenever the touchpoint has a new event, taking another "step"
touchpoint.path.on('segmentation', ...); // To be generated whenever the path identifies a segmentation boundary

// Accessing path data
touchpoint.path.coords(); 
touchpoint.path.segments();

// Which is easily distinct from (a potential)...
touchpoint.coord(); // the _current_ coordinate of the touchpoint.  (Not yet implemented)
// because of the collective structure of the related objects.

The 'path' may be 'terminated' with instructions to invalidate the related touchpoint (e.g, due to roaming beyond supported bounds) or to indicate that the touchpoint has been lifted, ending its lifecycle. (The core event processing 'engines' operate from this view, after all.) It also supports related events. Both of these aspects are separate from, but connected to, similar instructions and events on TrackedInput.


@keymanapp-test-bot skip

The following two PRs will set up better-tailored test resources.

Note that going commit-by-commit may be a little tricky here if that's your review style; a major rework occurred after a certain point. For that, I made the rework in a branch based on #6970 and then backported all relevant changes to their best-matching PRs, so there's likely to be quite a bit of 'splicing', and thus instability in the midst of those commits.

@jahorton jahorton added this to the 16.0 milestone Jul 8, 2022
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jul 8, 2022
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Jul 8, 2022

@jahorton jahorton force-pushed the feat/web/gesture-recognizer-rigging branch 2 times, most recently from 9db2da6 to f19b575 Compare July 11, 2022 06:08
@jahorton jahorton force-pushed the feat/web/recognizer-multipoint-tracking branch from bad929a to 849277f Compare July 11, 2022 06:11
@jahorton jahorton force-pushed the feat/web/recognizer-multipoint-tracking branch from 2212185 to 426cfd7 Compare July 15, 2022 01:13
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jul 15, 2022
Base automatically changed from feat/web/gesture-recognizer-rigging to feat/web/gesture-recognizer-main July 20, 2022 08:54
@jahorton jahorton marked this pull request as ready for review July 20, 2022 08:54
ermshiperete
ermshiperete previously approved these changes Jul 21, 2022
this.identifier = identifier;
if(obj instanceof EventTarget) {
this._currentTarget = obj;
this.isFromTouch = isFromTouch;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The isFromTouch property is not marked as optional but the parameter is?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TypeScript multi-signature constructors are... complicated to parse sometimes. Note that the first version of the signature doesn't include the third parameter. The first two signatures are the ones accessible for external code, while the final one is the constructor's "internal" signature, if that makes sense. True constructor overloading is impossible in JS/TS, so we have to make do this way.

When the first form of the constructor is intended, this constructor will reuse the value found at parsedObj.isFromTouch - parsedObj will be a JSON parse of a previously-stringified instance.

@jahorton jahorton marked this pull request as draft July 25, 2022 04:58
@jahorton
Copy link
Copy Markdown
Contributor Author

OK, the primary structural rework is done. Now to double-check old comments and determine if and how they map to the new version of the branch.

@jahorton
Copy link
Copy Markdown
Contributor Author

... and all previous comments & suggestions should now be properly addressed. Feel free to correct me on that if I did miss something somehow; there were quite a few moving parts at times during the rework.

// Defines the actual event-raising function.
emit(eventName: string, ...args: any[]);
// A full copy of the module's actual .d.ts file, save for the one line that prevents us from
// actually referencing it. (Since the project doesn't use modules.)
Copy link
Copy Markdown
Contributor Author

@jahorton jahorton Jul 27, 2022

Choose a reason for hiding this comment

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

If we prefer, I could, in theory, add something to the build script to nix that line, or to replicate the original file and then nix it for use instead of having this hardcoded copy. That said, I think this is likely to be the most preferred approach by our team lead, so I went with this.

The big benefit this more-explicit type definition file gives us: solid Intellisense & typing for any class's supported events. With the old version, we didn't really get typing or suggestions about supported events when using a subclass's event-related functions; it's a major help when developing related code.

@jahorton jahorton marked this pull request as ready for review July 27, 2022 04:07
@jahorton jahorton requested a review from mcdurdin July 29, 2022 00:27
@jahorton jahorton dismissed stale reviews from mcdurdin and ermshiperete July 29, 2022 00:31

Prior comments motivated a significant rework.

Copy link
Copy Markdown
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

I like this model a lot more than the previous one. It's less abstracted and really easy to understand how the different objects relate. The logic seems to fall out well and as far as I can see should be easy to model against all kinds of gestures. Good work!

Only nits I think in my comments.

@jahorton
Copy link
Copy Markdown
Contributor Author

And, of course, a TestFlight upload fail on the iOS build. That's known to be unrelated, so we ought be safe to merge now.

@jahorton jahorton merged commit 1b4ad74 into feat/web/gesture-recognizer-main Jul 29, 2022
@jahorton jahorton deleted the feat/web/recognizer-multipoint-tracking branch July 29, 2022 04:15
@jahorton jahorton mentioned this pull request Sep 19, 2022
1 task
@mcdurdin mcdurdin modified the milestones: 16.0, A16S7 Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants