Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a hold-to-drag speed feature to the video player, allowing users to dynamically adjust playback speed between 1.0x and 4.0x via long-press and horizontal drag gestures. It also adds a visual overlay with an animated chevron to indicate the active speed. Feedback focuses on improving code maintainability by extracting duplicated logic in the drag event handlers and replacing magic numbers with named constants.
| isLongPressing = true | ||
| viewModel.pause() | ||
| viewModel.sheetShown.update { Sheets.Screenshot } | ||
| } | ||
| }, | ||
| onDragEnd = { | ||
| if (isHoldingDoubleSpeed) { | ||
| isHoldingDoubleSpeed = false | ||
| MPVLib.setPropertyDouble("speed", capturedOriginalSpeed) | ||
| viewModel.playerUpdate.update { PlayerUpdates.None } | ||
| } | ||
| if (isLongPressing) isLongPressing = false | ||
| }, | ||
| onDragCancel = { | ||
| if (isHoldingDoubleSpeed) { | ||
| isHoldingDoubleSpeed = false |
There was a problem hiding this comment.
The logic in onDragEnd and onDragCancel is identical. To improve maintainability and avoid duplication, you could extract this logic into a local function or a lambda defined within the pointerInput scope.
| }, | ||
| onDrag = { change, dragAmount -> | ||
| if (isHoldingDoubleSpeed) { | ||
| change.consume() | ||
| dragTotalX += dragAmount.x | ||
| val dragSensitivity = 0.005f | ||
| val speedDelta = dragTotalX * dragSensitivity |
There was a problem hiding this comment.
This logic for hold-to-drag speed uses several magic numbers (0.005f, 2.0f, 1.0f, 4.0f, 0.1f, 10.0f). It would be beneficial for readability and maintainability to extract these into named constants at the top of the file or in a companion object.
For example:
private const val DRAG_SENSITIVITY = 0.005f
private const val INITIAL_HOLD_SPEED = 2.0f
private const val MIN_DRAG_SPEED = 1.0f
private const val MAX_DRAG_SPEED = 4.0f
private const val SPEED_UPDATE_THRESHOLD = 0.1fThis applies to lines 123, 210, 211, and this onDrag block.
|
Hi, can you give me a description and title of what you would like to do? |
2 participants
No description provided.