-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Enhancement: Add fast forward and rewind pop up to video player #7729
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
Enhancement: Add fast forward and rewind pop up to video player #7729
Conversation
|
Hi @caetano-dev i think you misinterpreted that issue wasnt talking about implementing a popup for fastforward and rewind. I will show you an example what they meant with their issue. YT: In this clip im fastforwarding the video. When pressing the key that fastforwards the seekbar and player UI appears firefox_iSH4qp2rjx.mp4FT: Here im doing the same but the seekbar and the rest of the player UI doesnt appear FreeTube_VbjsnaP7nX.mp4Your PR is more relatable to #6473 Please remove that issue from the PR body. Also maybe weird idea/request but if going forward show arrow on the right side of the seconds skipped and if going back show it on the left side. Please challenge me if this is something that will confuse users or is just wrong 😅 |
|
@efb4f5ff-1298-471a-8973-3d47447115dc oh, thanks for clarifying that. I did misunderstood the issue. Do you still want to add this pop up? I agree with you on the arrow placement and I can try fixing that. |
Yes, it will be a nice addition on top of the other pop ups we have. I think that this was the last of that was missing
Sure go for it, thanks |
Head branch was pushed to by a user without write access
Pull request was converted to draft
Head branch was pushed to by a user without write access
|
This doesnt seem like very useful information to me. Maybe hide it for this shortcut. Feel free to suggest idea's though VirtualBoxVM_GqgFiuFzFN.mp4 |
Head branch was pushed to by a user without write access
|
I restricted this specific pop up to the J, L and arrow shortcuts. I believe they are the ones that make sense having it. What are your thoughts? |
efb4f5ff-1298-471a-8973-3d47447115dc
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.
Seems totally reasonable to me, LGTM!
| ? { icon: 'arrow-right', invertContentOrder: true } | ||
| : { icon: 'arrow-left', invertContentOrder: false } | ||
| const formattedSeconds = Math.abs(seconds) | ||
| showValueChange(`${formattedSeconds}s`, popUpLayout.icon, popUpLayout.invertContentOrder) |
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.
nitpick: this isn't properly localized, but I don't believe we localize the s for seconds in other areas like our settings, so consistency is fine


Pull Request Type
Description
Adds a pop up when the user fast forwards or rewinds a video.
Screenshots
Testing
Desktop