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

[Discussion] Show video details in landscape with swipe-up gesture (middle of screen) same as menu long click #4505

Open
vkay94 opened this issue Oct 13, 2020 · 6 comments
Labels
discussion This needs to be discussed before anything is done

Comments

@vkay94
Copy link
Contributor

vkay94 commented Oct 13, 2020

Hey guys, this is no enhancement issue but a discussion of a behavior idea.

Somewhere in #4332 there was a short discussion whether to use the middle part of the screen in fullscreen/landscape (left for volume, right for brightness, middle for swipe/fling to video details) or not. Unfortunately it is a pretty mess to make this fling gesture to work (interception stuff bla bla).

During researching the code base I found that in VideoDetailsFragment there is also this kind of feature already (to be honest it's too hidden): If you long press on the menu arrow - next to resolution and speed button - , then the layout flings down automatically.

This callback can be used to call it within onScroll with positive distanceY. The 'problem' here is,that it isn't a real fling gesture, more like a gesture to 'make it appear' / call the onMoreOptionsLongClicked method.

What do you think? Would it be enough to be implemented or not? (It just doesn't feel completed ...)

@opusforlife2
Copy link
Collaborator

You can swipe up on the full screen button for the same effect. avently actually implemented this for the play/pause/next/previous buttons as well, but that was removed after discussion in the RC thread.

Also, the fact that you didn't know this means you read neither the release notes nor the blog post. :(

@avently
Copy link
Contributor

avently commented Oct 14, 2020

@vkay94 yeah, it's quite hard to do in a correct way (via swipe, not via button like I did).
From my point of view the gesture should be implemented in a hard way or #4456 should be implemented instead.
The issue I mentioned allows to bind to single click the arrow down button which is much easier to use then long press.

Your solution with real swipe still would be much better. Even if it is hard to implement.
Actually implementation is not tooo hard. You need to know when the player is in fullscreen and divide the player view into three areas, capture all swipes in the middle of the view inside custombottomsheetbehavior. So, one file + probably refactoring of VideoPlayerImpl which makes rootView as a custom view class (PlayerView class) with a property isFullscreen so you could know this value inside custombottomsheetbehavior. This allows to enable triggering only when the player in fullscreen.
Isn't it easy for you and your skills?

@vkay94
Copy link
Contributor Author

vkay94 commented Oct 14, 2020

Also, the fact that you didn't know this means you read neither the release notes nor the blog post. :(

I know that but this fullscreen button is only visible if the device is locked, so in auto-rotate you can't do scroll to the details (that's the point). The arrow-long-press-gesture isn't mentioned anywhere as I've seen, have you?

You need to know when the player is in fullscreen and divide the player view into three areas, capture all swipes in the middle of the view inside custombottomsheetbehavior.

@avently That's no problem at all also detecting forisFullscreen.

Isn't it easy for you and your skills?

To admit, as it seems, it isn't :') The real problem is the order. CustomBottomSheetBehavior is the first which gets the event, then passing the event down to the VideoPlayerImpl's rootView. Once PlayerGestureListener uses the DOWN-event it can't pass it upwards in one flow and the MOVE-event (scroll) is coupled to this previous DOWN-event.

Touch interception handling in CoordinatorLayout and AppbarLayout isn't so pleasant (for me) xD

@avently
Copy link
Contributor

avently commented Oct 14, 2020

To admit, as it seems, it isn't :') The real problem is the order. CustomBottomSheetBehavior is the first which gets the event, then passing the event down to the VideoPlayerImpl

Since it's the first which receive the touch it will be the last who can allow to react on it. Like in any other situation when we intercepting touches inside custombottomsheetbehavior. Didn't see a problem you mentioned

@vkay94
Copy link
Contributor Author

vkay94 commented Oct 14, 2020

Since it's the first which receive the touch it will be the last who can allow to react on it. Like in any other situation when we intercepting touches inside custombottomsheetbehavior.

Yeah, that's correct, but when it returns super (necessary ) it won't recognize other events anymore unless everything afterwards pass the event upwards or aren't clickable and focusable. And 'PlayerGestureListener' may not return false directly (otherwise simple clicks won't work).

I give it a last try (just for my ambition haha) - I've already something in mind ...

@opusforlife2
Copy link
Collaborator

The arrow-long-press-gesture isn't mentioned anywhere as I've seen, have you?

@vkay94 I didn't know about it myself. :|

@AudricV AudricV added the discussion This needs to be discussed before anything is done label May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This needs to be discussed before anything is done
Projects
None yet
Development

No branches or pull requests

4 participants