cordova-plugin-opentok
cordova-plugin-opentok copied to clipboard
opentok
#50 - Update the view when `<video>` elements are added or removed from the DOM (or when it's style or any parent style changes)
@wolfenrain @mark-veenstra If possible, could you please update the PR so we can get this merged in!
This would be great for 3.4.0 release
@wolfenrain @mark-veenstra It looks like this PR would break any Android device using API Level 16 or many other newer versions for that matter. It looks like MutationObserver has just been commoditized in the recent versions and will break applications that are using and older version for Android. Based on this, I'm very hesitant to merge this PR.
https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#Browser_compatibility
@msach22 Regarding the MutationObserver we are using this solution already in production since we migrated to TokBox. And it is working great, as is, from Android 5 and up as we know since we tested this. So I am a bit surprised about the compatibility they claim here (even if you look into the old compatibility tables on that website).
But on the compatibility table they also state that when you want support from Android API level 18 to 26 you can use the vendor prefix WebKit. So if we want to be absolutely sure we could say: var MutationObserver = MutationObserver || WebKitMutationObserver. Then it should be no problem at all, but as I said above on this moment it is not necessary to get it working on these API levels. @wolfenrain can you add this extra check?
Also note that Cordova is only supporting API level 19 ( Android 4.4) - 27 (Android 8) from the most recent version. See this link: https://cordova.apache.org/docs/en/latest/guide/platforms/android/#requirements-and-support. They also will dropped support when Android version dip below 5% on Google's distribution dashboard.
So Android level 16 is even not supported by the most recent Cordova version. Also the market share of API level 16 and 17 is too small to keep it supported, that is why Cordova dropped it also.
This PR helps you to update the views automatically. For everyone that has an unsupported WebView or browser they still can use the old API by calling updateViews() themselves.
So why shouldn't we merge it in, with the change mentioned before?
@msach22 @mark-veenstra Changes have been made, also implemented the check for MutationObserver, if we want to have more coverage we could implement a bigger shim, but as @mark-veenstra mentioned Cordova itself doesn't support lower then API level 19, so I dont see a direct use for it.
@wolfenrain @mark-veenstra Thanks for your comments. The OpenTok Android SDK supports Android API level 16 and above and this plugin currently supports Cordova 3.5.0 and above. This means that we would have to do a breaking change to add this functionality.
I would really like to see this feature get merged in because I think it would make using the plugin easier, but we cannot drop support for Android 16.
@msach22 If you still want to support API level 16 (Android 4.1.x) and 17 (Android 4.2.x), then this PR is blocking it. I understand you will not merge this in if this is the business decision.
But we can keep it open for users who want a seperate solution and merge it in themselves. Or when you drop support for those old versions, then it can be merged as well.
@wolfenrain @mark-veenstra Yes, unfortunately I cannot merge this in, but I will leave this open in case developers want to add it to their forks. Thanks again for the great PR!
@mark-veenstra @wolfenrain Do you think we can add conditional observers where if the platform is the supported version we can use this observer, if not they will not have this functionality?
It should be technically possible. It just dawned to me that a polyfill might be possible aswell? Could that be a more all round solution?
On Dec 17, 2018, 7:58 PM, at 7:58 PM, Manik Sachdeva [email protected] wrote:
@mark-veenstra @wolfenrain Do you think we can add conditional observers where if the platform is the supported version we can use this observer, if not they will not have this functionality?
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/opentok/cordova-plugin-opentok/pull/59#issuecomment-447958511
@wolfenrain Yea having a polyfill might be the best approach. Let me know what you think
As I said here we could use that polyfill. Haven't done an indepth look. But it does provide us with the functionality needed.