Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

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

Successfully merging this pull request may close these issues.

2 participants