feat(web): start of support for multi-touchpoint tracking 🐵#6914
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts |
9db2da6 to
f19b575
Compare
bad929a to
849277f
Compare
2212185 to
426cfd7
Compare
| this.identifier = identifier; | ||
| if(obj instanceof EventTarget) { | ||
| this._currentTarget = obj; | ||
| this.isFromTouch = isFromTouch; |
There was a problem hiding this comment.
The isFromTouch property is not marked as optional but the parameter is?
There was a problem hiding this comment.
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.
|
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. |
|
... 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.) |
There was a problem hiding this comment.
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.
Prior comments motivated a significant rework.
mcdurdin
left a comment
There was a problem hiding this comment.
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.
|
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. |
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
touchstartormousedown, a newTrackedPointis 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:
TrackedInputThis 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 correspondinggesture(or potential gesture states?), once determined, to be accessible here as a property.TrackedPointThis 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
TrackedPathis 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 apathproperty.TrackedPathThis 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:
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.