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: Allow plugins to override navigation #2872

Merged
merged 3 commits into from
May 6, 2020

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented May 5, 2020

variation of #2307

changes:
Android method renamed from shouldOpenExternalUrl to shouldOverrideLoad (more similar to the Android WebView method, and matches the iOS name). Logic changes, false load, true abort, null default.

ios method renamed from shouldOverrideLoadWith to shouldOverrideLoad, with a single WKNavigationAction param instead of two (you can get both request and navigationType from it).
Method return type changed from Bool to NSNumber so it can return nil.
And as it can return nil now, change logic to work as on Android (false load, true abort, nil default)

closes #2307
closes #70

Copy link
Contributor

@sbannigan sbannigan left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@imhoffd
Copy link
Contributor

imhoffd commented May 5, 2020

Only one plugin can do this in an app, right?

@jcesarmobile
Copy link
Member Author

jcesarmobile commented May 5, 2020

There can be multiple plugins, but totally depend on them, if they only handle certain url and return null/nil for other cases, then another plugin can take care of other URLs, if they return true or false for all URLs then other plugins won’t be able to handle them as it gets the value from the first plugin that returned true or false.

@imhoffd
Copy link
Contributor

imhoffd commented May 5, 2020

Gotcha.

@jcesarmobile jcesarmobile merged commit 41f9834 into ionic-team:master May 6, 2020
@jcesarmobile jcesarmobile deleted the shouldoverride branch May 6, 2020 09:39
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.

Add network request handling from cordova-plugin-ionic-webview
3 participants