vguillou / swipe-action Goto Github PK

In a few places this._currentAction is accessed without checking if it is null first, such as in _swipeUpdate, _swipeEnd, _applyGestureTranslation (called by _swipeStart), and _applyRubberBand (called by _applyGestureTranslation). Normally you'd think this would not be an issue, however putting <swipe-action> in a Polymer app-drawer, for example, causes the drawer being slide one way (a way that the swipe elements ignore, say the drawer is on the left and can be slid left, and the swipe-action can be swiped right) to cause the swipe-action to receive '_onTrackcallbacks, which then causes it to access anundefined this._currentAction`, thus crashing the event handling and causing exceptions in the scripts.

@vguillou I want to use left swipe to open an underlay and right swipe to close it. Is it possible?

The component seems to not work on Polymer 2.0.
The reason that i could understand was the change of the tag to and their behaviour.
Since I'm using this component in my project, I'm already working on a fix for this. Let me know if I should make a new file with the fix so that people with Polymer < 2.0 can still use the original component, or modify the same file.

Using Chrome (current version) on Android 6 (untested on others) the demo page, say at https://beta.webcomponents.org/element/vguillou/swipe-action/demo/demo/index.html go down to the <iron-list> example (though it does it on all, just using this as a specific example). Swipe a few of the entries left, they remain where you stopped swiping without 'bouncing back' to the default location, so you get this staggered pattern. It does bounce back roughly 50-10% of the time Up and down do not seem to have this issue. Swiping right only swipes, what looks to be only about 5-15px randomly, but cannot be dragged further, which does not give much room to interact with the element underneath. On the same android device with the same chrome the iron-swipeable-container demo at https://beta.webcomponents.org/element/PolymerElements/iron-swipeable-container/demo/demo/index.html does not have the same issue and swipes left and right work without issue (though it is significantly more limited and as such do not fancy rewriting it ^.^).

I'm not entirely sure why this happens, a quick look through the code seems correct on initial inspection... :-/

Love this element though, well designed. :-)

Specifically it is causing an access of undefined because on this line in swipe-action.html:

// Line 388 as of the latest release, inside the function activateAction
        var actionToActivate = this._actionsMap[actionId];

The this._actionsMap is undefined, the activateAction function is called from _onActivatedActionIdChange when this._activatedActionIdAlreadyChanged is undefined as well. _onActivatedActionIdChange appears to have a dependency on the element being isReady'd first, but that is not shown, thus it is being called before it is ready, and thus crashing.

What should probably happen from a quick look through the code is there should be an isReady check in _onActivatedActionIdChange and if not isReady then it should return early. Then in the function _refreshActions should probably call activateAction or so after it sets this._actionsMap.

To make more efficient updating, can a 'key' attribute or so be added to this, just whenever it changes from its previous value then reset the state of this? This would allow it to be used within dom-repeat when the items change on occasion (like removing them on delete), where currently if any are in a swiped state then they are still swiped even as the items are re-rendered with new content.

Basically just adding a new 'key' or 'keyed' or so property, have an observer on it, whenever it changes and does not equal its previous value then call this.reset(false) or so (perhaps with another property for whether it should be false/true?).

I can't set it with value other than ~64px or 100%

When activatedActionId is set to a falsey value like "" or 0 then as per the _onActivatedActionIdChange function (line 421 as of the current release version):

      _onActivatedActionIdChange: function(newValue, oldValue) {
        if (!this._activatedActionIdAlreadyChanged) {
          this._mutationObserverPaused = true;
          this._mute = 1;
          if (!newValue) {
            this._mute++;
            this.reset(false);
          } else {
            this.activateAction(newValue, false);
          }
        }
        this._activatedActionIdAlreadyChanged = false;
      },

It is setting this._mute = 1, then since newValue is falsey it then sets this._mute++, which is setting this._mute to be 2, then calls this.reset(false), and reset calls:

      reset: function(animate) {
        // Snap the item back into place
        this._snapBack(animate);
      },

And _snapBack calls:

      _snapBack: function(animate) {
        if (this.isSwiping || this.isSwipedAway) {
          Polymer.dom(this.target).classList.add('snap-back');
        }
        // Animation
        if (animate) {
          Polymer.dom(this.target).classList.add('animated');
        } else {
          this.target.style.transition = '';
          this._cleanupAfterTransition();
        }
      },

Which since animate is false it will call the else branch, which calls this._cleanupAfterTransition(), which is:

      _cleanupAfterTransition: function() {        
        // If item is swiped away, the swipe-action is activated: fire the appropriate event
        if (this._willBeSwipedAway) {
          this._willBeSwipedAway = false;
          this._changeActivatedAction(this._currentAction);
          // Fire activation event
          this._fireEvent('swiped-away');
          return;
        }
        // If the target node won't be swiped away, then it will be snaped back,
        // and the action won't be activated anymore
        else if (Polymer.dom(this.target).classList.contains('snap-back')) {
          this._changeActivatedAction(undefined);
          // Fire cancelation event
          this._fireEvent('swipe-canceled');
        }
        // Cleanup to do AFTER the event is fired
        this._mute--;
      },

Which does a few things, but ending up with this._mute--, which will set this._mute back to 1, so now this._mute is left at 1. Now later when the element is swiped away, which ends up calling this same function but with this._willBeSwipedAway set to true, so this._fireEvent('swiped-away') gets called, and it is:

      _fireEvent: function(eventName) {
        if (!this._mute) {
          this.fire(eventName, {action: this._currentAction, gesture: this._gesture});
        }
      },

Where this._mute is still 1, thus this event is skipped and never fired, ever again. The issue appears resolved by going back to the first _onActivatedActionIdChange function definition of:

      _onActivatedActionIdChange: function(newValue, oldValue) {
        if (!this._activatedActionIdAlreadyChanged) {
          this._mutationObserverPaused = true;
          this._mute = 1;
          if (!newValue) {
            this._mute++;
            this.reset(false);
          } else {
            this.activateAction(newValue, false);
          }
        }
        this._activatedActionIdAlreadyChanged = false;
      },

And just wiping out the this._mute++; line entirely, that will leave this._mute as 1 in the reset call, which ends up setting it back to 0 when this._mute-- is run in the _cleanupAfterTransition function, thus restoring the state.

However, this reveals another bug, this._mute is never initialized unless activatedActionId is set, meaning that it is by default undefined. When _cleanupAfterTransition is called on every swipe it will in turn call this._mute-- on an undefined value, thus setting this._mute to be NaN, which will allow the events to be fired when _fireEvent is called regardless. But by setting activatedActionId it makes this._mute = 0 by the end of its call (or 1 without the above fix in some cases), so when it is swiped or reset it decrements this._mute via the this._mute-- call inside _cleanupAfterTransition, which makes it <0, thus in the _fireEvent function:

      _fireEvent: function(eventName) {
        if (!this._mute) {
          this.fire(eventName, {action: this._currentAction, gesture: this._gesture});
        }
      },

Then !-1 via the !this._mute check (or lower value) will be false and the events stop firing again and never fire again until activatedActionId is set again, of which it will then only fire once more and never again until it is set yet again.

This second issue 'should' be fixed properly by always setting a default value to this._mute on construction and fixing up the code elsewhere, however a quick-fix that will always work (in my testing) is to just change this._mute--; in function _cleanupAfterTransition into being this._mute = this._mute>0 ? this._mute-1 : 0; since it being the only place doing the decrementing means that it needs to make sure it does not go below 0 and it will always default set it to 0 otherwise.