fix(web): early cherrypick of bugfixes in gesture-staging dependencies, minor cleanup 🐵 #9599
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
| @@ -276,9 +279,19 @@ export class GestureMatcher<Type> implements PredecessorMatch<Type> { | |||
| throw new Error(`The specified gesture model does not support more than ${existingContacts} contact points.`); | |||
| } | |||
|
|
|||
| this.addContactInternal(simpleSource.constructSubview(false, true)); | |||
There was a problem hiding this comment.
The addContactInternal method includes initial-state checking, while the constructor does not. As certain gestures (modipresses) will require filtering based on the initially-pressed key, we need initial-state checking to apply for the first source, not only sources after the first. (Which was an oversight to begin with.)
I only noticed this issue when I started implementing modipress tests.
There was a problem hiding this comment.
Reverts PR #9453; I was able to address the underlying concerns in a prior PR without needing that specialized class. (Per the concerns of these two comments: #9453 (review), #9407 (comment))
Ideally, it'd have been removed sooner... but I was already in the midst of developing this PR when I fully stabilized the alternate approach. This is the point where I merged in the related fixes, so it's easiest to remove here. If preferred, I can look into rebasing this removal to an earlier point in the timeline / commit history, but I figured that'd just be busywork.
There was a problem hiding this comment.
Update: once I started to look at integration with KMW and remembered that #9591 recently occurred... well, gotta merge master in. I haven't pushed all the intermediate PRs quite yet - let alone the merges with this one and its descendants, but I ended up reverting #9453 within #9454, the most direct descendant to the component being dropped/reverted.
| @@ -38,10 +38,12 @@ fi | |||
| # END - Script parameter configuration | |||
|
|
|||
| test-headless ( ) { | |||
| # During debugging, "--slow 0" allows reporting the duration of ALL tests, not just the ones that run long. | |||
| # Can be useful... but probably shouldn't be the default. | |||
There was a problem hiding this comment.
The implication: the minor change in line 46 makes it far easier to temporarily insert --slow 0 (in line 43) on a temporary basis if desired to get execution-time reports for all tests, not just those that run long enough to cross mocha's default threshold for execution-time reporting.
…into chore/web/misc-gesture-fixes-and-cleanup
…into chore/web/misc-gesture-fixes-and-cleanup
| MOCHA_FLAGS= | ||
|
|
||
| if [ $REPORT_STYLE == "ci" ]; then | ||
| MOCHA_FLAGS="--reporter mocha-teamcity-reporter" | ||
| MOCHA_FLAGS="$MOCHA_FLAGS --reporter mocha-teamcity-reporter" |
There was a problem hiding this comment.
Given line 43 above, this change doesn't appear to do anything.
While implementing the next layers of the gesture engine and seeking to tie everything together, I ran into a few bugs in prior layers once I started linking things together and testing. Fortunately, it's quite possible to implement those fixes without having to reference the upcoming new layers of code, so they should be easier to review here, separately.
While the "selection" layers tests had been passing regardless, the bugs that are addressed here only became particularly visible when looking at how tracked data is managed across a GestureSource's lifetime and across different stages (or components) of the corresponding gesture as transitions occur. That's what the upcoming 'staging' layer is all about, after all.
There are also a few tweaks to the build infrastructure that arose from a new minor issues I've occasionally run into when branch-hopping during cleanup phases of development for the gesture module. Nothing too major, though.
This is one of many PRs to be extracted from #9455, which has grown too large to review as a whole.
@keymanapp-test-bot skip