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

Auto-fill album for artist and track (closes #88) #250

Merged
merged 7 commits into from
Sep 15, 2024
Merged

Auto-fill album for artist and track (closes #88) #250

merged 7 commits into from
Sep 15, 2024

Conversation

mishmish-dev
Copy link
Contributor

@mishmish-dev mishmish-dev commented Aug 25, 2024

The feature uses last.fm API trackGetInfo to retrieve an album name based on track + artist.

To complete this, I'd ask for help with a couple of things:

  1. Translation, I added two new tooltips;
  2. Creating some animation telling user of success/failure of the operation.

Things to consider: rate-limit last.fm API calls? Delegate to backend?

@mishmish-dev mishmish-dev changed the title Suggest album for artist and track (closes #88) Auto-fill album for artist and track (closes #88) Aug 26, 2024
@mishmish-dev
Copy link
Contributor Author

@elamperti
Regarding item 2, I could try to re-use the catch-paste alert, but how to test alerts locally?

@elamperti
Copy link
Owner

The alerts use a context, so you can dispatch messages easily. I think that, for this feature, perhaps it would be more appropriate to show a spinner within the input (when loading) and create a "shake" animation if there's a failure (or some other indicator around this field).

@mishmish-dev
Copy link
Contributor Author

Hmm, when I run OpenScrobbler locally, the catch-paste thing doesn't work.

Anyway, I would be glad if you add this animation. I am really not a frontend guy.

@elamperti
Copy link
Owner

Sure! I'll look into it.

To catch pasted text, you should have the corresponding option enabled (from the settings modal)

@mishmish-dev mishmish-dev marked this pull request as ready for review September 3, 2024 15:34
@mishmish-dev
Copy link
Contributor Author

Hi @elamperti
I implemented shakey animation for album auto-fill failure, as well as highlighting the input field with cyan/red on success/fail

@elamperti
Copy link
Owner

Awesome! I'm a bit backed up with a few other upcoming changes but I'll review and merge as soon as possible. Thanks for your work and patience! 😁

@mishmish-dev
Copy link
Contributor Author

Resolved conflict with the main branch in previous commit

@elamperti
Copy link
Owner

Thank you! I'm finishing with a few adjustments on #237 and I'll get to this one \o/

Copy link
Owner

@elamperti elamperti left a comment

Choose a reason for hiding this comment

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

Fantastic! Great job adding the animation as well 😃

@elamperti elamperti merged commit 6daffcf into elamperti:main Sep 15, 2024
3 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