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

Make the series and season Play button play the next episode #4827

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

beef331
Copy link
Contributor

@beef331 beef331 commented Oct 3, 2023

Changes

Instead of starting the series or season at the first episode this queues up the next unwatched episode. If the selection is fully watched it marks all unplayed and starts over.

Issues

Partially Fixes #2570 but it does not continue a partially watched episode.

@beef331 beef331 requested a review from a team as a code owner October 3, 2023 05:04
src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
@thornbill thornbill added the enhancement Improve existing functionality or small fixes label Oct 4, 2023
Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

I like the new version for its sleek appearance. But we will miss episodes when watch status is:

S01E01  v
S01E02  v
S01E03     <-- we unmarked it as 'Played', playback starts from here
S02E01  v  <-- this will be skipped
S02E02
S02E03

If this behavior is acceptable, below are some necessary changes.

If no, your first attempt, but without the outer Promise.

UPD:
Logically, your first attempt has better behavior - you can rewatch the episodes in the correct order without skipping.
Please revert the changes to the first attempt, but without the outer Promise. Actually, that's what I expected.

src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
src/components/playback/playbackmanager.js Outdated Show resolved Hide resolved
@beef331
Copy link
Contributor Author

beef331 commented Oct 5, 2023

Please revert the changes to the first attempt, but without the outer Promise. Actually, that's what I expected.

Given that the resolve parameter of the Promise function was used I'm very much uncertain how you expect to drop the outer promise. I clearly do not have a great understanding of this code base nor JS 😛

@beef331 beef331 force-pushed the master branch 2 times, most recently from 629537c to d5fdb82 Compare October 6, 2023 09:17
@dmitrylyzo
Copy link
Contributor

Similar to my suggestion for seasons, series currently fails if it is complete.
We probably should restart it.

@beef331 beef331 force-pushed the master branch 2 times, most recently from 031445a to 10bb08a Compare October 8, 2023 08:38
@beef331 beef331 changed the title Made the series play button play the next episode Made the series and season play button play the next episode and mark unwatched Oct 8, 2023
@dmitrylyzo dmitrylyzo changed the title Made the series and season play button play the next episode and mark unwatched Make the series and season Play button play the next episode Oct 11, 2023
@sonarcloud
Copy link

sonarcloud bot commented Oct 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 6da458f
Status ✅ Deployed!
Preview URL https://jellyfin-web.pages.dev (https://e4e3a7f4.jellyfin-web.pages.dev)
Type ⚙️ Production

View build logs
View bot logs

Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

LGTM
I've fixed style to my taste 😄

I guess we will need to change UserData.Played to something else when that discussion is resolved.

but it does not continue a partially watched episode.

If you want to fix resuming, you need to pass startPositionTicks in options somehow:

const startPosition = playOptions.startPositionTicks;

But this can be done in another PR.

Also, for the future, it's better to create a separate branch for PR.

@thornbill thornbill merged commit 00a52a8 into jellyfin:master Oct 12, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Play button on thumbnail starts at show's pilot
4 participants