-
Notifications
You must be signed in to change notification settings - Fork 82
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
Call AuthorizeAndPlayURI
on main thread
#218
Conversation
AuthorizeAndPlayURI
on main thread which internally calls UIApplication.openURL
AuthorizeAndPlayURI
on main thread which internally calls UIApplication.openURL
ios/spotify_sdk.podspec
Outdated
s.platform = :ios, '13.0' | ||
s.ios.deployment_target = '13.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s.platform = :ios, '13.0' | |
s.ios.deployment_target = '13.0' | |
s.platform = :ios, '12.0' | |
s.ios.deployment_target = '12.0' |
In their readme they say "iOS 12 or higher"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to maintain compatibility with iOS 12. Isn't there another way to call it on the main thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, but It's not possible to throw error from non-throwable completion block and I don't know a proper way. most Swift cool features are not supported in iOS 12. the last option is to use completion block for connectToSpotify
, which I don't want to try.
Supporting iOS 12 is still needed? considering 6 version behind current iOS 18.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shouldn't be throwing at all here. You may replace throw with self.connectionStatusHandler?.connectionResult?(FlutterError(code: "spotifyNotInstalled", message: "Spotify app is not installed", details: nil))
.
We match the iOS version support of the native SDK. When the official SDK drops iOS 12 we will drop it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can work on it tomorrow, Thursday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fyi: I fixed the linting issues, just rebase on the current main :). @haashem thanks for contributing to this package!
AuthorizeAndPlayURI
on main thread which internally calls UIApplication.openURL
AuthorizeAndPlayURI
on main thread
Fixed a bug caused by my previous PR.
Feel free to change PR title if needed.