-
-
Notifications
You must be signed in to change notification settings - Fork 770
Feat: Add decrease/increase speed buttons option (closes #3282) #3314
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
Conversation
|
thank you guys !! |
| child: svgIncrease, | ||
| onclick: function () { | ||
| const currentSpeed = ImprovedTube.playbackSpeed(); | ||
| let newSpeed = Math.min(currentSpeed + 0.25, 2); |
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.
Could you add an option so the step can be configured?
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.
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.
PerchunPak
left a comment
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.
Those are nitpicks, but I am not a fan of AI artifacts
Co-authored-by: Perchun Pak <[email protected]>
Co-authored-by: Perchun Pak <[email protected]>
|
Looks like speed-changing keybinds do a bit more logic than just changing the speed. Can we reuse that function?
|
If we call it, it would be necessary to change a little bit this shortcutIncreasePlaybackSpeed. This method picks the step from shortcuts_playback_speed_step, which is a different variable from the one we use to set the step (player_custom_playback_speed_step). |
We should move this method somewhere in a shared place, rename it and just provide the step as a function argument |
|
thank you so much!! :) @pecazeco @PerchunPak
There is a lot of potential for this project to be more of a framework. |
I don't really understand what you mean. I only talked about business logic, which is identical in both of those places. |


What:
Why:
How to test:
Notes: