Skip to content
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

Remove unnecessary WKWebView/UIWebview logic #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

josh-m-sharpe
Copy link

@josh-m-sharpe josh-m-sharpe commented May 19, 2020

Following up from our convo here:
Telerik-Verified-Plugins#185

This PR merges in the changes I was making in that PR, which pretty much does the same thing, but also removes some additional unnecessary logic.

@tlacroix
Copy link
Owner

Hi @josh-m-sharpe, thanks for following up with your PR on the original project. I looked at it just after I submitted mine (on Telerik's repo), and it seemed a lot cleaner. I'll just need a bit of time to test it with a few of my apps before I merge it since there's no automated testing, but just by looking at the code, I'm 99.999% sure everything's fine. Thanks again!

@tlacroix tlacroix self-assigned this May 21, 2020
@tlacroix
Copy link
Owner

Tested and all good. Just one change request, since your PR removes all support for UIWebView:

Please add, in plugin.xml:

<plugin ...>
    <engines>
        ...
        <engine name="cordova-ios" version=">=5.1.1"/>
        ...
    </engine>
    ...
</plugin>

@josh-m-sharpe
Copy link
Author

@tlacroix would you mind adding/testing that bit? I don't use cordova, so I have no means to test/confirm that part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants