-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Cover Art Fetcher: allow applying just the found cover #11938
Conversation
The status message seems clunky, couldn't that be conveyed better by greying out the apply button when its not possible to apply and then updating the "current cover art" preview if it was successfully applied? |
Thank you, that slipped through. IIRC I already fixed that in #11112, I'll look into either backporting the fix or do it manually in DlgTagFetcher. |
Actually, the 'Current Cover' should be updated after it was written to the track file succesfully (need to click Okay to overwrite the track file). |
Furthermore, there are some inconsistencies in the file update process: the tags are written directly whereas the cover art update is initiated but results (dialog/success/failure) are processed via signals/slots, so there's no guarantee the messages are displayed in the correct order.
I'll check whether that can be fixed while keeping the tr strings in place. |
fa5ea5d
to
d619183
Compare
d619183
to
efe82fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
this removes the message added in mixxxdj#11938 and shifts the cover/cover+tags message to the correct spots
Thanks for your review!
Follow-up is #12041 |
... + some layouts tweaks
before:
this PR:
Closes #11086