RWAP / jquery-ui-touch-punch

A duck punch for adding touch events to jQuery UI
http://touchpunch.furf.com/
103 stars 29 forks source link

Seemingly incorrect binding, 1.1.1 onwards #39

Closed bazineta closed 6 months ago

bazineta commented 6 months ago

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.

https://github.com/RWAP/jquery-ui-touch-punch/commit/9efd7e1066a9c91960a543ea40f758d07bba8465

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 6 months ago

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);
  };
bazineta commented 6 months ago

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 6 months ago

Many thanks - now incorporated in the code