cordova-plugin-opentok icon indicating copy to clipboard operation
cordova-plugin-opentok copied to clipboard

#10 fix

Open wolfenrain opened this issue 6 years ago • 10 comments

Fix for some changes in #10

General:

Changed the way how scrolling of native elements work. We add a scroll EventListener to the document. And then on scroll we check if the scrolling target contains video-elements with the data-stream-id property. If so then we send the position(relative to the viewport, changed in the getPosition) of the video-element with the stream-id to the native side to update its position.

By doing this JavaScript implementation, we are now capable of scrolling multiple native video-views, independently within their own HTML parent. Which wasn't possible before using native scrollviews.


There are also some changed that are platform specific.

Android:

  • Removed the observable scroll view. It caused the webview to become as high as its content, instead of as high as the viewport.

iOS:

  • Added the cordova-plugin-wkwebview-engine as its main WebViewEngine, because the default from Cordova is the UIWebView, and it stops javascript-execution while you are scrolling. This dependency is only needed until Cordova implements the wkwebview self.
  • We also added a check if the cordova-plugin-statusbar is loaded. Because that plugin changes the origin position of the webview. Normally the top position is 0 and its below the statusbar. But that means that the native views should have 20 pixels more. Otherwise they will be on top of the statusbar. But when the StatusBarPlugin is loaded, the top position of the webView is on top of the statusbar(still 0). So a small ifstatement is made to check if we should add a small offset or not.

Those are in general the major/important changes.

wolfenrain avatar Mar 01 '18 12:03 wolfenrain

@msach22 This one is in WIP

mark-veenstra avatar Mar 07 '18 16:03 mark-veenstra

@msach22 Removed WIP, should all be fixed now.

wolfenrain avatar Mar 08 '18 15:03 wolfenrain

@wolfenrain @mark-veenstra have you noticed any performance issues or lagging with this scrolling solution? AFAIK, the scroll event is only available after the scroll has completed, is this the same in this case?

msach22 avatar Mar 15 '18 00:03 msach22

@msach22 only on iOS we experienced the issue that the scroll event was available after releasing the scroll. But this is solved in the new WKWebView of iOS. Therefore we added the dependency on WKWebView, which is also the future WebView for Cordova instead of the UIWebView. So this dependency will be obsolute over time.

On Android it worked without any other addition.

mark-veenstra avatar Mar 15 '18 06:03 mark-veenstra

@msach22 that is correct. This PR contains the new WKWebView, because the original UIWebView is blocking javascript when scrolling. So you only get the scroll event after you are done scrolling. That has been fixed in the new WKWebView

Also put the PR back to WIP because there are a few more things I need to be sure about before we merge.

wolfenrain avatar Mar 15 '18 08:03 wolfenrain

@msach22 I removed the WIP, ready for merge.

wolfenrain avatar Mar 19 '18 07:03 wolfenrain

@msach22 Can we get this one out? Maybe in a 3.4.0?

mark-veenstra avatar May 07 '18 06:05 mark-veenstra

@wolfenrain @mark-veenstra Yes, let's aim to get to this out in the next feature release. I'll review this PR in the upcoming week.

msach22 avatar Jun 08 '18 15:06 msach22

@msach22 @mark-veenstra According to this blog post by Cordova, the WKWebview is gonna become the default in new iOS platform version(5). Considering that its related to this issue, because its added as a dependency, I thought it would be useful to reference it here.

wolfenrain avatar Aug 09 '18 06:08 wolfenrain

Also asked a question about this plugin and the new bridge within the [email protected] here.

mark-veenstra avatar Aug 09 '18 06:08 mark-veenstra