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

feat(ios+android): Add possibility to configure loading scheme #1810

Merged
merged 7 commits into from
Aug 23, 2019
Merged

feat(ios+android): Add possibility to configure loading scheme #1810

merged 7 commits into from
Aug 23, 2019

Conversation

bartwesselink
Copy link
Contributor

Fixes #1736. Because Capacitor uses a different origin on iOS for serving content, it means that certain data might get lost when migrating from Cordova to Capacitor (like local storage for example).

This PR adds functionality to specify the scheme in the config.

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Can you rename the config name to iosScheme? Because it should be configurable for Android too and they can't have the same name. (Not sure if you also want to try to do the Android part in this PR)

Also, don't use the getSchemeForConfig, just read the value once and put it into a regular variable in CAPBridge, and in CAPAssetHandler change the setConfig to a setScheme that sets an internal scheme variable.

Finally, after reading the value but before setting it, make sure it's allowed, there is a function to check if a scheme value can be used or not, and in case it's not, default to capacitor too.

@bartwesselink
Copy link
Contributor Author

Thank you for your feedback. I will apply it this week, and will add the functionality for Android as well.

@bartwesselink bartwesselink changed the title feat(ios): Add possibility to configure webview origin for iOS feat(ios+android): Add possibility to configure webview origin for iOS Aug 20, 2019
@bartwesselink bartwesselink changed the title feat(ios+android): Add possibility to configure webview origin for iOS feat(ios+android): Add possibility to configure webview origin Aug 20, 2019
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a minor change on Android and simplified the iOS code as it was crashing and I realized we didn't really need the scheme on the CAPAssetHandler.

Thanks for the PR, will merge once tests finish.

@jcesarmobile jcesarmobile changed the title feat(ios+android): Add possibility to configure webview origin feat(ios+android): Add possibility to configure loading scheme Aug 23, 2019
@jcesarmobile jcesarmobile merged commit cd28d25 into ionic-team:master Aug 23, 2019
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.

Cordova-plugin-ionic-webview and Capacitor use different ‘origins’ by default
5 participants