Skip to content
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

Fast forward on long press #486

Merged
merged 33 commits into from
Sep 21, 2023

Conversation

Yashraj254
Copy link
Contributor

@Yashraj254 Yashraj254 commented Aug 3, 2023

sample5.mp4

@anilbeesetti
Copy link
Owner

anilbeesetti commented Aug 4, 2023

@Yashraj254 Could you use Coroutine instead of handler

@anilbeesetti
Copy link
Owner

And the gesture also works with two fingers. Make it only activate with one finger

@Yashraj254 Yashraj254 force-pushed the fast_forward_on_long_press branch from a7432d5 to 193c9d4 Compare August 4, 2023 13:39
@Yashraj254
Copy link
Contributor Author

Yashraj254 commented Aug 4, 2023

@anilbeesetti i am not able to make it work for the case, when user is holding the screen with two fingers and if he lift up one of his finger while holding the screen with the other one , it won't work unless he releases his other finger as well and tap and hold the screen again...
so any suggestions to make it work for that case as well..?

@Yashraj254
Copy link
Contributor Author

Yashraj254 commented Aug 10, 2023

@anilbeesetti i have made some changes to it, it works now.

@anilbeesetti
Copy link
Owner

@Yashraj254 Unfortunately, i am putting this pr on hold until the issue #459 is fixed since this pr also adds the slider dialog. some users are reporting crash on changing the slider value. I need to look into the crash issue first. Btw does slider crash issue happen on your side too or everything good

@Yashraj254
Copy link
Contributor Author

@anilbeesetti No, no crashes, not even for once

@Yashraj254
Copy link
Contributor Author

Yashraj254 commented Aug 11, 2023

@anilbeesetti how about using Firebase Crashlytics in this app...

@anilbeesetti
Copy link
Owner

No way, i'm against that proprietary crap

@Yashraj254
Copy link
Contributor Author

No way, i'm against that proprietary crap

@anilbeesetti ok no problem, btw is there any specific reason behind why not to use it.. i haven't personally used it before, all i know it helps in tracking app crashes..

@anilbeesetti
Copy link
Owner

Because, it's not open source and it's google product. And doesn't align with foss principles

@Yashraj254
Copy link
Contributor Author

Ok, got it.

@ShareASmile
Copy link
Contributor

Firebase Crashlytics has foss alternatives see z-huang/InnerTune#935 (comment)

@drogga
Copy link

drogga commented Aug 23, 2023

The reason for not merging this seems now resolved (for those affected in the first place) and the issue ticket is closed, so maybe you can merge this PR, I like this feature in XPlayer and it will be nice if this awesome player has it to.

@anilbeesetti Please read your E-Mails, there should be at least ~2 to 4 with some suggestions...
Thank you all in advance.
[if this comment is problematic, I will delete it, no problem, don't worry ;)]

@anilbeesetti
Copy link
Owner

@drogga do suggestions, issues in github only. And this will be merged as soon as possible

@drogga
Copy link

drogga commented Aug 23, 2023

It won't harm\hurt if you read them (3 E-Mails total) anyway, just to get a general idea of what they are about, but OK, no worries, I will post them here in the discussions tab, because I don't like creating issue tickets in GitHub, but prefer E-Mails (I have my reasons and not having PMs\DMs here doesn't help).

Thanks once again, looking forward to the version with this implemented ;)

Yashraj254 and others added 9 commits September 12, 2023 02:41
…press

# Conflicts:
#	core/model/src/main/java/dev/anilbeesetti/nextplayer/core/model/PlayerPreferences.kt
#	core/ui/src/main/res/values/strings.xml
#	feature/player/src/main/java/dev/anilbeesetti/nextplayer/feature/player/utils/PlayerGestureHelper.kt
#	feature/settings/src/main/java/dev/anilbeesetti/nextplayer/settings/screens/player/PlayerPreferencesScreen.kt
@Yashraj254
Copy link
Contributor Author

@anilbeesetti hey, i think it can be merged now,or do you still see any app crashes or any other issues..

@Yashraj254
Copy link
Contributor Author

my bad, i forgot to test it when the video is already playing at a different speed other than the default(1.0f) speed..

@anilbeesetti anilbeesetti merged commit 18d3693 into anilbeesetti:main Sep 21, 2023
@anilbeesetti
Copy link
Owner

@Yashraj254 Thank you for implementing this feature

@Yashraj254
Copy link
Contributor Author

@anilbeesetti thanks to you too as you worked more than me on this feature and i got to learn more about covering the edge cases..

@Yashraj254 Yashraj254 deleted the fast_forward_on_long_press branch September 22, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add long press gesture(添加长按手势)
4 participants