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] optionally show date/duration if recordings sort mode is size or duration #17691

Merged
merged 1 commit into from Apr 20, 2020

Conversation

phunkyfish
Copy link
Contributor

Description

When adding support for sorting by recording sizes the date field was removed from the right panel.

This PR allows for them when using different the sorting modes of duration/size. When size is selected both date and duration are shown as they can be seen nowhere else. If Duration is selected only data is how.

Motivation and Context

@ksooo pointed out the omission.

How Has This Been Tested?

Using iptvsimple and vuplus on MacOSX

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

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

@phunkyfish phunkyfish added Type: Improvement non-breaking change which improves existing functionality Component: PVR Component: Skin v19 Matrix labels Apr 17, 2020
@phunkyfish phunkyfish requested review from ronie and ksooo April 17, 2020 08:39
@phunkyfish phunkyfish added this to the Matrix 19.0-alpha 1 milestone Apr 17, 2020
@@ -213,7 +213,7 @@
<top>60</top>
<height>200</height>
<wrapmultiline>true</wrapmultiline>
<label>$INFO[ListItem.TimerType,[COLOR grey]$LOCALIZE[803]:[/COLOR] ,[CR]]$INFO[ListItem.Duration,[COLOR grey]$LOCALIZE[180]:[/COLOR] ,[CR]]$VAR[ExpirationDateTimeLabel,[CR]]$VAR[FlagLabel]</label>
<label>$INFO[ListItem.TimerType,[COLOR grey]$LOCALIZE[803]:[/COLOR] ,[CR]]$VAR[DateDurationLabel,[CR]]$VAR[ExpirationDateTimeLabel,[CR]]$VAR[FlagLabel]</label>x
Copy link
Member

Choose a reason for hiding this comment

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

stray x at the end of the line

don't think there's a need to include a [CR] in the $VAR?
it's already included in the individual infolabels.

likely $VAR[ExpirationDateTimeLabel,[CR]] doesn't need it either, could you please double-check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure where the x came from!

If I remove the CRs nothing displays. No date/duration/progress bar. It's all blank.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I just removed the X

Copy link
Member

Choose a reason for hiding this comment

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

If I remove the CRs nothing displays.

that shouldn't happen... please give this a shot:

<label>$INFO[ListItem.TimerType,[COLOR grey]$LOCALIZE[803]:[/COLOR] ,[CR]]$VAR[DateDurationLabel]$VAR[ExpirationDateTimeLabel]$VAR[FlagLabel]</label>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, you are right. I got mixed up in square brackets! that works.

PR updated ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this ok now @ronie ?

@phunkyfish phunkyfish force-pushed the size-duration-date branch 2 times, most recently from fe00601 to c784eac Compare April 17, 2020 17:24
@phunkyfish
Copy link
Contributor Author

@ronie is this ok now?

@ronie
Copy link
Member

ronie commented Apr 19, 2020

it's a bit hard for me to test as i do not have a proper pvr setup.
i have to get by using the pvr.demo addon, but that seriously lacks functionality.

as we now (potentially) display these 5 infolabels:

  • type
  • date
  • duration
  • expires
  • new flag

and a progress-bar in the same area, it could lead to this:

overlap

someone would need to double-check if such a thing could occur in any of the pvr windows:

  • channels
  • recordings
  • timers
  • timer rules
  • search

@phunkyfish
Copy link
Contributor Author

Good point. That could only happen for recordings with size as the sort method possibly. Everything else would be ok as we dropped date Or duration .

Let me check it tomorrow.

@phunkyfish
Copy link
Contributor Author

Ok, confirmed as only effecting recordings when size is the chosen sort mode. Note that for recordings Type is not possible and that the progress bar does not have any values associated with it so it's just the bar.

So what I did was make the duration value conditional on whether a not there is a expiration value available. Sorting by size will be used when a user wants to delete recordings and in this case duration is the least important field.

So we should be good now @ronie ;)

@ronie
Copy link
Member

ronie commented Apr 20, 2020

great, thx for the confirmation!

@ronie ronie merged commit e491a35 into xbmc:master Apr 20, 2020
@phunkyfish phunkyfish deleted the size-duration-date branch April 20, 2020 12:27
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 20, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
[estuary] optionally show date/duration if recordings sort mode is size or duration
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 v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants