feat(web): start of input boundary generalization 🐵#6844
feat(web): start of input boundary generalization 🐵#6844jahorton merged 19 commits intofeat/web/gesture-recognizer-mainfrom
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
1d3ff60 to
6b42436
Compare
dd6fa6e to
6e45ca9
Compare
…/web/gesture-recognizer-bounds
…/web/gesture-recognizer-bounds
…/web/gesture-recognizer-bounds
mcdurdin
left a comment
There was a problem hiding this comment.
There seems to be some code repetition which should be addressed. A number of other nits.
common/web/gesture-recognizer/src/gestureRecognizerConfiguration.ts
Outdated
Show resolved
Hide resolved
| readonly safeBoundPadding?: number | number[]; | ||
| } | ||
|
|
||
| export interface FinalizedGestureRecognizerConfiguration extends Nonoptional<GestureRecognizerConfiguration> { |
There was a problem hiding this comment.
Is there a real need for a separate interface? It seems like it adds a lot of boilerplate.
There was a problem hiding this comment.
The intent is to keep things relatively DRY while allowing different type specifications for the "public" version of the config and the pre-processed internal version. These fields will be used on every input event, so I feel a little optimization here is merited - especially for avoiding the need to constantly reinitialize paddedSafeBounds. Unless we wanted to expose that and make it optional on the input object?
It's also nice to eliminate the type-ambiguity of safeBoundPadding once configuration is done; that's less type-checking boilerplate elsewhere in the code. (Particularly, in the new zoneBoundaryChecker.ts.)
There was a problem hiding this comment.
It's just a little too OO for my comfort -- why not just make paddedSafeBounds optional on the input object as you say with corresponding comments? It's all internal anyway so it's not like we are making API contracts.
There was a problem hiding this comment.
I've made an adjustment in this regard; are the recent changes enough to address this concern?
| if(!ZoneBoundaryChecker.inputStartOutOfBoundsCheck(coord, this.config)) { | ||
| this.disabledSafeBounds = ZoneBoundaryChecker.inputStartSafeBoundProximityCheck(coord, this.config); | ||
| } |
There was a problem hiding this comment.
What is this doing? It's not obvious why you are doing this -- please add a code comment!
There was a problem hiding this comment.
This comment is a secondary target of this comment: #6843 (comment)
This addresses probably the most niche aspect of the OSK's touchpoint management logic, so I'm not exactly surprised by the request. Re #6842's visual: "If the touchpoint is within the larger set of dashed lines but not the smaller set, remember which border(s) we started near." And that has a second half. Not exactly the simplest thing to document using only words; this is part of why I spent as much time as I did on the host page's visual design.
…/web/gesture-recognizer-bounds
…/web/gesture-recognizer-bounds
The
VisualKeyboardclass has some... fun logic for determining when to cancel an ongoing touch or mouse interaction. Input may continue slightly out of bounds - but not too far. Input that gets too close to a screen edge is also interpreted as cancellation (since you can't detect past physical device boundaries) - unless the interaction started very near to those boundaries.After some thought, analysis, and effort... I've come up with a scheme that will allow this to be specified relatively cleanly - both within this module and within KeymanWeb once integrated. This scheme will also allow for other boundary specifications - like those that will be needed for a gesture testing host/demo page.
@keymanapp-test-bot skip