Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InputEvents not binding correctly #26

Open
ahumphreys87 opened this issue Aug 25, 2016 · 14 comments
Open

InputEvents not binding correctly #26

ahumphreys87 opened this issue Aug 25, 2016 · 14 comments
Labels

Comments

@ahumphreys87
Copy link

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
Copy link
Author

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
Copy link
Owner

akre54 commented Aug 26, 2016

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

@akre54 akre54 added the change label Aug 26, 2016
@akre54
Copy link
Owner

akre54 commented Aug 26, 2016

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

@ahumphreys87
Copy link
Author

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
Copy link
Author

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

@akre54
Copy link
Owner

akre54 commented Aug 26, 2016

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
Copy link
Author

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
#26 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACZOuCzH-OrXCH6Lyb6OIF8sLrk7wpnrks5qj1f6gaJpZM4JthKr
.

@ahumphreys87
Copy link
Author

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
Copy link
Owner

akre54 commented Aug 31, 2016

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
Copy link
Owner

akre54 commented Sep 12, 2016

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
Copy link
Author

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
Copy link
Owner

akre54 commented Sep 12, 2016

Sure thing. Wanna open a pull with an implementation?

@ahumphreys87
Copy link
Author

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

@cloakedninjas
Copy link
Contributor

Seen this?

samriley@c0d8210

@cloakedninjas cloakedninjas mentioned this issue Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants