Skip to content

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