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 for passing event objects thru to various event handlers #605

Merged
merged 1 commit into from
Mar 4, 2017

Conversation

paulbrom
Copy link
Contributor

@paulbrom paulbrom commented Mar 3, 2017

So in addition to the change we made to support horizontal scrolling, we also added support for passing event objects thru the various event handlers, particularly in defaultCellRenderer. Yes, we can (and did) build our own cellRenderer to accomodate passing these event objects thru (which we needed to check the CTRL and ALT key state for some behaviors we were looking at add), but it seemed to me that it would be nice if the default renderer passed these thru.

@bvaughn
Copy link
Owner

bvaughn commented Mar 3, 2017

I don't object to passing through the event object as a new named parameter, but I do object to the name chosen. (I don't like abbreviated names. They're too arbitrary.) If you'll rename the parameter to event - we're good to go.

I'd also love it if you updated any relevant documentation to list the new parameter. 😄

@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2017

FYI I'm going to make my own edit and merge this. Thanks~

@bvaughn bvaughn merged commit 5c96dd5 into bvaughn:master Mar 4, 2017
}

const onKeyDown = (event) => {
if (event.key === 'Enter' || event.key === ' ') {
onClick()
onClick(event)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weird- though I understand why you did it. I guess it's okay since people can ignore the event or check its type

@bvaughn
Copy link
Owner

bvaughn commented Mar 4, 2017

Merged!

@paulbrom
Copy link
Contributor Author

paulbrom commented Mar 6, 2017

Thanks Brian!

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.

2 participants