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

Make providers support react native #1286

Merged
merged 5 commits into from
Feb 24, 2019
Merged

Conversation

ifedapoolarewaju
Copy link
Contributor

@ifedapoolarewaju ifedapoolarewaju commented Feb 10, 2019

Providers can be instantiated with a storage module that behaves similarly to localStorage here

The auth URL can also be called similarly as done here, but with an additional redirect option specified. Like so:

const authState = btoa(JSON.stringify({ origin: location.origin, redirect: location.href }))
const link = `${this.Provider.authUrl()}?state=${authState}`

If this is done on react-native (on the same window, not a new one), Companion will redirect to the redirect value set after the OAuth dance is done with a uppyAuthToken query param passed.

@arturi
Copy link
Contributor

arturi commented Feb 11, 2019

🎉 🥇 Awesome!

Will this work if we use WebView for auth only? I guess yes, we get the token in the storage that can be used without WebView, directly in RN environment, right?

@ifedapoolarewaju
Copy link
Contributor Author

@arturi I have updated the PR so that the URL will contain the uppyAuthToken query param, so that react-native app can watch the url change event.

@@ -422,7 +422,7 @@ module.exports = class ProviderView {
}

handleAuth () {
const authState = btoa(JSON.stringify({ origin: location.origin }))
const authState = btoa(JSON.stringify({ origin: location.origin, redirect: 'http://localhost:3' }))
const link = `${this.provider.authUrl()}?state=${authState}`
Copy link
Contributor

Choose a reason for hiding this comment

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

http://localhost:3 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that's not meant to be there. Removed

@arturi
Copy link
Contributor

arturi commented Feb 21, 2019

Awesome news! Tested, seems to work with the browser. I’d need it merged in master, so I can easier test in RN branch. Left a small comment.

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