Skip to content

feat: Previous and next chapter buttons - #1063

Open
alex-rans wants to merge 2 commits into
DonutWare:developfrom
alex-rans:feature_chapter_buttons
Open

feat: Previous and next chapter buttons#1063
alex-rans wants to merge 2 commits into
DonutWare:developfrom
alex-rans:feature_chapter_buttons

Conversation

@alex-rans

Copy link
Copy Markdown

Pull Request Description

Added previous and next chapter buttons in the video player. This has not been added to tv_player_controls.dart, but I will (probably) add this in another PR.

Issue Being Fixed

None, besides my mild annoyance

Screenshots / Recordings

image

Tested On

  • Android
  • Android TV
  • iOS
  • Linux
  • Windows
  • macOS
  • Web

Checklist

  • If a new package was added, did you ensure it works for all supported platforms? Is the package well maintained
  • Check that any changes are related to the issue at hand.

@PartyDonut PartyDonut left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Functionality looks good, have some small requests.

final chapters = ref.read(playBackModel.select((value) => value?.chapters)) ?? [];
if (chapters.isEmpty) return;
final position = ref.read(mediaPlaybackProvider).position;
final threshold = const Duration(milliseconds: 3000);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is a extension in chapters_model.dart lets move the next/previous lookups in there and only pass the required paramaters to fetch previous/next chapters.

That way we can re-use it when needed.

Comment on lines +741 to +747
if (next != null) {
ref.read(videoPlayerProvider).seek(next.startPosition);
} else {
// fallback: load next video
final nextVideo = ref.read(playBackModel.select((value) => value?.nextVideo));
if (nextVideo != null) ref.read(playbackModelHelper).loadNewVideo(nextVideo);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about loading the next video. Would it not be better to disable the next chapter button if there is no next chapter?

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.

2 participants