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

[PVR] context menu rewrite & major gui actions refactoring #10870

Merged
merged 21 commits into from Dec 2, 2016

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Nov 5, 2016

This all started with the wish to support context menus for the new Estuary PVR home screen widgets and quickly turned big, because i wanted to avoid duplicate code. At the end i was in a rage to eliminate as many duplicate pvr gui actions code as possible.

This is not completely finished, but all runtime tested and working well.

@xhaggi @Jalle19 for review and general comments?
@tamland any comments regarding the new CContextMenuManager related code pieces?

@ksooo ksooo added Type: Feature non-breaking change which adds functionality Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: PVR WIP PR that is still being worked on v18 Leia labels Nov 5, 2016
@tamland
Copy link
Member

tamland commented Nov 6, 2016

wow, all at once. great work!

ReloadAddonItems();

std::vector<std::shared_ptr<IContextMenuItem>> pvrItems = m_pvrMgr.GetContextMenuItems();

This comment was marked as spam.

This comment was marked as spam.


std::string MarkWatched::GetLabel(const CFileItem &item) const
{
return g_localizeStrings.Get(16103); /* Mark as watched */

This comment was marked as spam.

This comment was marked as spam.

@ksooo
Copy link
Member Author

ksooo commented Nov 6, 2016

@tamland thanks for the review. The changes you suggested are now in.

const CPVRRecordingPtr recording(item.GetPVRRecordingInfoTag());
if (recording)
{
if (!recording->IsDeleted())

This comment was marked as spam.

This comment was marked as spam.

if (!recording)
recording = item.GetPVRRecordingInfoTag();

if (recording)

This comment was marked as spam.

This comment was marked as spam.

timer = epg->Timer();

if (!timer)
timer = item.GetPVRTimerInfoTag();

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

CPVRContextMenuManager();
CPVRContextMenuManager(const CPVRContextMenuManager&) = delete;
CPVRContextMenuManager const& operator=(CPVRContextMenuManager const&) = delete;
virtual ~CPVRContextMenuManager() {}

This comment was marked as spam.

This comment was marked as spam.

@@ -150,7 +120,6 @@ bool CGUIWindowPVRChannels::OnAction(const CAction &action)
{
switch (action.GetID())
{
case REMOTE_0:

This comment was marked as spam.

This comment was marked as spam.

@da-anda
Copy link
Member

da-anda commented Nov 7, 2016

I know I'm annoying, but I noticed that you kindly added some file reference comments to the strings.po file and now should also know their context. So I wanted to ask if you're also in the mood to add a small contextual help comment to those strings? At least an indication if a certain string is a settings option or a settings label. Thanks.

@ksooo
Copy link
Member Author

ksooo commented Nov 7, 2016

@da-anda will do, remember, this is still wip.

@ksooo
Copy link
Member Author

ksooo commented Nov 19, 2016

@Jalle19 thanks for the review. The changes you suggested are now in.

@ksooo ksooo force-pushed the pvr-contextmenu-rewrite branch 4 times, most recently from 40220c4 to b64b7b8 Compare November 24, 2016 10:23
@ksooo ksooo force-pushed the pvr-contextmenu-rewrite branch 2 times, most recently from bb3c868 to 8a8cac6 Compare November 30, 2016 15:31
@ksooo
Copy link
Member Author

ksooo commented Nov 30, 2016

@ronie mind taking a look at the strings.po cleanup commit? I inserted several "#. @@@ unused?" comments. maybe you can tell for some of those whether they are actually unused?

@ronie
Copy link
Member

ronie commented Nov 30, 2016

these are used by several skins:
13113
13204
19005
19080
19081
19144
19145
19203
19207
19216
19284

@ksooo
Copy link
Member Author

ksooo commented Nov 30, 2016

these are used by several skins:

thanks @ronie. will change remarks accordingly.

@ksooo ksooo force-pushed the pvr-contextmenu-rewrite branch 2 times, most recently from 003c241 to 8b108f4 Compare December 1, 2016 11:06
@ksooo ksooo removed the WIP PR that is still being worked on label Dec 1, 2016
@ksooo
Copy link
Member Author

ksooo commented Dec 1, 2016

jenkins build this please

…eader file instead to typedef again and again in several header files.
@ksooo
Copy link
Member Author

ksooo commented Dec 2, 2016

jenkins build this please

@ksooo ksooo merged commit d3f067c into xbmc:master Dec 2, 2016
@ksooo ksooo deleted the pvr-contextmenu-rewrite branch December 2, 2016 07:44
#include "pvr/timers/PVRTimers.h"
#include "utils/URIUtils.h"

#include "PVRContextMenus.h"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Feature non-breaking change which adds functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants