Skip to content

Plugin Audit proposal #28

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

Closed
wants to merge 0 commits into from
Closed

Plugin Audit proposal #28

wants to merge 0 commits into from

Conversation

stevengill
Copy link
Contributor

No description provided.

* demo of oauth workflow + demo of closing inappbrowser via js (url change)
* http://ngcordova.com/docs/plugins/oauth/
* get rid of insertcss & executescript. Use fileapi instead
* Recommendation: Keep. Needs a refactor. Remove insertcss & executescript. Encourage use through apis instead of window.open. Make demos doing oAuth. Maybe phonegap-plugin-oauth.
Copy link
Member

Choose a reason for hiding this comment

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

Something based on SFSafariViewController (iOS) and Chrome Custom Tabs (Android) would be ideal. Some OAuth providers (Fitbit, for instance) have started adding restrictions that you're not allowed to do the OAuth flow in non-browser web views.

@devgeeks
Copy link
Member

devgeeks commented Nov 7, 2015

Looking really good.

* Can we treat some urls consistency across all platforms? Possibly use Flex Air package for inspiration.
* ex: app:, temp:,
* cdvfile wasn’t consistent enough across platforms
* Recommendation: Keep. Work on creating more consistency across platforms. Especially with cdvfile. Create phonegap-plugin-file with improved api.
Copy link

Choose a reason for hiding this comment

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

Could use File wrapping in app-harness as one point of inspiration: https://github.com/apache/cordova-app-harness/blob/master/www/cdvah/js/ResourcesLoader.js#L149

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip!

downstream distro (PG file?) discussion wasn't supposed to be in here. Just removed that line

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.

7 participants