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

OSX Support #49

Closed
wants to merge 9 commits into from
Closed

OSX Support #49

wants to merge 9 commits into from

Conversation

Krivoblotsky
Copy link

Hello, everyone.

So, this is the pull request with OSX support. Basically I don't expect you will merge it very quickly, because I pretty sure you have 'Support OSX' task in your backlog and you have a plan how to do it, but this code works good for my iOS/OSX projects and I like to share it.

I've tried to do my best to not overcode with "#if os", but some parts of code still looks ugly (as for me. For ex. DropboxConnectController), maybe you'll have some suggestions how to simplify it?

I've also prepared quick working example, please check it out:
https://github.com/Krivoblotsky/SwiftyDropboxExample

Any feedback and questions are appreciated.
Thanks.

@Krivoblotsky Krivoblotsky mentioned this pull request Jan 9, 2016
@Krivoblotsky
Copy link
Author

UPD. I've refactored DropboxConnectController to get rid of those "ifdef os" madness, not it looks better a bit.
Krivoblotsky@6085268
I've also added basic iOS example to demonstrate the compatibility.
https://github.com/Krivoblotsky/SwiftyDropboxExample
Any feedback, questions and suggestions are very appreciated.
Thanks.

@varenc
Copy link
Contributor

varenc commented Jan 23, 2016

thanks for this! You're right that OS X is on our roadmap but since we haven't started yet it might make sense for us to work with you on this.

One high level piece of feedback: I believe that using WKWebView will create a fresh session for the user inside the application. We know that the hardest part of OAuth for users is just logging in. If you could send the user to Safari/any web brower directly then they'd leverage their existing logged in session as well have some additional comfort about where they're typing in their password. To make this work in the past we require the application associates it's self with a "db-<APP_KEY>://" URL scheme so that the web browser can trigger the application after the OAuth flow complete. I believe this is the approach our very old OS X SDK takes: https://www.dropbox.com/developers-v1/core/sdks/osx

Alternatively, maybe you could just use SFSafariViewController so that the browser window can leverage the user's logged in session. In practice many users can have trouble remembering passwords and they might have things like 2FA or SSO enabled so reducing friction here is pretty valuable.

That said, don't spend much time on this unless you're particularly excited. There's a chance we'll want to add this functionality entirely ourselves, though I'm optimistic we might be able to work together. Will figure out the plan for this soon.

@Krivoblotsky
Copy link
Author

Thanks for the reply.
I totally agree about an ability to use system browser instead of build-in web view. I've updated the PR with such functionality, its definitely quicker way to login because in most of the cases user will be already logged in.
Live Demo:
https://dl.dropboxusercontent.com/u/11819370/Videos/db.mov
See: Krivoblotsky@da173e8
Since there is no SFSafariViewController in AppKit, I use AppleEvents to handle external links clicked. (You did the same in your old SDK).
I also kept an ability to use build-in web view. (Actually I can't imagine the usecase when system browser is unavailable, but I decided the keep it for now. ).

Authentification code looks like much clearer now:

//Use direct auth
if application.canOpenURL(dAuthURL(nil)) {

    self.directAuth({ (nonce) -> Void in

        application.openURL(self.dAuthURL(nonce))
    })

//Use Browser Auth
} else if BrowserAuth.available() {

    self.browserAuth(self.authURL(), redirectHandler: { (url) -> Void in

        //Check for url returned
        guard let redirectURL = url else { return }

        //Handle it
        application.openURL(redirectURL)
    })

//Fallback to WebView Auth
} else {

    self.webViewAuth(self.authURL(), controller: controller, redirectHandler: { (url) -> Bool in

        if self.canHandleURL(url) {
            application.openURL(url)
            return true
        } else {
            return false
        }
    })
}

@scobbe scobbe force-pushed the master branch 4 times, most recently from b49e9b5 to b4bf702 Compare August 3, 2016 01:21
@scobbe scobbe force-pushed the master branch 10 times, most recently from 5fbad95 to 5253602 Compare September 16, 2016 18:06
@scobbe
Copy link
Contributor

scobbe commented Sep 16, 2016

Fixed by ab71ab5.

@scobbe scobbe closed this Sep 16, 2016
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.

None yet

3 participants