-
-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(web): better element inputmode management 📴 #7395
Changes from 1 commit
482e743
089077d
ce6c05a
773a619
f80fec1
15e4e37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,11 @@ namespace com.keyman.dom { | |
*/ | ||
enablementObserver: MutationObserver; | ||
|
||
/** | ||
* Tracks changes in inputmode state. | ||
*/ | ||
inputModeObserver: MutationObserver; | ||
|
||
/** | ||
* Tracks a list of event-listening elements. | ||
* | ||
|
@@ -99,6 +104,9 @@ namespace com.keyman.dom { | |
if(this.attachmentObserver) { | ||
this.attachmentObserver.disconnect(); | ||
} | ||
if(this.inputModeObserver) { | ||
this.inputModeObserver.disconnect(); | ||
} | ||
|
||
for(let input of this.inputList) { | ||
this.disableInputElement(input); | ||
|
@@ -233,6 +241,7 @@ namespace com.keyman.dom { | |
|
||
if(!this.isAttached(Pelem)) { | ||
this.setupElementAttachment(Pelem); | ||
Pelem._kmwAttachment.inputMode = Pelem.inputMode ?? 'text'; | ||
Pelem.inputMode = 'none'; | ||
} | ||
|
||
|
@@ -264,7 +273,13 @@ namespace com.keyman.dom { | |
*/ | ||
disableTouchElement(Pelem: HTMLElement) { | ||
// Do not check for the element being officially disabled - it's also used for detachment. | ||
Pelem.inputMode = 'text'; | ||
const intendedInputMode = Pelem._kmwAttachment.inputMode; | ||
jahorton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
this.disableInputModeObserver(); | ||
Pelem.inputMode = intendedInputMode; | ||
this.enableInputModeObserver(); | ||
|
||
this.setupNonKMWTouchElement(Pelem); | ||
} | ||
|
||
/** | ||
|
@@ -907,6 +922,30 @@ namespace com.keyman.dom { | |
} | ||
}.bind(this); | ||
|
||
_InputModeObserverCore = function(this: DOMManager, mutations: MutationRecord[]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment explaining the purpose of this function? Also I think we should be naming it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other observer core functions have similar capitalization:
Should I change them, too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's probably good sanitisation to iteratively cleanup to javaScript camelCasing preference, so yeah, if it's not too big a yak shave, sure! |
||
const keyman = com.keyman.singleton; | ||
// Prevent infinite recursion from any changes / updates made within the observation handler. | ||
this.disableInputModeObserver(); | ||
try { | ||
for(const mutation of mutations) { | ||
const target = mutation.target as HTMLElement; | ||
if(!(this as DOMManager).isAttached(target)) { | ||
continue; | ||
} | ||
|
||
const newValue = target.inputMode; | ||
|
||
target._kmwAttachment.inputMode = newValue; | ||
jahorton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if(keyman.util.device.touchable) { | ||
target.inputMode = 'none'; | ||
} | ||
} | ||
} finally { | ||
this.enableInputModeObserver(); | ||
} | ||
}.bind(this); | ||
|
||
/** | ||
* Function _MutationAdditionObserved | ||
* Scope Private | ||
|
@@ -1611,6 +1650,12 @@ namespace com.keyman.dom { | |
observationConfig = { subtree: true, attributes: true, attributeOldValue: true, attributeFilter: ['class', 'readonly']}; | ||
this.enablementObserver = new MutationObserver(this._EnablementMutationObserverCore); | ||
this.enablementObserver.observe(observationTarget, observationConfig); | ||
|
||
/** | ||
* Setup of handlers for dynamic detection of change in inputMode state. | ||
*/ | ||
this.inputModeObserver = new MutationObserver(this._InputModeObserverCore); | ||
this.enableInputModeObserver(); | ||
} else { | ||
console.warn("Your browser is outdated and does not support MutationObservers, a web feature " + | ||
"needed by KeymanWeb to support dynamically-added elements."); | ||
|
@@ -1630,6 +1675,16 @@ namespace com.keyman.dom { | |
return Promise.resolve(); | ||
}.bind(this); | ||
|
||
enableInputModeObserver() { | ||
const observationTarget = document.querySelector('body'); | ||
const observationConfig = { subtree: true, attributes: true, attributeFilter: ['inputmode']}; | ||
jahorton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.inputModeObserver.observe(observationTarget, observationConfig); | ||
} | ||
|
||
disableInputModeObserver() { | ||
this.inputModeObserver.disconnect(); | ||
} | ||
|
||
/** | ||
* Initialize the desktop user interface as soon as it is ready | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be done again in
enableInputElement()
, called on line 247, albeit in a more nuanced manner (handling iframes)