Cover Art Label & Cover Art Full Size composition with menu #4864
Conversation
|
Thank you. I think we can go out of draft state and ask for a second pair of unbiased eyes for review. |
|
Done 👍 Thank you for the help and the review! |
|
This LGTM. @fatihemreyildiz can you rebase out the experimental commits? |
|
(this still needs a 2nd approval : ) I can take a look at the UI in the next days. |
fatihemreyildiz
force-pushed
the
coverartmenu
branch
from
6a49502 to
8cf2206
Compare
Thank you, Done, all clear 👍 . All experimental commits are out and just one left. |
Oh, when I wrote that I must have confused this PR with the cover art fetcher. |
|
I already did a detailed review. We just need a second pair of eyes that I have not missed something obvious, because I am probably biased. @ronso0 so if there is nothing wrong from you perspective I would be happy about a merge so that we can start to integrate it in the new MusicBrainz dialog. Thank you. |
|
Alright, LGTM* |
Hey everyone!
I want to add cover art fetcher [on this PR], but for to do that I need to have another cover art label's without/different menu and DlgCoverArtFullSize. I have tried to make a base class without menu and inherit the existing label-full-size from that. But this caused as code duplication and new files which can make the code base crowded. Also, adding everything in a huge pr would make it difficult to review and also to merge. So, we wanted to move with small steps. Here is the first little PR of the cover art-fetcher.
To display cover arts without menu, I have tried composition approach, according to my tests, if there is no cover art menu passed to these two classes, it doesn't pop-up the menu on right click. In order for me to move forward, I need to find a best solution for to display cover art's with or without menu.
Any feedback would be so good!
Thanks in advance.