mixxx
mixxx copied to clipboard
mixxxdj
DlgTagFetcher new feedback system
As we discussed that on Zulip, we wanted to get rid of the full screen status page in Import metadata from MusicBrainz - DlgTagFetcher.
The new GUI is simply like this:
-
On startup, progress
-
After fetching completed, successful
-
Any case of error, failure:
What do you think generally?
Thank you. This already better for me than the original.
Just looking at the layout the buttons are looking a bit "random" ordered, unfortunately I have not yet a better idea. Here my points:
- The Apply button is not visually connected to the selected line which should be applied.
- The Retry button is very prominent visible. Maybe it helps to gray them out when not in action.
Would it be better to have retry between Previous and Next? Unsure because that break them apart ... @ronso0 Do you have Ideas?
Other issues:
- You can select (highlight) the existing set of metadata, is this necessary?
- The progress bar starts with a few small steps and the jumps to 100 %. That can be better scaled.
- We can even start it at 10 % and progress form there.
- After Retry the bar sticks at 100 %. I have expected that it falls back to 10 % or such
Most of the issues where already part of the existing layout, so it is not necessary required to solve them here.
Thank you for the feedback. I'm glad that you liked it.
I have two ideas regarding with the retry button.
The Retry button is very prominent visible. Maybe it helps to gray them out when not in action.
What I thought at the first place, I also mentioned than on Zulip, we can put an icon instead of the text Retry, with that the button can be little in width and height can stay still. So the button will be square.
Making them gray can be an option, we can also make it invisible by `setVisible' if the fetch is processing it can be invisible in case of failure or success the retry button can appear. So we can get user's attention. What do you think?
The Apply button is not visually connected to the selected line which should be applied.
I didn't get that part, should the apply next to the found metadatas.
You can select (highlight) the existing set of metadata, is this necessary?
It was like this before, I didn't want to change the original version. But yeah, I realize that it is not necessary. I thought it can be useful for the cover art fetcher, but not necessary for this too.
The progress bar starts with a few small steps and the jumps to 100 %. That can be better scaled.
This is happening due to less available metadata. Normally the scaling works related with the number of metadatas. For the less available metadata I think manual scaling can be done.
We can even start it at 10 % and progress form there.
This is an also good approach,
After Retry the bar sticks at 100 %. I have expected that it falls back to 10 % or such.
Oh I didn't notice that until now, Thanks for letting me know.
With the quick tests that I did, this is happening with the same as above, less available number of metadatas. I will fix it asap :+1:
I have the case that the dialog is at 100% forever with: "Fetching track data from the MusicBrainz database" It looks like an error case is not full handled.
The "Fetching track data from the MusicBrainz database" issue remains even after Retry. I have to reopen the dialog to make it work again.
Can you also implement the graying out of Retry and Apply? Yeah, I can do that.
We can also make Retry button invisible unless if there is a failure. So user can only interact-see it if there is a failure. Non button is better than gray button IMHO. What do you think?
I have the case that the dialog is at 100% forever with: "Fetching track data from the MusicBrainz database" It looks like an error case is not full handled.
The "Fetching track data from the MusicBrainz database" issue remains even after Retry. I have to reopen the dialog to make it work again.
Sometimes some tracks have special cases. Such as the track has only one AcoustID but many MusicBrainz releases or many AcoustID's or just one for each. These can cause different behavior on the status bar.
I haven't encountered these in my tests. It can be because of the tracks that I have in my library, I need to expand my library in order to test it better.
Could you please give me the songs that these errors occurs in your library? So I can have a better perspective.
Non button is better than gray button IMHO. What do you think?
In this case yes. Is there a use case to "retry" even if we have results, probably not.
Could you please give me the songs that these errors occurs in your library?
Today o cannot reproduce the issue, so I cannot identify a specific track. I did not expect that this is a track related issue, more a network responds thing. We need to keep an eye on it.
Today o cannot reproduce the issue, so I cannot identify a specific track. I did not expect that this is a track related issue, more a network responds thing. We need to keep an eye on it.
Exactly, we need to keep an eye on it. I have been trying to detect why it is happening. It happened to me once. I added a lot of track into my library to find out. But I couldn't find out why exactly it is happening. All I can say that this is no related with the amount of metadata found as I mentioned earlier.
It actually get results from the MusicBrainz, but It doesn't update the stack. I can see the network result on console it says received network reply after ***ms but not on stack no result displayed. It is not happening for the same track after closing and re-opening or changing the track with the prev-next buttons.
I will work on it more to solve it :+1:
Thanks for all the feedback :)
I have tried to fix all the errors mentioned above. I think it has a good use case.
I have also made retry button invisible if the track has no metadata available on MusicBrainz database. So with that, I prevent over requests for these tracks. The button will be only displayed if the user lost network connection while fetching, or the track metadata didn't fetched due to 403 (there is still an interesting issue regarding with it) .
Sorry I'm a bit late to the party..
Would it be better to have retry between Previous and Next? Unsure because that break them apart ... @ronso0 Do you have Ideas?
First impression when looking at the successful query: the status bar is very prominent (maybe it's just your theme but I doubt it's less prominent in others). Can we show the status bar only when fetching metadata, then replace it with an unobrusive label like "query successful"?
Same for the the Retry button: it is only required in case something goes wrong / gets stuck. On success it can be ~~hidden~~ greyed out.
We can also make Retry button invisible unless if there is a failure. So user can only interact-see it if there is a failure. Non button is better than gray button IMHO. What do you think?
IMO it's better to always show all buttons and disable / enable them when necessary.
Hey @ronso0 Welcome to the Party! Thanks for the feedback.
I will make all the button visible, and also I will try to change the progress bar to the label if the fetching successfull. I think that this will be better in user case.
I have another question related with the cover art fetcher. It is quite early but i wanted to ask. Maybe we should considwe it after the cover art fetcher PR update after this one.
So my question is, We also want to use the Progress Bar while fetching the cover art thumbnails to inform user about the status. I can not be sure if we should add another progress bar for the cover art fetcher (below the layouts), or should we use this existing one. Would this design look good when we implement the cover art fetcher?
With the latest changes Tag Fetcher Dialog looks like this after success.
I have also added 2 launchpad bug related links in order to increase the success of the importing metadata. If we are able to fix those, that will be better, success will be increased and we won't need to inform user for rate-limiting or empty XML.
This is my view from this branch. Unfortunately the text is cut of.
Did you consider to remove the test on to p of the window?
The progress bar works good. It is only a pity that the dialog is not yes populated on the fly.
With the last changes,
-
I have tried to add QTimer. Now every request done in a second without sleep and hot loops. That was also mentioned on launchpad. Launchpad Bug
-
I have also tried to solve the bug '404/Empty-XML' mentioned on launchpad
-
I think now, there is no cut of issue aswell.
-
Retry button was still was enabled in some situations which shouldn't, that fixed.
-
At last, namings are changed.
I wanted to request for two. I guess, I can't request two reviews at the same time. Sorry if there are a lot of notifications, I couldn't get it why, it was rejecting the other request when I ask for a request.
This PR fixes a bug in 2.3 that popular tracks cannot be looked up when they have been released on many recordings. Instead of a usable error message it just times out. https://bugs.launchpad.net/mixxx/+bug/1983204
See now some options to continue:
- Rebase this on 2.3 if it is possible without conflicts (need to check)
- Only use the backbend fix without a GUI feedback.
- Re-create the simple sleep function from the early state of this PR for 2.3 without GUI feedback.
- Don't fix the issue in 2.3
I am heading to 3.
@ronso0 what do you think? Does this PR work for you regarding the UX?
I totally agree with the option 3. IMHO, This can extended and divided into 3 different PR's.
Because this PR actually tries to solve 2 bugs at the same time. Rate Limit and 404 - Empty XML. These 2 can be solved in 2.3 (I can have different reviews for these two that would be better for me) and then GUI can be the third one.
What do you think?
Ok, than it would be nice if you could create a PR with just the QThread::sleep() call. This is IMHO a good band aid, even though abort will still not work, just like before.
Ok, than it would be nice if you could create a PR with just the QThread::sleep() call. This is IMHO a good band aid, even though abort will still not work, just like before.
Thank you, I will open just a pr with the QThread::sleep() call, what do you think about 404 issue, looping tasks?
what do you think about 404 issue, looping tasks?
This can be also a separate PR against 2.3, right?
Since the bugs are resolved I have rebased it on upstream/main, this PR only has the GUI changes now.
The retry button makes me think that we also need a cancel button. Can this replace the retry button in case the fetching is in progress?
The retry button makes me think that we also need a cancel button. Can this replace the retry button in case the fetching is in progress?
I think that would be confusing as a user case. What do you think @ronso0 ?
I would prefer Close instead.
After close, you cannot retry. Or can we dispose retry? User can always close and retry.
After close, you cannot retry. Or can we dispose retry? User can always close and retry.
Retry works really well especially these days. I don't know what situation others have, but these days I can not connect to AcoustID time to time. So I need to try again for a few times then it works just fine.
Normally the AcoustID wasn't giving this response when I started to implement this feature.
Also it helps if the connection is not stable.
As an addition, we can disable the retry button if there is no track data on MB database (just in case to not send redundant request). If the error is different than that (Can't connect for example), retry button will be there to try again.
OK, than lets stick with the current situation: A Retry button but no cancel.