Newpipe: Non-horizontal drags to the seek bar activate vertical dragging instead of seeking

Created on 13 Aug 2020  ·  23Comments  ·  Source: TeamNewPipe/NewPipe

Version

  • d7af0195118e53c38037516bec10be92a33c68ce

Steps to reproduce the bug

  • Install recent dev build with unified player.
  • Open a video in the main player.
  • Tap the video so the seek bar appears.
  • Press the seek bar either on or off the current position (doesn't matter), optionally hold your finger still, then drag the seek bar diagonally or vertically.
  • Expected behavior

    If you press the seek bar, then all drags will be interpreted as seeks, and not vertical sliding.

    Actual behaviour


    If your first movement (including after you hold still) is more vertical (either up or down) than horizontal, then the finger press is interpreted as a vertical scroll instead of seeking.

    If your initial press is above a horizontal line (slightly below the seek bar), a vertical drag moves the video down. Below that line (but still in the video and not the title), it drags the description/comments section up.

    In the official YouTube app (or possibly an older version), the seek bar appears between the video and the description, and dragging it vertically is still considered a seek, not an attempt to collapse the video.

    Screenshots/Screen recordings

    https://youtu.be/c7M55-zRahQ

    bug player

    All 23 comments

    drags are bad, mkay.

    Vertical dragging triggers vertical dragging and ... it's wrong? ¯\_(ツ)_/¯

    In 0.19.8, in both portrait and landscape modes, the moment you touch the seekbar anywhere, the thumb (why is it called that?) jumps to under your finger, and a timer comes on screen which tells you the seek position.

    In the unified player, touching the seekbar does nothing. It is the swipe action _after_ touching which activates the seekbar and repositions the thumb. Until then, the app doesn't know if you're going to swipe horizontally or vertically, so it doesn't show the same behaviour as the current release.

    I think the point here is usability. There is now a non-zero likelihood (versus zero in the current release) that a person will trigger the wrong gesture and drag the video when they actually want to seek.

    While testing this, I discovered that in landscape mode, the unified player's seekbar and rotation button (but not the playing time and video length, strangely) are _another swipe zone_ in addition to the one at the top. Why did you make this design choice?

    Why did you make this design choice?

    I didn't do anything special. It just works like this because the whole player view located inside AppBarLayout which responds to a swipe. So when you swipe inside player in landscape all touches intercepted by volume and brightness gestures but when you touching a button this interception is not working and the swipe gesture intercepted by top-level viewGroup which is AppBarLayout.

    Got it, got it. So just like the volume and brightness gesture areas, is it possible to 'protect/reserve' the entire bottom area such that it isn't intercepted by AppBarLayout? This would solve this bug in portrait mode as well as the funky jumpy behaviour the video shows when swiping in landscape, as I discovered above.

    is it possible to 'protect/reserve' the entire bottom area such that it isn't intercepted by AppBarLayout?

    Yes, it's possible

    Do this:
    enter , R.id.bottomControls
    here: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/player/event/CustomBottomSheetBehavior.java#L28

    enter

                            // Makes bottom part of the player draggable in portrait when
                            // playbackControlRoot is hidden
                            if (element == R.id.bottomControls
                                    && child.findViewById(R.id.playbackControlRoot)
                                    .getVisibility() != View.VISIBLE) {
                                return super.onInterceptTouchEvent(parent, child, event);
                            }
    

    before this line: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/player/event/CustomBottomSheetBehavior.java#L54

    enter

        final View seekBar = child.findViewById(R.id.playbackSeekBar);
            if (seekBar != null) {
                final boolean visible = seekBar.getGlobalVisibleRect(globalRect);
                if (visible && globalRect.contains((int) ev.getRawX(), (int) ev.getRawY())) {
                    allowScroll = false;
                    return false;
                }
            }
    

    before this line: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/com/google/android/material/appbar/FlingBehavior.java#L72

    Your problem and the problem from this issue will be fixed. If you confirm that the solution works, I'll make PR someday. Or you can do it yourself, just copy-paste the code and ensure that it works without problems.

    🤭 Since you're basically spoon-feeding me the code, I _could_ try it out and even make a PR, but the moment the devs review it and ask me to make changes, I'll be like a deer caught in the headlights.

    @Stypox Is this something you have time for? If not, I could try making a PR, but I expect it to be a messy affair.

    @opusforlife2 it's not difficult for me to make a PR when code is ready. I just don't have a time on testing. I did some testing and the code works ok. Just need a confirmation that it works in every situation

    Could you send me the apk real quick? I'll test it and get right back to you.

    @opusforlife2

    app-release.zip

    While testing this, I discovered that in landscape mode, the unified player's seekbar and rotation button (but not the playing time and video length, strangely) are another swipe zone in addition to the one at the top. Why did you make this design choice?

    I tested the apk download. The playing time and video length still allow you to drag the issue up and scroll down to the description and comments, but do not allow you to drag the page down to collapse the video player. Not a big deal, but an inconsistency nonetheless.

    Other than that, this functionality seems better than the current version of the unified UI. Thanks!

    The playing time and video length still allow you to drag the issue up and scroll down to the description

    scroll down to the description and comments

    scroll down to the description

    Intentional because of this. So now you can do it in landscape when you have visible fullscreen button (swipe from down to up on the button)

    Nitpick: In the build you posted:

    • When not in full screen, both dragging the timestamps and full-screen button allow scrolling down.
    • In full screen, only dragging the full-screen button lets you scroll down while maintaining full-screen. Dragging the timestamps does nothing, and dragging the seek bar seeks instead.

    I didn't know you could scroll down in full-screen before reading your comment.

    Nitpick

    i do not accept nit-picking. I have my life not for this.

    @avently

    So now you can do it in landscape when you have visible fullscreen button (swipe from down to up on the button)

    This is AWESOME. I just tried it out. It looks amazing, and pretty useful for those who want to view comments/suggested videos while the video is playing.

    Anyway, first of all, the main bug this issue was opened for is confirmed FIXED. Vertical swipes from the seekbar no longer activate dragging, but seek to that position instead. So no more of this:

    There is now a non-zero likelihood (versus zero in the current release) that a person will trigger the wrong gesture and drag the video when they actually want to seek.

    Please add fixes linkage to this issue when you open the PR.

    Now, apart from that,

    While testing this, I discovered that in landscape mode, the unified player's seekbar and rotation button (but not the playing time and video length, strangely) are another swipe zone in addition to the one at the top.

    FIXED. What I forgot to mention here yesterday was that I discovered that swiping up on the seekbar would cause the video to "jump" down to accommodate the status bar appearing. I did mention it in passing, but not the full thing:

    as well as the funky jumpy behaviour the video shows when swiping in landscape, as I discovered above.

    It looked glitchy as hell. Experimenting with _that_ was what led me to discover that the bottom area was a swipe zone. This is now fixed. No glitchy jumps. We have probably avoided a bunch of new bug reports complaining about this.

    ^ I'll open this as a separate issue so that you can add fixes linkage to it in your PR whenever you open it.

    In 0.19.8, in both portrait and landscape modes, the moment you touch the seekbar anywhere, the thumb (why is it called that?) jumps to under your finger, and a timer comes on screen which tells you the seek position.

    It still takes a swipe to activate the seekbar instead of just a touch. Is the current release's behaviour impossible to implement now or did you decide not to for a design reason?

    @nyanpasu64 Open a separate issue. Maybe someone decides to take a shot at it. Personally, I don't mind it since it's only a cosmetic issue, and no functionality is impacted.

    @opusforlife2 would you have time to provide a video with this "scroll up gesture" ? I cant reproduce so i wonder if i understand well what you are all 3 talking anout :)
    This would be pretty kind from you

    @justanidea Lock system rotation. Open a video. Notice the fullscreen button on the right side of the seekbar. Tap to go fullscreen. Now swipe upwards on this same button.

    Is the current release's behaviour impossible to implement now or did you decide not to for a design reason?

    I just didn't try to change the behavior. Single tap is working, swipe is working, no problem for me.

    Not a tap. Touch and hold there.

    @opusforlife2 Oh thx. System rotation locking was missing👍🏻

    i think this is great... it would solve the constant rotation between landscape and portrait mode. although I wish it would work regardless of rotation states... I mean if swiping from bottom to top would work always the same like swiping from top to bottom... video info could have at the bottom the same gesture zone size as the top gesture zone

    anyway I think this feature should be rotation states free😉

    another cool thing would be if the video would keep playing in the background the same as when pressing playlists bottom in full screen mode... what do you guys think of this?

    @MD77MD New issue. New issue. Or, if you want to discuss only, then come to Newpipe's IRC.

    Was this page helpful?
    0 / 5 - 0 ratings