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] refactor advanced setting showepginfoonselect to setting #3909

Merged
merged 2 commits into from Dec 30, 2013

Conversation

xhaggi
Copy link
Member

@xhaggi xhaggi commented Dec 28, 2013

This removes the advanced setting showepginfoonselect and adds it to settings, so users can change it within the gui. In addition, the setting is extended and you can now select the following values:

  • Show context menu
  • Switch to channel
  • Show information
  • Record

@da-anda
Copy link
Member

da-anda commented Dec 28, 2013

Hmm, I somehow don't like the "switch to channel (active only)" option. Spinner should probably only have two options and in case "switch to channel" is selected a boolean subsetting "only switch for ongoing slots/shows" could appear. At first I thought that one spinner to rule them all should work, but it feels/reads odd now. Opinions?

@xhaggi
Copy link
Member Author

xhaggi commented Dec 28, 2013

i don't think we should blow up the settings for this. If you look at the video settings the spinner holds up to 5 different options. We could change the label for the option "active only" to "switch to channel only for ongoing shows" and remove the braces.

@xhaggi
Copy link
Member Author

xhaggi commented Dec 28, 2013

as discussed with @da-anda we changed the epg select options a bit. we removed the "switch to channel (active only)" option and added two new options "show context menu (default)" and "record".

We also re-order the context menu so the "switch to channel" option is on top. If your remote only has a select button you could click "select" twice to switch the channel, if you choose the option "show context menu" and you have full access to the context menu, which is currently impossible if your remote does not support a context menu button.

This removes the advanced setting showepginfoonselect and adds it to
settings, so users can change it within the gui. In addition, the
setting is extended and you can now select the following values:

* Show context menu
* Switch to channel
* Show information
* Record
@xhaggi
Copy link
Member Author

xhaggi commented Dec 28, 2013

@opdenkamp fine for you?

@da-anda
Copy link
Member

da-anda commented Dec 28, 2013

As already mentioned on IRC/PM, default should probably be "show info" to have the same default behavior as in Frodo. And the idea with the subsetting still exists (now it would have to be "- always switch to channel for ongoing items")

@xhaggi
Copy link
Member Author

xhaggi commented Dec 29, 2013

@da-anda changed the default to "Show information" in fixup commit.

@da-anda
Copy link
Member

da-anda commented Dec 29, 2013

@jmarshallnz up to you to merge if @opdenkamp is fine with it

@jmarshallnz
Copy link
Contributor

Does the fixup commit need squashing? Other than that, if it builds, it can go in.

@xhaggi
Copy link
Member Author

xhaggi commented Dec 30, 2013

Yeah the fixup needs to be squashed before it go in. Will do this asap.

@xhaggi
Copy link
Member Author

xhaggi commented Dec 30, 2013

jenkins build this please

xhaggi added a commit that referenced this pull request Dec 30, 2013
[pvr] refactor advanced setting showepginfoonselect to setting
@xhaggi xhaggi merged commit ff80c1f into xbmc:master Dec 30, 2013
@xhaggi xhaggi deleted the epg-info-onselect branch January 4, 2014 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants