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] move rollback feature to the update dialog #8167

Merged
merged 3 commits into from Oct 4, 2015

Conversation

tamland
Copy link
Member

@tamland tamland commented Oct 1, 2015

This fixes and unifies the rollback and update feature by listing all available version (local packages or remote) in one place, in the update dialog.

@tamland tamland added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality labels Oct 1, 2015
@tamland tamland force-pushed the rollback_rework branch 2 times, most recently from 4eafebb to 0bac8d6 Compare October 2, 2015 13:56
@stefansaraev
Copy link
Contributor

jenkins build this please

@mkortstiege
Copy link
Member

Could you change the dialog to highlight only the currently used one? btw, do we want the users to be able to replace currently used with currently used?

Other than that i really like it ;)

@tamland
Copy link
Member Author

tamland commented Oct 3, 2015

@mkortstiege As we don't track the origin there is no way to know which is the currently used one, only the version. I agree it's a bit weird to highlight all versions, but that's what can be done currently.

@mkortstiege mkortstiege added this to the Jarvis 16.0-alpha4 milestone Oct 3, 2015
@mkortstiege
Copy link
Member

OK.

@@ -259,6 +259,8 @@ bool CAddonInstaller::InstallFromZip(const std::string &path)
if (!g_passwordManager.CheckMenuLock(WINDOW_ADDON_BROWSER))
return false;

CLog::Log(LOGDEBUG, "CAddonInstaller: installing from zip '%s'", path.c_str());

This comment was marked as spam.

@tamland
Copy link
Member Author

tamland commented Oct 4, 2015

good to go then?
jenkins build this please

@mkortstiege
Copy link
Member

Jep, merge away once build successfully.

tamland added a commit that referenced this pull request Oct 4, 2015
[addons] move rollback feature to the update dialog
@tamland tamland merged commit ceef836 into xbmc:master Oct 4, 2015
@ronie
Copy link
Member

ronie commented Nov 16, 2015

@tamland can we display a '..' item in the select dialog if no versions are available?

empty lists in kodi are evil and break navigation.
see: http://forum.kodi.tv/showthread.php?tid=248012

empty

@MartijnKaijser
Copy link
Member

Or "none available" or something

@phil65
Copy link
Contributor

phil65 commented Nov 16, 2015

I think in that case the SelectDialog shouldnt even open, just a notification with "none available" would be enough.

@phil65
Copy link
Contributor

phil65 commented Nov 16, 2015

...or perhaps even disabling that button.

@razzeee
Copy link
Member

razzeee commented Nov 16, 2015

Disable that button in that case should be preferred

@tamland
Copy link
Member Author

tamland commented Nov 17, 2015

I dislike disabling it as it would make the info dialog lag again. it's an architectural problem. Now, the delay is at least contained to the update dialog.

@tamland
Copy link
Member Author

tamland commented Nov 17, 2015

This is really weird btw. Why would an empty list outright break navigation? if that's the case we probably have a lot more broken dialogs.

@MartijnKaijser
Copy link
Member

we prevented broken navigation by always adding an item iirc

@ronie
Copy link
Member

ronie commented Nov 17, 2015

This is really weird btw. Why would an empty list outright break navigation?

we can't focus a list if it contains no listitems.

skins have to define the id of the control that should be focused when a window/dialog opens.
in case of the select dialog, it's the id of the list.

if the list is empty, kodi can't focus it. as a result no control will be focused.
in that case, onleft/onright/onup/ondown navigations are undefined
and you're basically stuck.

@tamland
Copy link
Member Author

tamland commented Nov 18, 2015

Then the correct solution is to focus the close button. Possible to fix in skin?

@ronie
Copy link
Member

ronie commented Nov 18, 2015

nope.

@tamland
Copy link
Member Author

tamland commented Nov 21, 2015

see #8416 / #8417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue 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

7 participants