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

Add a watched option to the continue watching pop up #330

Merged

Conversation

jackrvan
Copy link
Contributor

@jackrvan jackrvan commented Dec 8, 2020

This adds a button to mark an episode as watched to the popup box when you click on an episode in the "Continue Watching" section.

Fixes #329

@neilsb
Copy link
Member

neilsb commented Dec 8, 2020

Welcome, and congratulations on your first contribution. Looks like you've done everything right, and the PR has been created correctly.

We're just in the final stages of getting a few bugs ironed to get a release into the Roku store before the "Holiday Blackout", so it may be a few days before someone gets round to testing and reviewing the PR. We really appreciate folk contributing so thanks :)

@neilsb
Copy link
Member

neilsb commented Dec 10, 2020

Marking an episode seen from the continue watching section on the home screen should probably update the Home Screen. This would remove the watched item from continue watching and (if it was a TV Series) update the "Next Up" row if there were other episodes to watch.

Similarly, if "Mark Watched" was selected from the Movie Details screen, then ideally screen would update to show the item had been watched (i.e the "Watched" button would turn red - which is how this is shown currently).

@jackrvan
Copy link
Contributor Author

@neilsb i added another commit to refresh the home page when we mark an episode as watch. from my testing this appears to be working. I have spent the past few hours trying to figure out how to refresh the movie details page when we do the same thing for a movie but I have not been successful. I understand you are only a volunteer so if you dont have time I completely understand but if someone could give me a hand and maybe point me in the right direction I would appreciate it greatly.

I added a refresh() function to MovieDetails.brs which just calls itemContentChanged but for some reason when i am setting:
video.content.watched = not video.content.watched
in VideoPlayer.brs I can see that this works and it becomes True however when I then get into MovieDetails.brs and we are getting the content with:
item = m.top.itemContent
its still set to false. I have tried figuring out why this is not getting updated from MovieDetails.brs but I have not been able to figure it out.

In addition I appreciate you taking the time to look at my changes and I understand if it takes a while to look again. Am I supposed to do something special after making my new change (aka ready for another review) or is just making this comment good enough?

@neilsb
Copy link
Member

neilsb commented Dec 27, 2020

@jackrvan Sorry for the lack of response. Things have been busy with the day job. New changes you push the PR (as you did with this one) all get tracked, so that's all that is required.

I see the issue with the MovieDetails screen and marking an item watched from the dialog. The VideoPlayer player does not have access to m.top.itemContent, and since the Video it being marked as played it's just not updated in the itemContent.

Since the original intention of this is for the Continue Watching section on the home screen, maybe the better thing to do would be to only show the "Watched" option when displaying the dialog on the Home Screen. Since from the Movie Details screen you have the "Watched" button there anyhow that can be used to mark as watched.

That said, I'm not sure how easy it would be to (currently) detect where the video playback was started from (i.e. the Home Screen or the Movie Details screen) to enable showing/hiding option....

Delete unused function from MovieDetails
@jackrvan
Copy link
Contributor Author

jackrvan commented Jan 6, 2021

I definitely agree that only having the watched option available on the home screen makes the most sense. I couldnt figure out a nice way to do this. I was hoping I was trying to find an isinstance (from python) function or something build into brightscript that i could call on the screen to see if its a Home class or MovieDetails or whatever else but couldnt find it. I did find type() but that just seems to return roSGNode which isnt helpful.

So what I ended up doing is just reading the overhang title of the current screen and if its Home then we add the option. If its not then we dont. From looking around this was the only way i could figure out how to do it becauseas you mentioned I couldnt find a way to detect where the playback was started from.

Copy link
Member

@neilsb neilsb left a comment

Choose a reason for hiding this comment

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

Looks good.

@neilsb neilsb merged commit 593a017 into jellyfin:master Jan 16, 2021
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.

Add a watched button to the continue watching pop up
2 participants