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

FlickToDismiss drops frames when it is removed from composition before it completes its dismiss animation #47

Open
MV-GH opened this issue Sep 15, 2023 · 6 comments

Comments

@MV-GH
Copy link

MV-GH commented Sep 15, 2023

Follow up on #43 (comment)

I have implemented your library to replace the existing functionality and it seems to work well.

But I was wondering if I could remove the delay of the the dismiss action?

If I remove the delay it becomes very choppy. But I would like it to be instant like the other functionality, I already have.

    (flickState.gestureState as? FlickToDismissState.GestureState.Dismissing)?.let { gestureState ->
        LaunchedEffect(Unit) {
            delay(gestureState.animationDuration / 6) // Seems to be the lowest I can get away with.
            appState.navigateUp()
        }
    }

see choppyness

IiccX2jkJm.mp4
@saket
Copy link
Owner

saket commented Sep 15, 2023

This entirely depends on your navigation stack. If your app only supports showing one screen at a time then you can't do much. You must wait till the dismiss animation is complete before removing it from the UI hierarchy.

Can you refactor your media viewer screen to be shown as an overlay above other screens? This way you'll also be able to use a translucent background for media as they're being swiped up/down for dismissal.

@saket saket changed the title Delay in Flick, can it be removed? FlickToDismiss drops frames when it is removed from composition before it completes its dismiss animation Sep 15, 2023
@MV-GH
Copy link
Author

MV-GH commented Sep 15, 2023

We used to have it as a overlay through a Dialog but that had numerous issues. So I had resort to using a separate screen. (see LemmyNet/jerboa#980)

@saket
Copy link
Owner

saket commented Sep 15, 2023

Yeah dialogs can be annoying. But does compose-navigation allow other ways of showing overlays without using a dialog? Can Popup() be used?

@MV-GH
Copy link
Author

MV-GH commented Sep 15, 2023

We do have some places that we use a Popup. But I am reluctant to make this change as we have dropped Flick for now (and the previous impl). As Flicks behaviour doesn't really fit for our usecase. As soon it enters the flick animation due to moving touch up or down. It will no longer allow to zoom. While ideally we want when a movement happens above a certain threshold. It would dismiss the action without having a impact on any actions listening to that movement.

@saket
Copy link
Owner

saket commented Sep 19, 2023

As soon it enters the flick animation due to moving touch up or down. It will no longer allow to zoom. While ideally we want when a movement happens above a certain threshold

Do I understand this correctly that FlickToDismiss is too sensitive right now?

@MV-GH
Copy link
Author

MV-GH commented Sep 19, 2023

You could say that but what they want, is to not be interrupted while attempting to zoom. (I am mostly assuming as I don't really experience this, maybe one finger comes down significantly before the other, or their devices are much more sensitive)

Feedback i got:

It’s pretty much back to the behavior explained above. If you slightly move your fingers before zooming in, you are stuck at flicking the image away and can’t zoom.

It’s not, as soon as the image moves vertically I can’t zoom in anymore and it is easy to accidentally make it move before pinching. I think the image shouldn’t respond to vertical swipes with two fingers at all and should have a small dead zone for single finger swipes. That’s how WhatsApp seems to do it
Also the black background takes too long to disappear after a flick, it could be nicer if it faded away

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

No branches or pull requests

2 participants