Remove jQuery

[simonihmig]

This changes event handling to use native events only, thus removing the dependency on jQuery.

Unfortunately, there aren't really many tests in this repo, that would cover the changes here. But I used this fork in a bigger app of us, with a pretty high test coverage, and this was working just fine!

[simonihmig]

Note: I removed quite a bit of code duplicated from the original EventDispatcher, by calling its super method here, instead of overriding and duplicating everything.

[eriktrom]

this is great work -- just so i fully understand -

previously we called assign(events, this.get('_gestures')); then this._fastFocus();, and then registered each event, including those added via this.get('_gestures') --- now we're calling this.set('events', events); then this._fastFocus(); and then calling return this._super(additionalEvents, rootElement); to allow the ember event dispatcher to do the for (event in events) {... this.setupHandler ... for us, correct?

If I'm missing something, please let me know.

One more q, where'd this guy go?

$root.on('click.ember-gestures touchend.ember-gestures', function (e) {

other than that, at least with just an eyeball review on github here, it looks like you nailed it... is there any way u could bring a piece of code from ur app into a test? (there isn't really much testing now.. so u can provide another way to prove its correctness) -- basically anything that will verify the new behavior matches the previous behavior, without careful reading through the diff here, would be nice, in some form, even logging the events registered through ember vs those on the rootElement itself, via this._fastFocus(), aka, those not using the previous verbose this.setupHandler loop -- would ease any concerns I have over my own ability to read and understand this code, old and new :)

also @runspired should approve this as he wrote it originally... i think i made any concerns obvious, the rest checks out except the ordering where mentioned, and the missing .on('click... from jquery (maybe i just missed it).

[simonihmig]

@eriktrom the changes here lived in fork I created a while ago, so I have to refresh my memories a bit... 😬

to allow the ember event dispatcher to do the for (event in events) {... this.setupHandler ... for us, correct?

Yes, exactly. setup() supports a list of additional events, so I just use that to pass the gestures specific events to the parent to let it attach the event listeners for them.

One more q, where'd this guy go?

This adds click and touchend listeners via jQuery, and the same happens here:
https://github.com/html-next/ember-gestures/pull/114/files#diff-89ef10d070270d2503408f9f16d8b558R91
for the fastFocusEvents here: https://github.com/html-next/ember-gestures/pull/114/files#diff-89ef10d070270d2503408f9f16d8b558R27

is there any way u could bring a piece of code from ur app into a test

Hm, not really. These are app specific (lots of acceptance and component integration tests), which would certainly have failed indirectly if the EventDispatcher was not working correctly. I also verified that some gestures we used like swipes were still working.

For adding some real tests here, I am afraid to not have enough time for this atm, to do it properly. :(

[simonihmig]

This file seems to not be used at all!?

[eriktrom]

if no one imports it, i'd say yep -- looks like an initial or refactor attempt at the event dispatcher override...

[eriktrom]

@simonihmig - great work, thanks. published as v1.1.0 to npm, let me know of any issues.

[danDanV1]

Using the 1.1.1 release, I am receiving a jquery event. Is this expected?

import RecognizerMixin from "ember-gestures/mixins/recognizers";
export default Component.extend(RecognizerMixin, {
  recognizers: "swipe",
swipe(e) {
      debugger;
      console.log("swipe");
    }

debugger:

>e
Proxy {originalEvent: Event, type: "swipe", isDefaultPrevented: ƒ, target: img, currentTarget: div#ember2047.h-full.w-full.overflow-hidden.__swipers__swiper__f0899.ember-view, …}
[[Handler]]: Object
[[Target]]: jQuery.Event
[[IsRevoked]]: false

[simonihmig]

I am receiving a jquery event. Is this expected?

I think so, if you are still using jQuery in your app, Ember's EventDispatcher will use it and dispatch jQuery.Events. Not sure how events are handled by hammer.js, but by looking at your debug output the "fake" swipe event seems to be a Proxy to the "real" event, in this case a jQuery event.

When you opt out of jQuery, you should only receive native events!

[danDanV1]

thanks @simonihmig got me on the right path. Can't really turn of jquery yet in the App because ember data is still dependant.

Learned the following about the transition:
jQuery events copy most of the properties of their native event counterpart, but not all of them.

eg. Before:

// your event handler:
click(event) {
  let x = event.originalEvent.clientX;
  ...
}

After:

// your event handler:
click(event) {
  let x = event.clientX;
  ...
}

"But there are cases where jQuery specific properties have to be used (when jQuery events are passed). This is especially true for the originalEvent property, for example to access TouchEvent properties that are not exposed on the jQuery.Event instance itself."

So for the case of ember gesture and touch events we have to normalize the event:

import { normalizeEvent } from 'ember-jquery-legacy';

// your event handler:
click(event) {
  let nativeEvent = normalizeEvent(event);
  ...
}

Going to note this to help others.
https://github.com/ember-learn/deprecation-app/blob/master/content/ember/v3/jquery-event.md
https://github.com/emberjs/rfcs/blob/master/text/0294-optional-jquery.md

[simonihmig]

Can't really turn of jquery yet in the App because ember data is still dependant.

Getting a bit off-topic here for this repo, but you can use ember-data without jQuery, using ember-fetch and its adapter mixin! Built-in support will hopefully land soon as well.