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 tooltips previewing URLs in ArtistLinks menu #227

Merged
merged 1 commit into from
Oct 14, 2023

Conversation

herrhotzenplotz
Copy link
Contributor

@herrhotzenplotz herrhotzenplotz commented Aug 8, 2023

Just one of those little features that I was missing when I was digging around in Spotify :-)

Also, this enables tooltips on the artist menu such that you can preview the URL you're about to open in your web browser.

EDIT: somehow I was confusing something here. Lemme go dig into this a bit tomorrow.

@herrhotzenplotz herrhotzenplotz changed the title Feature: Add list of external artist urls to ArtistLinks menu Draft: Add list of external artist urls to ArtistLinks menu Aug 8, 2023
@herrhotzenplotz herrhotzenplotz marked this pull request as draft August 8, 2023 22:13
@herrhotzenplotz herrhotzenplotz changed the title Draft: Add list of external artist urls to ArtistLinks menu Show tooltips previewing URLs in ArtistLinks menu Aug 9, 2023
@herrhotzenplotz
Copy link
Contributor Author

Ok after I looked at this again I found out that I was comparing two different artists before and after my changes. The second was showing more links than the first so I assumed that it worked. However, it turns out that the Spotify API seems to never actually return any useful value in the list of external URLs, so my changes (except for the tooltips) were a no-op. In any case, I still believe the tooltips are a useful feature.

@herrhotzenplotz herrhotzenplotz marked this pull request as ready for review August 9, 2023 14:38
Just a minor feature that I was missing when I was digging around
in Spotify. This enables tooltips on the artist menu such that you
can preview the URL you're about to open in your web browser.

Signed-off-by: Nico Sonack <[email protected]>
Copy link
Owner

@kraxarn kraxarn left a comment

Choose a reason for hiding this comment

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

Feels like either all links should have tooltips, or none, but I guess these links can be less obvious where they go, so it might be a good idea to show. Looks good! 😄

Also let me know if you don't have commit signing set up, and I'll temporarily disable it to merge this. It's mostly as a reminder for myself to not forget. You can just ignore the failed FreeBSD action. It does that sometimes, but for some reason it doesn't let me restart it from a pull request.

@herrhotzenplotz
Copy link
Contributor Author

Also let me know if you don't have commit signing set up, and I'll temporarily disable it to merge this. It's mostly as a reminder for myself to not forget.

I haven't bothered configuring commit signing with hg-git. Maybe I should if it is possible?

You can just ignore the failed FreeBSD action. It does that sometimes, but for some reason it doesn't let me restart it from a pull request.

I've done the testing on FreeBSD already 😄 and tbh this is a bit a fail on Github's side as they don't have true runners for FreeBSD.

@kraxarn
Copy link
Owner

kraxarn commented Oct 14, 2023

In this case I don't think it makes a huge difference, as I believe GitHub adds its own "Verified" status for code merged from within GitHub itself anyway.

I'll merge this as is, and you can configure GPG signing later if you want 😄
Thank you for your contribution! 😃

@kraxarn kraxarn merged commit a5aacee into kraxarn:master Oct 14, 2023
6 of 7 checks passed
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