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

EventListener is limiting #55

Closed
slimsag opened this issue Jul 9, 2016 · 4 comments
Closed

EventListener is limiting #55

slimsag opened this issue Jul 9, 2016 · 4 comments

Comments

@slimsag
Copy link
Member

slimsag commented Jul 9, 2016

I've noticed when working on a project that EventListener can be limiting in regard to the fact that we don't expose the raw *js.Object event OR similiar functionality.

For example, in one scenario I wanted to call preventDefault on the event object within the event handler, i.e. dependent upon a condition within my event handler. Particularly, if a specific key was pressed I would call preventDefault but if it wasn't a specific key then I would retain the default behavior by not calling preventDefault. Right now, however, it is only possible to call PreventDefault as a blanket-statement for the entire event handler -- not conditionally -- because we don't expose the raw event nor such a method to do it conditionally (it can only be done on the EventHandler object, which the callback has no access to).

@dave
Copy link
Contributor

dave commented Jul 14, 2016

Seconded... Exposing the raw event object would make things more flexible... Exposing these as methods on the handler is neat, but it always struck me as a bit weird. In fact, the first time I needed to use preventDefault, I spent quite a while scratching my head and referring to examples before I worked it out. It's a lot more familiar to call it within the event handler, and not being able to call it conditionally is an important omission.

p.s. We recently fixed this to expose stopPropagation in the same way as preventDefault.

@neelance
Copy link
Contributor

The initial idea for that preventDefault implementation was to be able to automatically call the event listener in a new goroutine to allow blocking code by adding a go statement here: https://github.com/gopherjs/vecty/blob/master/dom.go#L97. This may not be the best solution.

Keep in mind that React still sees a reason for adding an abstraction on the native event: https://facebook.github.io/react/docs/events.html This might be especially necessary when we add event pooling for performance.

@slimsag
Copy link
Member Author

slimsag commented Nov 12, 2016

From @davelondon

In order to do a drag+drop file upload, you need access to dataTransfer from the js event. The js native event object isn't currently presented to the vecty event handler. I've added this in my fork as an anonymous struct member, and it seems to feel good in use:

https://github.com/davelondon/uploader/blob/master/editor.go#L133

I seem to remember this was discussed before?

From @shurcooL

need access to dataTransfer from the js event. The js native event object isn't currently presented to the vecty event handler.

Just asking, but have you considered simply filling that gap and exposing dataTransfer natively?

From @slimsag

I think this is a duplicate or related to #55

I agree it is limiting, and the best choice for now is probably to expose the native event object. Although in #55 neelance does make some good points about event pooling in the future.. I'm not sure yet.

@shurcooL there are so many DOM event properties I think it would be a bit crazy to add them all :P

@slimsag
Copy link
Member Author

slimsag commented Nov 12, 2016

From React's event documentation it seems like the main reason they use synthetic events is to standardize how events act across browsers. That actually feels a bit strange to do IMO, but I do not know all the details I suppose.

They too, also expose the native event:

If you find that you need the underlying browser event for some reason, simply use the nativeEvent attribute to get it.

And event pooling appears to just be for purposes of not creating the same JS object over and over again (something that could be achieved by simply using the event object the browser gives us).

I think they might do more behind the scenes to improve event performance, but I think for now it is OK for us to expose native event objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants