-
-
Notifications
You must be signed in to change notification settings - Fork 119
feat: add subtitle toggle hotkey #961
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import 'package:screen_brightness/screen_brightness.dart'; | |
|
|
||
| import 'package:fladder/models/item_base_model.dart'; | ||
| import 'package:fladder/models/items/media_segments_model.dart'; | ||
| import 'package:fladder/models/items/media_streams_model.dart'; | ||
| import 'package:fladder/models/media_playback_model.dart'; | ||
| import 'package:fladder/models/playback/playback_model.dart'; | ||
| import 'package:fladder/models/settings/video_player_settings.dart'; | ||
|
|
@@ -78,10 +79,13 @@ class _DesktopControlsState extends ConsumerState<DesktopControls> { | |
| double? _vDragStartValue; | ||
| double? _vDragLastValue; | ||
|
|
||
| int? _lastSelectedSubtitleIndex; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| timer.reset(); | ||
| _lastSelectedSubtitleIndex = null; | ||
| } | ||
|
|
||
| @override | ||
|
|
@@ -909,6 +913,58 @@ class _DesktopControlsState extends ConsumerState<DesktopControls> { | |
| _vDragLastValue = null; | ||
| } | ||
|
|
||
| void _toggleSubtitles() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure about the logic here. Does it make sense for the user to press the "toggle" button to all of a sudden ask what subtitles they want to toggle in-between? Would it make more sense to just do nothing if the subtitles are already disabled and only allow toggling when a subtitle is enabled? No strong opinion on my end just thinking out loud.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not against the idea, but I think this might lead users to open issues on GitHub saying that the toggle key is not working, when in fact it would be working. Then from experience I know that most people don't check previous issues before opening one which would lead to a bunch of duplicate issues regarding this. |
||
| final playbackModel = ref.read(playBackModel); | ||
| final player = ref.read(videoPlayerProvider); | ||
| final subStreams = playbackModel?.subStreams; | ||
|
|
||
| if (subStreams == null || subStreams.isEmpty) return; | ||
|
|
||
| // Filter out the "off" track (index == -1) | ||
| final availableSubtitles = subStreams.where((s) => s.index != -1).toList(); | ||
| if (availableSubtitles.isEmpty) return; | ||
|
|
||
| final currentIndex = playbackModel?.mediaStreams?.defaultSubStreamIndex ?? -1; | ||
|
|
||
| // If only one subtitle track, toggle between it and off | ||
| if (availableSubtitles.length == 1) { | ||
| final singleSub = availableSubtitles.first; | ||
| final newSub = currentIndex == singleSub.index ? SubStreamModel.no() : singleSub; | ||
| _setSubtitleTrack(newSub, playbackModel, player); | ||
| return; | ||
| } | ||
|
|
||
| // Multiple subtitle tracks | ||
| if (_lastSelectedSubtitleIndex == null) { | ||
| // First time - show selection dialog | ||
| showSubSelection(context).then((_) { | ||
| // After dialog closes, store the selected subtitle | ||
| final newModel = ref.read(playBackModel); | ||
| final selectedIndex = newModel?.mediaStreams?.defaultSubStreamIndex; | ||
| if (selectedIndex != null && selectedIndex != -1) { | ||
| _lastSelectedSubtitleIndex = selectedIndex; | ||
| } | ||
| }); | ||
| } else { | ||
| // Toggle between off and last selected | ||
| final lastSub = subStreams.firstWhere( | ||
| (s) => s.index == _lastSelectedSubtitleIndex, | ||
| orElse: () => availableSubtitles.first, | ||
| ); | ||
| final newSub = currentIndex == _lastSelectedSubtitleIndex ? SubStreamModel.no() : lastSub; | ||
| _setSubtitleTrack(newSub, playbackModel, player); | ||
| } | ||
| } | ||
|
|
||
| void _setSubtitleTrack(SubStreamModel subModel, PlaybackModel? playbackModel, dynamic player) async { | ||
| if (playbackModel == null) return; | ||
| final newModel = await playbackModel.setSubtitle(subModel, player); | ||
| ref.read(playBackModel.notifier).update((state) => newModel); | ||
| if (newModel != null) { | ||
| await ref.read(playbackModelHelper).shouldReload(newModel); | ||
| } | ||
| } | ||
|
|
||
| bool _onKey(VideoHotKeys value) { | ||
| final mediaSegments = ref.read(playBackModel.select((value) => value?.mediaSegments)); | ||
| final position = ref.read(mediaPlaybackProvider).position; | ||
|
|
@@ -974,6 +1030,9 @@ class _DesktopControlsState extends ConsumerState<DesktopControls> { | |
| case VideoHotKeys.prevChapter: | ||
| ref.read(videoPlayerSettingsProvider.notifier).prevChapter(); | ||
| return true; | ||
| case VideoHotKeys.toggleSubtitles: | ||
| _toggleSubtitles(); | ||
| return true; | ||
| case VideoHotKeys.stepForward: | ||
| playing ? ref.read(videoPlayerProvider).playOrPause() : stepForward(ref); | ||
| return true; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silly question but why
T? Youtube for instance uses theCbutton for "Captions"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially going to go with
Cto mirror Youtube's logic, but since the request suggestedTI just went with that, we can do whatever I don't mind.