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

[Estuary][PVR] Season/Ep info in PVRGuide #13300

Merged
merged 2 commits into from
Jan 6, 2018
Merged

Conversation

fuzzard
Copy link
Contributor

@fuzzard fuzzard commented Jan 4, 2018

Add Season/Ep info to PVR EPG Guide screen

Description

Show season and episode on main PVR Guide screen when available. Move Episode Title
to same line with ep/season when available, similar to DialogPVRInfo.xml. Havent reused the existing variable SeasonEpisodeLabel as i thought it may need to be altered for colour/formatting as it doesnt look nice with the grey on the Guide screen.

Would appreciate any feedback on how to make it look prettier. Im not familiar with the skinning side of kodi at all, so not entirely sure how to manage colours.

Motivation and Context

Provides further information at a glance regarding series/episode without having to open the EPG entry.

How Has This Been Tested?

Have runtime tested on Mac OS X running latest tip of Kodi

Screenshots (if appropriate):

Series + Episode + Episode Title
screen shot 2018-01-04 at 1 53 59 pm

Episode Number + Title
screen shot 2018-01-04 at 1 54 10 pm

Episode Name Only
screen shot 2018-01-04 at 1 54 06 pm

No tag info
screen shot 2018-01-04 at 1 54 08 pm

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@ksooo
Copy link
Member

ksooo commented Jan 4, 2018

Nice. What I do not like that much is that the episode title has a bigger font then the item in the title line. It is visually much more present than the show title, which feels wrong. And I would make the episode/season number colored (COLOR button_focus or COLOR grey). I think we handle it this way in other places.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 4, 2018

Have updated. Thats exactly the color code i was after thanks ksooo. Removed the bold, as it was a carry over from the other seasonepisode variable.

screen shot 2018-01-04 at 5 01 00 pm
screen shot 2018-01-04 at 5 01 13 pm

<right>60</right>
<height>30</height>
<textcolor>button_focus</textcolor>
<label>$VAR[GuideSeasonEpisodeLabel]</label>

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@ksooo
Copy link
Member

ksooo commented Jan 4, 2018

Blue numbers look ugly. make it grey (like it is in var SeasonEpisodeLabel ;-) I'd say.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 4, 2018

Changed colour to grey in the guide window as requested.

Ill be honest and say i couldnt work out the whole include thing. It still required the variable to handle the different states of ep/season/title, and i couldnt work out how to do the colours/bolds using parameters and the variable with what i could find as include examples.
Im hoping this solution is adequate. Both the guide window and info window use the variable to populate se/ep numbers, and then wrap that in colours/bold with ep name at the end that can be empty in the original control label.

screen shot 2018-01-04 at 9 46 39 pm
screen shot 2018-01-04 at 9 46 41 pm
screen shot 2018-01-04 at 9 47 16 pm

No visual differences on the DialogPVRinfo

<left>300</left>
<right>60</right>
<height>30</height>
<textcolor>button_focus</textcolor>

This comment was marked as spam.

<value>$INFO[ListItem.Season,[COLOR grey]S,[/COLOR]]$INFO[ListItem.Episode,[COLOR grey]E,[/COLOR]]: [B]$INFO[ListItem.EpisodeName][/B]</value>
<value condition="String.IsEmpty(ListItem.Season) + String.IsEmpty(ListItem.Episode)"></value>
<value condition="String.IsEmpty(ListItem.EpisodeName)">$INFO[ListItem.Season,S]$INFO[ListItem.Episode,E]</value>
<value>$INFO[ListItem.Season,S]$INFO[ListItem.Episode,E]: </value>

This comment was marked as spam.

This comment was marked as spam.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 4, 2018

Changes as requested.
Ive added a second commit (for uniformity sake more than anything) that adds season/ep numbers to the channel selection.
Any preferred layout for this, or just continue the italics of the ep title as is shown in the attached images?

screen shot 2018-01-04 at 10 08 38 pm
screen shot 2018-01-04 at 10 09 05 pm

<value>$INFO[ListItem.Season,[COLOR grey]S,[/COLOR]]$INFO[ListItem.Episode,[COLOR grey]E,[/COLOR]]: [B]$INFO[ListItem.EpisodeName][/B]</value>
<value condition="String.IsEmpty(ListItem.Season) + String.IsEmpty(ListItem.Episode)"></value>
<value condition="String.IsEmpty(ListItem.EpisodeName)">$INFO[ListItem.Season,S]$INFO[ListItem.Episode,E]</value>
<value>$INFO[ListItem.Season,S]$INFO[ListItem.Episode,E,: ]</value>

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Show season and episode on main Guide screen when available. Move Episode Title
to same line with ep/season when available, similar to DialogPVRInfo.xml
@ksooo
Copy link
Member

ksooo commented Jan 4, 2018

Looking good now. Many thanks.

@ronie any concerns regarding what we hacked into xml?

@da-anda
Copy link
Member

da-anda commented Jan 4, 2018

I personally don't think the S01E04 spelling is userfriendly. I know this spelling is used accross the web, but when I think about f.e. my parents, they wouldn't understand what this means, especially because it's based on English abbreviations. Wouldn't it be nicer to split it up into Season: X and Episode: Y and have these labels translated?

@ksooo
Copy link
Member

ksooo commented Jan 4, 2018

I personally don't think the S01E04 spelling is userfriendly.

For now, we leave it as it is. Changing it everywhere is far beyond the scope of this PR.

@ksooo
Copy link
Member

ksooo commented Jan 5, 2018

@ronie good to go?

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 5, 2018

Would you prefer the following @da-anda

screen shot 2018-01-05 at 3 53 04 pm
screen shot 2018-01-05 at 3 53 09 pm
screen shot 2018-01-05 at 3 53 15 pm

I have this implemented using strings from resource.language.en_gb. I assume this is the only resource needed to be done, and then something like transifex will populate the other languages?

Happy to push that ksooo if you want. Not totally sold on this style visually personally, but i do understand the parent usage scenario da-anda brought up.

@ksooo
Copy link
Member

ksooo commented Jan 5, 2018

Would you prefer the following @da-anda

As I said, -1 on this for this PR.

It's using way to much space. There are more changes needed to make this "good", especially consistent with other Kodi components, where the SxEx notation is also used.

@fuzzard
Copy link
Contributor Author

fuzzard commented Jan 5, 2018

Wasnt pushed, was only created locally so its not in this PR as it stands right now

@Jalle19
Copy link
Member

Jalle19 commented Jan 5, 2018

I don't have a strong opinion on it, but SXXEYY is very much an unofficial internet standard. The word for "season" begins with an "S" in quite a few languages too so I don't think "average" folk would have too big of a problem figuring out what it means.

@ksooo
Copy link
Member

ksooo commented Jan 5, 2018

what @Jalle19 said

@ksooo
Copy link
Member

ksooo commented Jan 5, 2018

@ronie I think now we have it. ;-) Good to go?

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go, function-wise.

@ksooo ksooo merged commit ce126f2 into xbmc:master Jan 6, 2018
@Rechi Rechi added Type: Improvement non-breaking change which improves existing functionality Component: PVR Component: Skin v18 Leia labels Jan 8, 2018
@Rechi Rechi added this to the L 18.0-alpha1 milestone Jan 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Component: Skin Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants