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

[confluence] Chapter foward always hidden by broken condition #6115

Merged
merged 1 commit into from
Jan 6, 2015

Conversation

un1versal
Copy link
Contributor

I dont understand this condition nor why the chapter forward button is always hidden even when a file has chapters.

Further makes no sense whatsoever because chapter back is possible/visible but not chapter forward.

If we hiding buttons conditionally then hide correct ones properly.

I dont understand this condition nor why the button is always hidden when a file has chapters.

Further makes no sense whatsoever because chapter back is possible/visible but not chapter forward which actually makes sense if we hiding buttons conditionally then hide correct ones.
@phil65
Copy link
Contributor

phil65 commented Jan 5, 2015

if the "next" button also works for chapters it is a mistake on my side, I thaught that vis condition would only trigger for additional playlist items.
I would like to have the opinion of someone who is reasonable first though. @ronie please for clarification.

@un1versal
Copy link
Contributor Author

Yes the next button works for chapters always has its what I have always used it for.

This should fix that and I have actually tested this PR before forwarding it here.

@phil65
Copy link
Contributor

phil65 commented Jan 5, 2015

yeah uNiversaI, we all know how clever you are and how much better your PRs are than mine. good to know.

@un1versal
Copy link
Contributor Author

That is a mean comment and uncalled for. I am merely pointing out and fixing the issue best I can!

Clearly you take offence where none was intended in first place.

@ronie
Copy link
Member

ronie commented Jan 5, 2015

i've never used chapters before, so i just had to test it.
the next button indeed allows you to skip to the next chapter.

@phil65
Copy link
Contributor

phil65 commented Jan 5, 2015

Thx ronie. Do you want to use this PR or should i put up a new one which hides that button when both no next track in playlist available + no next chapter available?

@un1versal
Copy link
Contributor Author

If you going to hide buttons via condition then please hide previous/next chapters not just next

Absolutely no point in leaving previous chapter visible if none exists in that case. and only if no chapters exist in file/source does that scenario work.

@phil65
Copy link
Contributor

phil65 commented Jan 5, 2015

not totally comparable. previous track can be used all the time to "restart" a playlist item, "next track" can have the same functionality as stop button depending on actual situation. In that case it makes sense to hide / disable it in my opinion because it gives a visual indication then that nothing comes after the actual playing track / chapter.

@ronie
Copy link
Member

ronie commented Jan 5, 2015

Do you want to use this PR or should i put up a new one which hides that button when both no next track in playlist available + no next chapter available?

let's say the user is using the next button to skip to the last chapter.
when he reaches the last chapter, then all of a sudden the next button will disappear?
that would look odd imo. besides, the player controls will loose focus when that happens.

@phil65
Copy link
Contributor

phil65 commented Jan 5, 2015

it does not have to disappear, it could also get half-transparent and de-activated. losing focus could be avoided with an additional conditional < onclick >. If that is not wanted behaviour though then just pull this PR, it reverts to old behaviour. I´m just not a fan of redundant buttons and I dont see a point in a next button when starting a single movie from library (which is probably the most used case) if that movie does not have any chapters. :)

@un1versal
Copy link
Contributor Author

let's say the user is using the next button to skip to the last chapter.
when he reaches the last chapter, then all of a sudden the next button will disappear?
that would look odd imo. besides, the player controls will loose focus when that happens.

@ronie

tbh I dont mind if a better solution is proposed and merged instead of this!

Maybe its best if @phil65 solution are proposed in another PR and tested to see how it works then?

@ronie
Copy link
Member

ronie commented Jan 6, 2015

nah, i think i prefer to keep things simple & stupid for our default skin.

ronie added a commit that referenced this pull request Jan 6, 2015
[confluence] Chapter foward always hidden by broken condition
@ronie ronie merged commit 4b80c7b into xbmc:master Jan 6, 2015
@un1versal un1versal deleted the patch-1 branch January 7, 2015 02:14
@MartijnKaijser MartijnKaijser added this to the Helix 15.0-alpha1 milestone Jan 10, 2015
phil65 pushed a commit to phil65/xbmc that referenced this pull request Jan 10, 2015
[confluence] Chapter foward always hidden by broken condition
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