feat: add holding gesture and improve swipe gesture flexibility#757
feat: add holding gesture and improve swipe gesture flexibility#757
Conversation
…r element viewmodel
This reverts commit 8720909.
rename player gesture enum add new action for playback speed control
There was a problem hiding this comment.
Pull request overview
Refactors the media player gesture system to be configurable by mapping gesture directions to playback actions, and adds tap-and-hold speed boost plus improved pointer wheel handling.
Changes:
- Introduces
PlaybackActionKindand replaces gesture booleans with per-gesture action mappings (tap + swipe directions). - Updates Settings UI/resources to allow selecting actions per gesture, and adds a tap-and-hold toggle.
- Adds an enum-to-resource-string converter and routes pointer wheel/hold/swipe events through the view + view model.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| Screenbox/Strings/en-US/Resources.resw | Adds new localized strings for gesture settings and PlaybackActionKind display names. |
| Screenbox/Screenbox.csproj | Includes the new enum-to-resource converter in the app project. |
| Screenbox/Pages/SettingsPage.xaml | Replaces gesture toggles with action ComboBox selectors and adds tap-and-hold toggle. |
| Screenbox/Converters/EnumToResourceStringConverter.cs | Adds helper/converter to localize enum values for UI templates. |
| Screenbox/Controls/PlayerElement.xaml.cs | Adds GestureRecognizer hold handling and moves pointer wheel handling to the view. |
| Screenbox/Controls/PlayerElement.xaml | Wires pointer/gesture events to new handlers and adjusts size-changed handling. |
| Screenbox.Core/ViewModels/SettingsPageViewModel.cs | Adds gesture action properties, loads/saves them via settings service, exposes options list. |
| Screenbox.Core/ViewModels/PlayerPageViewModel.cs | Updates click-to-hide-controls behavior based on PlaybackActionKind.None. |
| Screenbox.Core/ViewModels/PlayerElementViewModel.cs | Implements unified gesture processing + hold speed boost + wheel horizontal seek support. |
| Screenbox.Core/Services/SettingsService.cs | Replaces old gesture bool settings with action-based settings + new defaults. |
| Screenbox.Core/Services/ISettingsService.cs | Updates settings contract to expose gesture action mappings and hold toggle. |
| Screenbox.Core/Screenbox.Core.csproj | Adds PlaybackActionKind and removes ManipulationLock. |
| Screenbox.Core/Enums/PlaybackActionKind.cs | Introduces enum defining supported playback actions. |
| Screenbox.Core/Enums/ManipulationLock.cs | Removes the old manipulation lock enum. |
| private const string PlayerShowChaptersKey = "Player/ShowChapters"; | ||
| private const string PrivacyPersistPlaybackPosition = "Privacy/PersistPlaybackPosition"; | ||
|
|
||
| private const string PlayerGestureTapKey = "Player/Gesture/Tap"; |
There was a problem hiding this comment.
The settings key for tap gesture remains "Player/Gesture/Tap" but the stored type changed from bool (previous implementation) to int (enum). On upgrade, GetValue<int> may fail or return an unintended value if an existing installation has a boolean stored under that same key. Consider adding an explicit migration path (e.g., attempt to read the old boolean and map true => PlaybackActionKind.PlayPause, false => PlaybackActionKind.None), or use a new key name/versioned key for the enum and migrate once.
| private const string PlayerGestureTapKey = "Player/Gesture/Tap"; | |
| private const string PlayerGestureTapKey = "Player/Gesture/TapAction"; |
|
|
||
| public PlaybackActionKind PlayerTapGesture | ||
| { | ||
| get => (PlaybackActionKind)GetValue<int>(PlayerGestureTapKey); |
There was a problem hiding this comment.
The settings key for tap gesture remains "Player/Gesture/Tap" but the stored type changed from bool (previous implementation) to int (enum). On upgrade, GetValue<int> may fail or return an unintended value if an existing installation has a boolean stored under that same key. Consider adding an explicit migration path (e.g., attempt to read the old boolean and map true => PlaybackActionKind.PlayPause, false => PlaybackActionKind.None), or use a new key name/versioned key for the enum and migrate once.
| get => (PlaybackActionKind)GetValue<int>(PlayerGestureTapKey); | |
| get | |
| { | |
| // Handle migration from legacy bool setting ("Player/Gesture/Tap") to int (PlaybackActionKind) | |
| if (SettingsStorage.TryGetValue(PlayerGestureTapKey, out var rawValue)) | |
| { | |
| if (rawValue is int intValue) | |
| { | |
| return (PlaybackActionKind)intValue; | |
| } | |
| if (rawValue is bool boolValue) | |
| { | |
| // Legacy mapping: true => PlayPause, false => None | |
| var migrated = boolValue ? PlaybackActionKind.PlayPause : PlaybackActionKind.None; | |
| SetValue(PlayerGestureTapKey, (int)migrated); | |
| return migrated; | |
| } | |
| } | |
| // Fallback if value is missing or of an unexpected type | |
| var defaultValue = PlaybackActionKind.None; | |
| SetValue(PlayerGestureTapKey, (int)defaultValue); | |
| return defaultValue; | |
| } |
|
|
||
| public event RoutedEventHandler? Click; | ||
|
|
||
| private GestureRecognizer _gestureRecognizer; |
There was a problem hiding this comment.
Windows.UI.Input.GestureRecognizer should be cleaned up to avoid event-handler leaks and unnecessary processing after the control is unloaded. Consider making the field readonly, unhooking Holding on Unloaded, and disposing the recognizer (it implements IDisposable) when the control is no longer in use.
| _gestureRecognizer = new GestureRecognizer | ||
| { | ||
| GestureSettings = GestureSettings.Hold | GestureSettings.HoldWithMouse, | ||
| }; | ||
|
|
||
| _gestureRecognizer.Holding += GestureRecognizer_OnHolding; |
There was a problem hiding this comment.
Windows.UI.Input.GestureRecognizer should be cleaned up to avoid event-handler leaks and unnecessary processing after the control is unloaded. Consider making the field readonly, unhooking Holding on Unloaded, and disposing the recognizer (it implements IDisposable) when the control is no longer in use.
improve playback rate formatting and gesture cleanup minor converter xml doc update
update playback rate handling and enum converter
fix ci building error
update keyboard accelerator helper
545eb88 to
a75de41
Compare
Refactors the gesture handling system for the media player, making it more flexible and configurable. Instead of using simple boolean flags for gestures like volume and seek, it now allows mapping various gestures (tap and swipe directions) to specific media commands via a new
PlaybackActionKindenum. This improves extensibility and allows for more customized user interactions.Adds press and hold gesture support (boosts playback speed to 2x)
It also refactors the
VideoView.PointerWheelChangedandVlcVideoView.SizeChangedmethods to the view, and adds support for horizontal touchpad/mouse wheel input.