ckirkendall / enfocus

DOM manipulation and templating library for ClojureScript inspired by Enlive.
http://ckirkendall.github.com/enfocus-site/
370 stars 41 forks source link

listen-live looses event information #87

Closed twillis closed 10 years ago

twillis commented 10 years ago

So, while playing around with enfocus this afternoon I came across a little weirdness with listen-live, which I will do my best to describe via example.

the callback for listen :keypress will get an event of type goog.events.BrowserEvent

whereas the callback for listen-live :keypress will get an event of type goog.events.Event

If i'm following the code correctly, this function is the culprit

https://github.com/ckirkendall/enfocus/blob/master/src/cljs/enfocus/events.cljs#L87

So for a dynamically added element you can't get the keyCode etc...

It seems like there is an init method that you can pass another target to and get the same effect as that function at least for browser events and all those that inherit from it.

http://docs.closure-library.googlecode.com/git/class_goog_events_BrowserEvent.html

I'm not cool enough at clojuring to do a pull request, but maybe this will help at the moment.

twillis commented 10 years ago

ok, well I tried, but I'm not able to run lein test on the project. alas, here's a patch that describes what I was thinking.

--- a/src/cljs/enfocus/events.cljs
+++ b/src/cljs/enfocus/events.cljs
@@ -85,7 +85,7 @@
     (conj (get-node-chain top (.-parentNode node)) node)))

 (defn- create-event [type cur tar]
-  (let [event (goog.events.Event. type)]
+  (let [event (or (.init type tar) (goog.events.Event. type))]
     (set! (.-currentTarget event) cur)
     (set! (.-target event) tar)
     event))
ckirkendall commented 10 years ago

You need to have phantom JavaScript engine installed to run the tests. Unfortunately, I am still concerting the event test over to new structure. I think using goog.object to clone the original event and then set the current target might be our best option. On Dec 31, 2013 7:53 PM, "Tom Willis" notifications@github.com wrote:

ok, well I tried, but I'm not able to run lein test on the project. alas, here's a patch that describes what I was thinking.

--- a/src/cljs/enfocus/events.cljs+++ b/src/cljs/enfocus/events.cljs@@ -85,7 +85,7 @@ (conj (get-node-chain top (.-parentNode node)) node)))

(defn- create-event [type cur tar]- (let [event (goog.events.Event. type)]+ (let [event (or (.init type tar) (goog.events.Event. type))](set! %28.-currentTarget event%29 cur) (set! (.-target event) tar) event))

— Reply to this email directly or view it on GitHubhttps://github.com/ckirkendall/enfocus/issues/87#issuecomment-31416460 .

ckirkendall commented 10 years ago

I did some testing and ended up cloning the original event and setting the currentTarget to solve the issue.

ckirkendall commented 10 years ago

If you could can you pull down the newest 2.1.0-SNAPSHOT and validate it solves your issue.

twillis commented 10 years ago

sure, i'll do that this evening, about to go get my hobbit on now :)

twillis commented 10 years ago

just gave it a go and it works as expected. thanks so much for the change over the holidays.