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

peitschie commented 1 year ago

Framework7 version: 7.0.8
Platform and Target: Chrome + Firefox browsers, Android Cordova
Live Link or CodeSandbox: https://codesandbox.io/p/sandbox/dazzling-feather-0s7qc6

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

Start the panel opening
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
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 https://github.com/framework7io/framework7/blob/94ffca087841e0b8e8c305e37157c0d9ddd0e3e0/src/core/components/panel/panel-class.js#L460

    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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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.

framework7io / framework7