search

brim-borium / spotify_sdk

Flutter Package to connect the spotify sdk
https://pub.dev/packages/spotify_sdk
Apache License 2.0
153 stars 82 forks source link

Bump iOS SDK to v3.0.0 #222

Closed fotiDim closed 1 month ago

fotiDim commented 1 month ago

Spotify released today v3.0.0 of the iOS SDK and they removed the checkIfSpotifyAppIsActive method. @brim-borium please verify if it will still be supported on Android.

brim-borium commented 1 month ago

@fotiDim we currently don't use any method from the sdk itself but do something like this for android

.setResultCallback { playerState ->  result.success(!playerState.isPaused && playerState.playbackSpeed > 0)}

So this should still work

brim-borium commented 1 month ago

@fotiDim we currently don't use any method from the sdk itself but do something like this for android

.setResultCallback { playerState ->  result.success(!playerState.isPaused && playerState.playbackSpeed > 0)}

So this should still work

Maybe this is something we can adapt for iOS as well ?

fotiDim commented 1 month ago

@fotiDim we currently don't use any method from the sdk itself but do something like this for android

.setResultCallback { playerState ->  result.success(!playerState.isPaused && playerState.playbackSpeed > 0)}

So this should still work

Maybe this is something we can adapt for iOS as well ?

Good point. I will make a commit. Then this PR is not even a breaking change.

EDIT: On second thought, we added the Android implementation only to have feature parity with iOS. Now that the native iOS method is gone maybe we should completely remove it from all platforms as it is custom business logic that by convention does not belong to the library. It could also be implemented by the end developer. I will make the commit anyway and if we decide against it I can remove it.

fotiDim commented 1 month ago

@brim-borium I have made the change as suggested. Let me know if you prefer to keep it or not, like I mention in my comment above.

brim-borium commented 1 month ago

@fotiDim we currently don't use any method from the sdk itself but do something like this for android

.setResultCallback { playerState ->  result.success(!playerState.isPaused && playerState.playbackSpeed > 0)}

So this should still work

Maybe this is something we can adapt for iOS as well ?

Good point. I will make a commit. Then this PR is not even a breaking change.

EDIT: On second thought, we added the Android implementation only to have feature parity with iOS. Now that the native iOS method is gone maybe we should completely remove it from all platforms as it is custom business logic that by convention does not belong to the library. It could also be implemented by the end developer. I will make the commit anyway and if we decide against it I can remove it.

You are right the I will remove the android code on this branch as well and then we have feature parity to the sdks

brim-borium commented 1 month ago

@fotiDim I removed the android method and removed it from the sdk and the example as well. Can you remove it from iOS and after that we can merge this

fotiDim commented 1 month ago

@brim-borium done. Please double that all is all right.

brim-borium commented 1 month ago

@brim-borium done. Please double that all is all right.

Will do, can you check iOS?

brim-borium commented 1 month ago

@fotiDim android looks good you can merge if iOS is fine as well :)

fotiDim commented 1 month ago

All good. Merging.