fix(web): fixes illegal KMW event state - can't focus a null element #11385
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
jahorton
force-pushed
the
fix/web/unexpected-null-element
branch
from
40f3a9a
to
4fe7192
Compare
| activeControl: previousTarget?.getElement() | ||
| }); | ||
| const blurredElement = previousTarget?.getElement(); | ||
| const focusedElement = target?.getElement(); |
There was a problem hiding this comment.
This is the same as Ltarg above.
Also, there seems to be some overlapping logic in this function -- subtly different in what it tests, e.g. lines 278-282:
if(this.focusAssistant.restoringFocus) {
this._BlurKeyboardSettings(target.getElement());
} else if(target) {
this._FocusKeyboardSettings(target.getElement(), !hadRecentElement);
}
So I am not clear on the reasons for the differences and what they add up to.
There was a problem hiding this comment.
I did miss that it's a match for Ltarg.
As for the block you quoted, that's about maintaining proper connection between the active element and any control-specific keyboard settings. If focus was lost - like when the user interacts with a UI module - and then regained while the "restoring focus" mode is on, that indicates that the user just directly adjusted the keyboard settings for the current control. _BlurKeyboardSettings saves those settings, while _FocusKeyboardSettings restores them.
Admittedly, those details - what the two functions represent - are a case of "headspace documentation", I suppose - the two functions were originally refactored our from general control focus and control blur handling. The names made sense then, but probably do make less sense now in their present form.
So, the quoted block here is about handling of keyboard settings, which is independent from detecting and updating which control is focused. They are a bit related though - if restoring focus, we're explicitly re-focusing an element that should already be considered active. If we're not, we could be changing focus in a number of different ways.
| // Note: indicates the previous control being blurred (as | ||
| // `activeControl`), so 'controlfocused' and 'controlblurred' are | ||
| // mutually exclusive. |
There was a problem hiding this comment.
I don't really understand this comment -- the "so" second part doesn't seem to follow from the first part.
|
I notice the web build failed. Haven't investigated why... |
|
Changes in this pull request will be available for download in Keyman version 17.0.322-beta |
Fixes #11383.
@keymanapp-test-bot skip