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] extend context menu system #7544

Merged
merged 7 commits into from Aug 8, 2015
Merged

Conversation

tamland
Copy link
Member

@tamland tamland commented Jul 18, 2015

This extends the context menu functionality as discussed in the original PR, by allowing addons to:

  1. Add multiple items with the same addon.
  2. Add sub-menus, and arbitrary nested sub-menus and items.
  3. Allow addons to hook into different core menus, and menus of other addons.

This deprecates the old xml definition. In the new scheme, definition must start with a <menu id="menu-id"> element which defines menu the following elements should be added to. Addons using the old <item> can be ported by wrapping it in a <menu id="kodi.core.main"> and moving the library attribute to the <item> element.

The new preferred way of adding a single top-level menu item is as follows:

<menu id="kodi.core.main">
  <item library="contextitem.py">
    <label>30000</label>
    <visible>true</visible>
  </item>
</menu>

Furthermore, menus may contain multiple nested menus an items. Example: creating a sub-menu with two items:

<menu id="kodi.core.main">
  <menu>
    <label>Sub-menu</label>
    <item library="contextitem1.py">
      <label>Item 1</label>
      <visible>true</visible>
    </item>
    <item library="contextitem2.py">
      <label>Item 2</label>
      <visible>true</visible>
    </item>
</menu>

Similar to the two core menus kodi.core.main and kodi.core.manage, menus may be shared or conditionally extend by other addons. For example, if two addons defines

<menu id="kodi.core.main">
  <menu id="context.menu.shared">
    <label>Shared</label>
    <item ...>...</item>
</menu>

using the same id, the two items will show in the same shared sub-menu.

TODO: VS/xcode files

@tamland tamland added the Type: Feature non-breaking change which adds functionality label Jul 18, 2015
@phil65
Copy link
Contributor

phil65 commented Jul 18, 2015

@tamland nice addition, thx!
Would it be possible to add the ability to somehow make use of the context menu addon settings for the visible condition in order to disable/enable specific context menu entries? (something like <visible>Addon.HasSetting(button1_enable)</visible> or <visible>Addon(my.addon.id).hasSetting(button1_enable)</visible>


if (StringUtils::IsNaturalNumber(m_label))
{
return m_addon->GetString(boost::lexical_cast<int>(m_label.c_str()));

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@MartijnKaijser
Copy link
Member

wouldn't it make sense to rename the two files instead of delete/add?

@Montellese
Copy link
Member

@MartijnKaijser: git automatically detects a rename but only if the content of the files are similar. Once they differ too much they are not detected as renames anymore.

Will review this later.

@tamland
Copy link
Member Author

tamland commented Jul 19, 2015

@phil65 It uses the same visibility conditions as everything else so if something like like that was added to GUIInfoManager it'll be available here too.

@Montellese
Copy link
Member

Would it make sense to split the CContextMenuItem into an additional CContextMenuGroup which is derived from CContextMenuItem to avoid all the if (IsGroup())? But obviously that would mean that you'd have to work with (shared) pointers instead of objects.

Apart from the minors I've commented on I couldn't spot anything wrong. And thanks for adding backwards compatibility.

@tamland
Copy link
Member Author

tamland commented Jul 21, 2015

@Montellese Yeah, that's what I originally started with, but it got very messy. IsGroup is currently only used by Unregister, IsVisible and OnClick, all of which needs to handle groups specially, so you wont get rid of those checks anyway.

@tamland tamland force-pushed the context_menu_groups branch 2 times, most recently from d647b12 to 5d64d4b Compare July 26, 2015 11:37
@tamland
Copy link
Member Author

tamland commented Jul 26, 2015

I've updated the sorting part to simply only sort the two internal menus. That means items in submenus from addons will be kept in the original order, while item in the internal ones will not. (A possible extension is to allow submenus to specify sorting or no sorting). I don't think it makes sense, from a users perspective, to sort the root menu in any other way.

@tamland
Copy link
Member Author

tamland commented Jul 26, 2015

@mkortstiege Could you do a xcode sync?

@Memphiz
Copy link
Member

Memphiz commented Jul 26, 2015

@tamland Memphiz@833effd

@tamland
Copy link
Member Author

tamland commented Jul 27, 2015

Thanks!

@tamland
Copy link
Member Author

tamland commented Jul 27, 2015

jenkins build this please

@tamland tamland added this to the Jarvis 16.0-alpha2 milestone Aug 6, 2015
@tamland
Copy link
Member Author

tamland commented Aug 6, 2015

jenkins build this please

tamland added a commit that referenced this pull request Aug 8, 2015
[addons] extend context menu system
@tamland tamland merged commit e32987a into xbmc:master Aug 8, 2015
@tamland tamland deleted the context_menu_groups branch August 9, 2015 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants