fix: delays kmw-dependent code 'til full page load #28
Conversation
| // Promises only started being supported in KMW 13 (keymanapp/keyman#1432), but this site supports | ||
| // earlier versions, too, so we can't rely on their presence. |
There was a problem hiding this comment.
Happy for this site to support 13.0+ at this point if that makes things more elegant?
| setTimeout(function(){ | ||
| if (!!$('.messageBox').length) { | ||
| getKeymanWeb().moveToElement('message'); | ||
| } | ||
| // On touch devices, this is necessary (but not sufficient) for ClipboardJS compatibility. | ||
| // Must take affect AFTER KMW has initialized. | ||
| $('#message').removeAttr('disabled'); | ||
| },1000); |
There was a problem hiding this comment.
Is setTimeout really needed here now? Would be awesome if we could eliminate it.
There was a problem hiding this comment.
It would be. That's the part that needs KMW 13.0.
If KMW's script is the last thing to load on the page, keyman.init may not have been called, meaning the touch-aliases won't have been built yet. If we could wait on init's Promise, we could eliminate this timeout in favor of that.
Assuming that making an init call here is safe, granted: this function doesn't know whatever options the site may be setting elsewhere for its init.
There was a problem hiding this comment.
So... if we make this 13.0+ then we can eliminate the timeout? Sounds good to me.
There was a problem hiding this comment.
I've gone ahead and made this into #29. That way, we get the current batch of work out, and just in case there is any issue with use of init-promises, we can more easily revert just that part.
Fixes #26.
Mitigates (and may fix) #27.
Moves all KMW-dependent code (including the 'kmw-disabled' removal bit that fixes #26) until the window has fully loaded, meaning that all script resources (like KMW) have loaded. The old placement of the code could trigger at the 'interactive' state, and before KMW's script loaded at that... which would naturally cause null reference errors.
Note that the clipboard may not work on old iOS devices, but that limitation is due to Safari's support for the underlying command - it's only supported at and after iOS 10. Unfortunately, the mobile layout gives no indication of failure due to use of an older iOS version.
There are a few additional spots in code that may allow #27 to continue occurring; additional work is suggested in these regards. This PR should still mitigate things, though, and releasing the current fixes should help evaluate the scale needed for any remaining work in that direction.