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

[addons] allow skins etc. to be activated via info dialog #7703

Merged
merged 3 commits into from
Sep 19, 2015

Conversation

tamland
Copy link
Member

@tamland tamland commented Aug 2, 2015

Allows addons that can only have one in use at a time (skins, gui sounds, languages, visualization and screensavers) to be "activated" in the addon info dialog without having to dig through settings.

Reuses the "launch" button for plugins.

@mkortstiege
Copy link
Member

In case an addon is already in use it would be nice to alter the button label instead of just disabling it. I also think Set as default instead of just Use would be the better label choice. Other than that i really like the change ;) Thanks.

@mkortstiege mkortstiege added Type: Improvement non-breaking change which improves existing functionality v16 Jarvis labels Aug 3, 2015
@tamland
Copy link
Member Author

tamland commented Aug 3, 2015

First part's a great idea:), but I'm not sure 'Set as default' would make sense here. (I'm not sure what any of it has to do with the "default" actually). E.g when you select a skin in the settings you select the skin you want to use, you don't select the skin you want to have as default.. Makes sense?

@mkortstiege
Copy link
Member

Well, if you select one of those addons it will become the default for the given section (eg. weather, skin, webif etc). Let's see what the others think about it.

@phil65
Copy link
Contributor

phil65 commented Aug 3, 2015

I agree with tamland that "set as default" doesnt really make sense in this context. Perhaps "Select", "Choose", "Set" or "Apply"?

@mkortstiege
Copy link
Member

"Choose" sounds good to me and the label is already there.

@tamland
Copy link
Member Author

tamland commented Aug 31, 2015

So are we going with Choose and Chosen(?). I'm going to need two labels, like Use / In use, not just one.

@tamland
Copy link
Member Author

tamland commented Sep 4, 2015

On second thought, having the disabled label show the state looks really weird. It's barely visible and not clear at all that it shows actual information. No other buttons does this. E.g "Update" is just disabled if it's already up-to-date.

I've done what I think works best here. It now shows Open, Run or Use depending on the type. Any objections to that?

@mkortstiege
Copy link
Member

Needs rebase. Could you please add some screenshots to make easier?

@tamland
Copy link
Member Author

tamland commented Sep 15, 2015

Rebased. I'm not sure what you want me to take screenshot of though @mkortstiege All it does is change the label of the "launch" button depending on the addon type. If not applicable it's disabled, like the current launch button.

@MartijnKaijser
Copy link
Member

concept sounds ok to me.

@mkortstiege
Copy link
Member

Could you please add some doxy so it's clear what's the difference between CanRun and CanUse and CanOpen without having to check the code in detail? Other than that no objections.

@tamland
Copy link
Member Author

tamland commented Sep 19, 2015

done. jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit 64f90f5 into xbmc:master Sep 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality v16 Jarvis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants