-
-
Notifications
You must be signed in to change notification settings - Fork 6.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
[Music]Information Provider dialog #12987
Conversation
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.
+1 Having tested this it's IMHO it's a big improvement to the workflow for adding music to DB. The old way was a right pain in the arse if anything needed changing.
Can't comment on the actual code however.
This is a nice UI improvement! definitely +1 And I thought you were a database guy... :) |
{ | ||
bool result; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
e7d0f1a
to
7fbb29e
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.
Just a couple of knee-jerks. The lesson I learn here is that if you just put off things like this for approximately 10 years, somebody eventually cleans up your mess! :D
|
||
#include <limits.h> | ||
|
||
#include "GUIDialogInfoProviderSettings.h" |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
||
void CGUIDialogInfoProviderSettings::OnSettingChanged(std::shared_ptr<const CSetting> setting) | ||
{ | ||
if (setting == NULL) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
||
void CGUIDialogInfoProviderSettings::OnSettingAction(std::shared_ptr<const CSetting> setting) | ||
{ | ||
if (setting == NULL) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
…aper addons, and their settings. Improve add music source work flow by making setting default music info providers available from a "settings" button on the "Add to library" yes/no prompt. Improve the UI for the Change Info Provider facility for artists and albums. Use a single scraper view of this dialog from the context menu on artists and albums nodes. This replaces using the SetContent dialog and a sub-menu in GUIWindowMusicNav with use of list of options on how to apply settings in Info Provider dialog itself.
7fbb29e
to
96beb2a
Compare
Thanks for code review @notspiff. I tend to copy exitsting code, and as styles evolve that is not always a good thing. |
Add a new dialog for selection of both artist and album scraper addons, and their settings.
The workflow when adding a music source is improved by making the default music scraper settings readily available, from a "settings" button on the "Add to library" prompt. This way it avoids cluttering the UI or adding unnecessary complexity for beginners.
Previously if a user wanted to change the music scraping settings (that scanning was about to use) they had to leave the music library, go to System>Media>Music to pick scrapers and then to each addon separately to change any settings. There are few scrapers to choose from, and settings are generally static but much better to be able to do this from where you are adding a music source.
The same dialog is also used to provide the UI for the facility to change information provider for artists and albums separately. This replaces the ugly sub-menu added by #12597 with a select list on the dialog itself.
This is based on a standard dialog so no skin changes needed.
I have had an early test build up and had positive feedback from @karellen and @jjd-uk