Skip to content

Add follow and unfollow playlist support#399

Merged
Insprill merged 11 commits intojpochyla:masterfrom
kespii:master
May 4, 2023
Merged

Add follow and unfollow playlist support#399
Insprill merged 11 commits intojpochyla:masterfrom
kespii:master

Conversation

@kespii
Copy link
Copy Markdown

@kespii kespii commented May 2, 2023

Users can now follow and unfollow playlists. Updated the UI [context menu] to include buttons for following and unfollowing playlists.

Copy link
Copy Markdown
Collaborator

@Insprill Insprill left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! In Spotify, there's a distinction between playlists you've created and those you've followed. You can't unfollow a playlist you created, only delete it. With this PR, if you unfollow a playlist you created, it'll still exist, but won't show in your library. If I'm misunderstanding something, please correct me, but I don't think this is intended behavior.

@kespii
Copy link
Copy Markdown
Author

kespii commented May 3, 2023

Yes, you are correct about the behavior related to unfollowing a playlist you've created. So far, I haven't seen an option to delete such a playlist.

On spotify web api concepts page spotify says:

We have no endpoint for deleting a playlist in the Web API; the notion of deleting a playlist is not relevant within the Spotify’s playlist system. Even if you are the playlist’s owner and you choose to manually remove it from your own list of playlists, you are simply unfollowing it. Although this behavior may sound strange, it means that other users who are already following the playlist can keep enjoying it. Manually restoring a deleted playlist through the Spotify Accounts Service is the same thing as following one of your own playlists that you have previously unfollowed.

As a workaround we can omit "Unfollow" menu item for playlists you've created while retaining the ability to follow and unfollow other playlists.

@Insprill
Copy link
Copy Markdown
Collaborator

Insprill commented May 3, 2023

Looking at it again, I think the current behavior is fine, and I was just misunderstanding Spotify's UI. As it says in the documentation you referenced, there's no such thing as deleting a playlist. When the Spotify UI says 'Delete', it simply unfollows it, and it can still be accessed and refollowed again if you know its ID.

The only other critique I have is the fact that there's no confirmation before deleting a playlist. I can easily see someone deleting a playlist they meant to copy the link to, and having no way of finding it again. This could be implemented by creating a small child window, similar to how the preferences window is handled, that just has some text and Yes/ No buttons. Implementing that will be a little complex, so if anyone has another idea feel free to share.

@kespii
Copy link
Copy Markdown
Author

kespii commented May 3, 2023

I've added a confirmation dialog window, but since I'm not very familiar with Druid, I may have made some kludges 🚓 concerning the handling of events in inappropriate places (new window creation and their environment). Additionally, I'm uncertain about the padding and window size of the confirmation dialog window.

Please, feel free to provide any corrections or suggestions, as I appreciate your input.

Comment thread psst-gui/src/ui/playlist.rs Outdated
Comment thread psst-gui/src/ui/playlist.rs Outdated
Comment thread psst-gui/src/ui/playlist.rs Outdated
Comment thread psst-gui/src/ui/playlist.rs
@kespii
Copy link
Copy Markdown
Author

kespii commented May 4, 2023

Thank you for the review!
It seems that I have incorporated all the suggestions into the code and corrected the typos.

Copy link
Copy Markdown
Collaborator

@Insprill Insprill left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@Insprill Insprill merged commit 2edb45d into jpochyla:master May 4, 2023
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