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

Always include node in event callbacks #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielma
Copy link

This is a big change, but I think it would make development a lot more
pleasant.

Similar to working with React in a browser context, there are a lot of
reasons that having a reference to a Node is useful.

One of the most basic React DOM cases is text inputs. You create the
input, and use event.target.value to set the state of your
component.

In react-blessed, having access to nodes in events would be useful,
but they're largely not available (except focus and blur events).

Because of this, I end up needing to create refs for almost every
element, which isn't necessarily bad, but my preference would be to
just have access to the node in event callbacks.

Please let me know what you think of this change!

This is a big change, but I think it would make development a lot more
pleasant.

Similar to working with React in a browser context, there are a lot of
reasons that having a reference to a `Node` is useful.

One of the most basic React DOM cases is text inputs. You create the
input, and use `event.target.value` to set the state of your
component.

In react-blessed, having access to nodes in events would be useful,
but they're largely not available (except `focus` and `blur` events).

Because of this, I end up needing to create refs for almost every
element, which isn't _necessarily_ bad, but my preference would be to
just have access to the node in event callbacks.
@Yomguithereal
Copy link
Owner

Hello @danielma. Thanks for your PR. I need to check the event emitting code again though. I don't remember what was chosen and how it worked. I guess we should give access to the blessed event data but I don't remember what blessed yield as event.

@danielma
Copy link
Author

As far as I can tell from looking at blessed, this would give us full access to everything from blessed. Every blessed Node extends from EventEmitter, which recommends using this to get access to the emitter instance.

@Yomguithereal
Copy link
Owner

Yomguithereal commented Dec 21, 2017

So there's no way in blessed to access current node in an event handler without needing to hit this? The payload never provides it?

@danielma
Copy link
Author

@Yomguithereal yep, that's correct as far as I'm aware

@geolessel
Copy link

👍 I'd like to see this happen as well.

@Yomguithereal
Copy link
Owner

Can someone checks if blessed evolved regarding this matter? I did not track its most recent updates.

@geolessel
Copy link

geolessel commented Jul 23, 2018

@Yomguithereal blessed hasn't been updated in over 3 years.

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

Successfully merging this pull request may close these issues.

3 participants