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

Opening and closing panel quickly breaks any further tap/click events #4108

Closed
peitschie opened this issue Dec 6, 2022 · 4 comments · Fixed by #4127
Closed

Opening and closing panel quickly breaks any further tap/click events #4108

peitschie opened this issue Dec 6, 2022 · 4 comments · Fixed by #4127

Comments

@peitschie
Copy link
Contributor

peitschie commented Dec 6, 2022

Describe the bug

If a panel is closed very quickly after opening (can happen if the user accidentally double-taps, for example), the app suddenly becomes unresponsive to further tap/click events. It seems that the backdrop element is left behind, despite the panel being closed.

To Reproduce

Steps to reproduce the behavior:

See CodeSandbox link: https://codesandbox.io/p/sandbox/dazzling-feather-0s7qc6
Tapping the "Open panel normally" shows a normal behaving panel. Tapping "Open panel and break app" will show no panel, but after tapping, no other link will work (i.e., app appears unresponsive).

In essence, the key is

  1. Start the panel opening
  2. Very quickly, close the panel (e.g., on the panelOpen event). This can be done manually by tapping very quickly on the panel open button, as it's possible to tap on the backdrop while the panel is still sliding open
  3. When the bug happens, the panel is closed, but the app stops responding to any taps or clicks

Expected behavior

The panel should close, but the app should remain responsive

Actual Behavior

The panel closes but the app becomes unresponsive to taps

Additional context

The root cause seems to be that the transitionEnd trigger never fires if the CSS properties are reverted quickly enough. Surprisingly, it's possible to visibly see the animation running when manually tapping quickly to cause the bug.

One possible idea (I'd be happy to raise a PR if you'd like) is to use Element.getAnimations() to see if the transitionEnd callback might not fire: https://developer.mozilla.org/en-US/docs/Web/API/Element/getAnimations

Basically, the panel open/close logic would be updated to set the CSS, then queue up a microtask (or even setTimeout(..., 0) task) which checks if getAnimations returns an active transition. If the check detects no transition and the transitionEnd callback hasn't fired, the callback would then be fired.

As pseudo-code, something would be added at

    function panelTransitionEnd() {
      transitionEndTarget.transitionEnd(...);
      // NEW: handle case where transitionEnd is not called
     queueMicroTask(() => {
       // TODO more accurately detect transition animations
       if (transitionEndTarget[0] && transitionEndTarget[0].getAnimations().length == 0) {
         transitionEndTarget.trigger('animationend'); // TODO is this the right way to trigger this?
       }
    }
    if (animate) {
      ...
      panelTransitionEnd();
      $el.removeClass('panel-out not-animated').addClass('panel-in');
      panel.onOpen();
    } else {
      ...
    }

@Simone4e
Copy link

Simone4e commented Dec 6, 2022

I tried to replicate the bug without using the script it doesn't seem to show up as you described, in fact I don't think there is a way to double click thus bugging the panel (the current event should be panelOpened instead of panelOpen)

@peitschie
Copy link
Contributor Author

peitschie commented Dec 6, 2022

@Simone4e the easiest way to replicate manually is with two fingers.

With the standard hamburger menu on the left side, tapping on the hamburger button on the left and then immediately tapping on the right side of the screen (i.e. on the backdrop with is immediately tappable) will usually cause the bug for me in a few attempts.

I've deliberately placed the close in the panelOpen event because that demonstrates the "perfect" case.

I agree this is not a common and easy to trigger bug, but the consequences for that unlucky user are reasonably severe in that an app restart is required to recover.

Imo, while I agree the code in the sandbox is a bit nonsensical, it still shouldn't break the whole app the way it does!

@Simone4e
Copy link

Simone4e commented Dec 7, 2022

@Simone4e the easiest way to replicate manually is with two fingers.

With the standard hamburger menu on the left side, tapping on the hamburger button on the left and then immediately tapping on the right side of the screen (i.e. on the backdrop with is immediately tappable) will usually cause the bug for me in a few attempts.

I've deliberately placed the close in the panelOpen event because that demonstrates the "perfect" case.

I agree this is not a common and easy to trigger bug, but the consequences for that unlucky user are reasonably severe in that an app restart is required to recover.

Imo, while I agree the code in the sandbox is a bit nonsensical, it still shouldn't break the whole app the way it does!

Tested with phone and bug exist.

peitschie added a commit to peitschie/framework7 that referenced this issue Feb 2, 2023
fixes framework7io#4108 (again)

In practice, the time until the transition start is variable depending on
the browser, amongst other things. Android + Chrome is between 27 => 44ms
with a fast phone, but can take longer under many circumstances.

This removes the hardcoded time and instead looks for the transition start.
@peitschie
Copy link
Contributor Author

I've still been experiencing this, as it seems the time between panel open and transition start is often 20-50ms. If the tap falls within this gap, the bug reoccurs.

The easiest way to show this in a normal app is using the Android monkey tool:

adb shell monkey -p com.some.appliaction --throttle 10 --pct-trackball 0 -v 1000

In a standalone app with just a menu button that launches a panel, this fails very reliably.

nolimits4web pushed a commit that referenced this issue Feb 2, 2023
fixes #4108 (again)

In practice, the time until the transition start is variable depending on
the browser, amongst other things. Android + Chrome is between 27 => 44ms
with a fast phone, but can take longer under many circumstances.

This removes the hardcoded time and instead looks for the transition start.
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 a pull request may close this issue.

2 participants