akre54 / Backbone.NativeView

A reference implementation of a native Backbone.View
MIT License
113 stars 18 forks source link

InputEvents not binding correctly #26

Open ahumphreys87 opened 8 years ago

ahumphreys87 commented 8 years ago

Hi,

I have the following events hash:

events: {
  'blur input': 'someHandler
}

someHandler is never called> I think this may be because NativeView is relying on the Event propagating up the tree and checking whether selectors match on the eventTarget. Is the issue that InputEvents such as blur, input or change don't get propagated up to non-input nodes?

Would a better way to attach events be to query the view for the selector and use addEventListener on the Elements that returns rather than the Views root element?

ahumphreys87 commented 8 years ago

Noticed this works if we change: https://github.com/akre54/Backbone.NativeView/blob/master/backbone.nativeview.js#L125 to enable useCapture

elementAddEventListener.call(this.el, eventName, handler, true);

Any objections to a PR for this?

akre54 commented 8 years ago

This would be a breaking change. Do you see any downsides?

akre54 commented 8 years ago

We could also specifically enable useCapture only for change and submit events which might solve this.

ahumphreys87 commented 8 years ago

I haven't noticed any but haven't done any perf tests. Maybe performance would be effected slightly?

Not sure if it would be breaking. The API would still work as it is to the user, would just fix this bug

ahumphreys87 commented 8 years ago

What about other input specific events? Blur and input seem to be effected also

akre54 commented 8 years ago

The problem with useCapture is that it would break users' existing stopPropagation calls. The events would be out of order which is potentially a big deal. We definitely can't enable it by default.

Is it obvious how jQuery does it?

ahumphreys87 commented 8 years ago

Hmm let me investigate that and I'll post back :)

Thanks btw, I'm build a Marionette.Native plugin. This library and Backbone.Fetch are the main parts of it, nice work!

On Friday, August 26, 2016, Adam Krebs notifications@github.com wrote:

The problem with useCapture is that it would break users' existing stopPropagation calls. The events would be out of order which is potentially a big deal. We definitely can't enable it by default.

Is it obvious how jQuery does it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/akre54/Backbone.NativeView/issues/26#issuecomment-242854147, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZOuCzH-OrXCH6Lyb6OIF8sLrk7wpnrks5qj1f6gaJpZM4JthKr .

ahumphreys87 commented 8 years ago

So looking at the jquery source, they seem to attach the events to each element individually rather than the document: https://github.com/jquery/jquery/blob/76c5237cc408c01ae67f5c68d39ed6f1e4ef8003/src/event.js#L92-L94

This would probably be quite a large piece of work for this library as we'd need to manage unbinding as well. What do you think @akre54 ?

akre54 commented 8 years ago

Right. (sorry I was thinking of jQuery's delegate, or the React event system which *does handle event bubbling). This may be a 'known issue' if we can't work around it without writing a huge layer on top of native events. Any ideas?

* Also the 4-arg form of $.fn.on

akre54 commented 8 years ago

I'm split on this. I think our two options are to whitelist events that don't bubble (blur, change, etc) and special-case them with the useCapture flag, or acknowledge that these are special events and require you to bind the event handler yourself:

_.first(this.$('input')).addEventListener('change', () => {})

Both have their downsides, but also aren't horrific in the grand scheme of things.

ahumphreys87 commented 8 years ago

Yeah I agree, the $ way of searching for elems with the selector and attaching events to those elems rather than document seems like overkill here.

I'd be happy with a whitelist - I'm yet to see adverse effects using useCapture so this may well be a good compromise.

akre54 commented 8 years ago

Sure thing. Wanna open a pull with an implementation?

ahumphreys87 commented 8 years ago

Yeah sure, ill get on it tomorrow morning (UK based 😄)

cloakedninjas commented 7 years ago

Seen this?

https://github.com/samriley/Backbone.NativeView/commit/c0d8210471923243f19ac288fbc10bd354ae98de