-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
issue-10971-swipe-down-gesture-too-sensitive #11642
Conversation
…imation disappear within the threshold distance
Issue10971
Thank you for your PR. Due to the influx of new PRs review might take some time. Please let me know if you are an ANU student for internal statistics and because I am writing my master thesis about open source contributions as part of learning computer science and computer science education at university. |
revert changes to gradle files sice its failed
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.
Thank you for the PR. Please rebase it and follow the linter / checkstyle. I'll take a closer look once the pipelines succeed.
One general tip when working with platforms like GitHub and GitLab to propose changes and / or get code reviews: You do not need to post screenshots of your changes. The changes are tracked. If you want to comment on your changes, you can either reference them in the PR body or start discussions directly as part of the code review.
app/src/main/java/org/schabi/newpipe/player/gesture/CustomBottomSheetBehavior.java
Outdated
Show resolved
Hide resolved
@@ -199,8 +199,7 @@ dependencies { | |||
// name and the commit hash with the commit hash of the (pushed) commit you want to test | |||
// This works thanks to JitPack: https://jitpack.io/ | |||
implementation 'com.github.TeamNewPipe:nanojson:1d9e1aea9049fc9f85e68b43ba39fe7be1c1f751' | |||
// WORKAROUND: v0.24.2 can't be resolved by jitpack -> use git commit hash instead | |||
implementation 'com.github.TeamNewPipe:NewPipeExtractor:176da72cb4c3ec4679211339b0e59f6b01bf2f52' | |||
implementation 'com.github.TeamNewPipe:NewPipeExtractor:v0.24.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.
please clean your git history (rebase) and this is fixed on the dev branch.
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.
Thanks for your detailed advice. I am still wondering about whether implementing swipe down threshold is a proper methods to solve this. As when the threshold is implemented. Within the threshold, the user gesture will be ignored and no interaction can be seen. Previously, the instantce user drag the screen, the view window will move with the drag. After distance threshold implementation, within the threshold distance, when you drag, there is no response, the window will not move. I am not very sure if this is the correct direction to solve this problem.
…omSheetBehavior.java Co-authored-by: Tobi <[email protected]>
Hello, and thank you for your contribution. I agree that just ignoring the gesture is not good UX. However in the refactored NewPipe gestures will be handled by NewPlayer, so any improvements should go there. Thank you anyway for your PR, I'm sorry for not giving you the chance to improve it right now, but we want to focus all of our energies on the refactor. |
What is it?
Description of the changes in your PR
Before :
After:
Files that changed: app/src/main/java/org/schabi/newpipe/player/gesture/CustomBottomSheetBehavior.java
Change screenshoots:
the distance threshold can be changed.
Before/After Screenshots/Screen Record
before.mp4
after.mp4
Fixes the following issue(s)
APK testing
Due diligence