RWAP/jquery-ui-touch-punch

Seemingly incorrect binding, 1.1.1 onwards

Closed this issue · 3 comments

We'd been using version 1.0.8, and updated to 1.1.4. Found that horizontal scrolling within an overflow-x: scroll container wasn't working on mobile, and traced it to this change, where $.proxy was replaced with standard binding.

9efd7e1

The change is a bit different in implementation than the proxy calls, which bound to self within the _mouseInit function; the change instead binds to mouseProto, outside that function. Moving the binding back into that function, and using self as the binding context does appear to restore functionality.

RWAP commented

That makes sense - what would you suggest as the changed code - something like this:

  /**
   * A duck punch of the $.ui.mouse _mouseInit method to support touch events.
   * This method extends the widget with bound touch event handlers that
   * translate touch events to mouse events and pass them to the widget's
   * original mouse event handling methods.
   */
  mouseProto._mouseInit = function () {

    let self = this;

    let _touchStartBound = self._touchStart.bind(mouseProto),
        _touchMoveBound = self._touchMove.bind(mouseProto),
        _touchEndBound = self._touchEnd.bind(mouseProto);
	  
    // Microsoft Surface Support = remove original touch Action
    if ($.support.mspointer) {
      self.element[0].style.msTouchAction = 'none';
    }	  

    // Delegate the touch handlers to the widget's element
    self.element.on({
      touchstart: _touchStartBound,
      touchmove: _touchMoveBound,
      touchend: _touchEndBound
    });

    // Call the original $.ui.mouse init method
    _mouseInit.call(self);
  };

  /**
   * Remove the touch event handlers
   */
  mouseProto._mouseDestroy = function () {

    let self = this;

    // Delegate the touch handlers to the widget's element
    self.element.off({
      touchstart: _touchStartBound,
      touchmove: _touchMoveBound,
      touchend: _touchEndBound
    });

    // Call the original $.ui.mouse destroy method
    _mouseDestroy.call(self);
  };

Close; here's what we have; the variables still do need to be in the outer scope.

  let _touchStartBound;
  let _touchMoveBound;
  let _touchEndBound;

  /**
   * A duck punch of the $.ui.mouse _mouseInit method to support touch events.
   * This method extends the widget with bound touch event handlers that
   * translate touch events to mouse events and pass them to the widget's
   * original mouse event handling methods.
   */
  mouseProto._mouseInit = function () {

    let self = this;
	  
    // Microsoft Surface Support = remove original touch Action
    if ($.support.mspointer) {
      self.element[0].style.msTouchAction = 'none';
    }	  

    _touchStartBound = mouseProto._touchStart.bind(self);
    _touchMoveBound  = mouseProto._touchMove.bind(self);
    _touchEndBound   = mouseProto._touchEnd.bind(self);

    // Delegate the touch handlers to the widget's element
    self.element.on({
      touchstart: _touchStartBound,
      touchmove: _touchMoveBound,
      touchend: _touchEndBound
    });

    // Call the original $.ui.mouse init method
    _mouseInit.call(self);
  };

  /**
   * Remove the touch event handlers
   */
  mouseProto._mouseDestroy = function () {

    let self = this;

    // Delegate the touch handlers to the widget's element
    self.element.off({
      touchstart: _touchStartBound,
      touchmove: _touchMoveBound,
      touchend: _touchEndBound
    });

    // Call the original $.ui.mouse destroy method
    _mouseDestroy.call(self);
  };
RWAP commented

Many thanks - now incorporated in the code