Support searching Soundcloud for playback in BrainzPlayer #2506
Conversation
There was a problem hiding this comment.
Looking good !
Hope you don't mind, I jumped in for a preliminary review of the react bits.
You can safely ignore my rambling about refactoring the refreshToken methods, they're mainly notes for myself.
Let me know if you need any UI work that I can help with.
| @@ -86,6 +86,7 @@ export default class UserTaste extends React.Component<UserTasteProps> { | |||
| listenBrainzAPIBaseURI={APIService.APIBaseURI} | |||
| refreshSpotifyToken={APIService.refreshSpotifyToken} | |||
| refreshYoutubeToken={APIService.refreshYoutubeToken} | |||
| refreshSoundcloudToken={APIService.refreshSoundcloudToken} | |||
There was a problem hiding this comment.
This is getting really annoying, isn't it?
Since each of those functions is just calling the APIService method with the name of the service as a string, I think we should be able to refactor it (separate PR, I don't mind getting in there and doing it) and just pass the refreshAccessToken method to BrainzPlayer.
Then in BrainzPlayer we can pass the prop along, for example
<SoundcloudPlayer
...
refreshToken={refreshAccessToken}
And finally, in SoundcloudPlayer, we can call this.props.refreshToken(this.name) <- the name string is already stored in the class.
Anyway, I'll have a look at refactoring that once this PR is merge, so as not to be disruptive :)
amCap1712
changed the title
Interim checkin
update some test descriptions now that SoundCouldPlayer is not added by default when user is not signed into SC. Also some minimal cleanup of invalid props passed to the BrainzPlayer component in tests.
|
Server tests are failing, but apart from that I think it is ready to go into beta. |
BrainzPlayer already supported SoundCloud playback if a soundcloud id was present in the original listen. Now we have access to SoundCloud search APIs, so adding search support to SoundCloud Player in BP.