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

Show album cover art #568

Merged
merged 10 commits into from
Dec 20, 2024
Merged

Show album cover art #568

merged 10 commits into from
Dec 20, 2024

Conversation

jacksongoode
Copy link
Collaborator

This allows users to click on the cover art of a currently playing song and see it in higher quality.

@jacksongoode
Copy link
Collaborator Author

@SO9010 Interested in seeing if there are other suggestions. Maybe add a download button on right click? Esc button could trigger it's closing?

@SO9010
Copy link
Contributor

SO9010 commented Dec 13, 2024

I think having a download option would bear TOS sadly.

@SO9010
Copy link
Contributor

SO9010 commented Dec 13, 2024

I will have a better look soon :)

@jacksongoode
Copy link
Collaborator Author

I think having a download option would bear TOS sadly.

I actually don't know if this would be a TOS breach? I can't really find any information on this?

@kingosticks
Copy link

I think having a download option would bear TOS sadly.

I actually don't know if this would be a TOS breach? I can't really find any information on this?

I think it would be. You are not supposed to store any Spotify content, anything that comes from their API. See section 4, sub-section 3. There's a specific mention for coverart where they explicitly say it can be temporarily cached.

Local caching. Do not locally cache any Spotify Content, except as strictly necessary to enhance the performance of your SDA and its functionality, and limited to the temporary caching of:

metadata and cover art; or

Obviously apps like this probably also want to cache audio data, but technically their public API doesn't actually provide that...

@jacksongoode
Copy link
Collaborator Author

Specifically, I need some testing around Windows & Linux appearance of the cover art

@SO9010
Copy link
Contributor

SO9010 commented Dec 15, 2024

Having a look on windows now :)

@SO9010
Copy link
Contributor

SO9010 commented Dec 15, 2024

Okay, so on windows, the window size isn't quite the same as the image, so there are borders on the side, which make it look a bit odd. Also, note the fact that the window doesn't have a name. It feels incomplete having a whole window to itself; maybe we could take this as an opportunity to have a mini-player? This could be pretty simple by just adding the controls at the bottom.

Maybe there's a way to have a pop-up that doesn't have a frame, can't move, and disappears when the focus changes. A bit like a right-click menu?

Also, another thing that is needed is for the image to have rounded corners.

image

@jacksongoode
Copy link
Collaborator Author

jacksongoode commented Dec 16, 2024

Yeah I figured that wouldn't work in Windows. In macOS you get a nice transparent menu. So maybe for windows we just keep the title and add the name of the artist - album in the window title. Would you be able to add that conditional logic @SO9010 since you can test it on Windows?

image

I think the mini-player is an interesting direction with this, but maybe out of scope for this small PR. I also think having the album art its own window is nice from an aesthetic point of view, just so it can take up as much space as we want it to.

@jacksongoode
Copy link
Collaborator Author

@SO9010 I believe this now looks proper on Windows.

@SO9010
Copy link
Contributor

SO9010 commented Dec 20, 2024

Checking it out on linux now!

@SO9010
Copy link
Contributor

SO9010 commented Dec 20, 2024

Looks lovely, I really like the fact that you can close it with esc! I think it would be nice if we also had that for the credits window! I submitted a PR.

@jacksongoode
Copy link
Collaborator Author

Super, I merged that PR in and I'll merge this!

@jacksongoode jacksongoode merged commit da32333 into main Dec 20, 2024
5 checks passed
@jacksongoode jacksongoode deleted the jackson/show-album-cover branch December 20, 2024 03:53
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.

3 participants