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

Support declarative event filters #26

Merged
merged 1 commit into from Jul 3, 2019
Merged

Conversation

frenchy64
Copy link
Contributor

Adds extension lifecycle cljfx.ext.node/with-event-filter-props to declaratively specify event filters.

Fixes #2

@vlaaad
Copy link
Contributor

vlaaad commented Jul 3, 2019

Hey, that's a cool example!
I initially thought about having declarative event filters as a map from event type to handler or list of handlers, but since there is a catch-all "any" event, and you can check your event type in handler, this is quite an elegant solution!

I think it should not be an extra-props lifecycle, but instead a part of a core node props in cljfx.fx.node/props. 2 main use cases for extra-props lifecycles are:

  • adding props that I missed without having to patch cljfx itself when you are writing an application and just want to get stuff done
  • adding synthetic props which are not present on mutable objects as a "place", such as selection props: they are part of selection model, but we pretend it is a part of a list/table because it makes sense.

Event filter feels more like an essential part of a Node in that regard, that's why I think it should be a part of node props.

@frenchy64
Copy link
Contributor Author

Thanks for your review. I moved the functionality to the :event-filter prop of fx.node.

I initially used an extension lifecycle to make a small library based on the devtools example that "instrumented" your scene automatically by passing it a description of your root node. With this convenient Node prop, perhaps it's better exposing the functionality as an event filter "handler", ie. a library providing the MouseEvent logic in the example.

src/cljfx/fx/node.clj Outdated Show resolved Hide resolved
@frenchy64
Copy link
Contributor Author

Updated to vector style, and updated example use a pure[r] helper function like I suggested above.

@vlaaad
Copy link
Contributor

vlaaad commented Jul 3, 2019

Looks good, I'll try it at home one more time later today and hopefully will merge it!

@vlaaad vlaaad merged commit 0531874 into cljfx:master Jul 3, 2019
@vlaaad
Copy link
Contributor

vlaaad commented Jul 3, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

Missing event filters
2 participants