Handling events README does not have an example that handles the eventHandler AND takes a parameter

[Frazer]

https://github.com/facebook/react/blob/master/docs/docs/handling-events.md

There are example for eventHandlers that receive e (SyntheticEvents),
but no example of passing any other parameters to an eventHandler.

[gaearon]

You can't pass any other arguments, as React only passes the event.
Do you mean something like index inside a map()?
Perhaps we could add a section on events to "Loops and Conditions".

[Frazer]

I was thinking something like this

  openSlideshow(viewingWhat){
    this.props.toggleSlideView(viewingWhat);
  }

<button onClick={this.openSlideshow.bind(this, 'due')}>Due</button
<button onClick={this.openSlideshow.bind(this, 'all')}>All</button>

Where would the event be handled here?

[gaearon]

Oh, I see. Normally you'd create two functions for this, or use partial application with either bind() or arrow functions. This is more in the realm of JavaScript than a React guide but we should probably mention this in some way. Thanks for input.

[hedgerh]

I'm going to take a look at this. I also noticed that the Handling Events doc could use some formatting to make it easier to read. Things like headers indicating each topic that is discussed.

[hedgerh]

By the way, @Frazer, as an alternative to the extra param approach, you could use the name attribute if you're trying to identify the clicked element:

<button name='due' onClick={openSlideshow}>Due</button>
<button name='all' onClick={openSlideshow}>All</button>

function openSlideshow (e) {
  console.log(e.target.name)
}

[hedgerh]

Hey @lacker, I was taking a look at this issue, and had some suggestions for revisions of the "Event Handling" documentation. I just submitted a PR #8297 with some of them, but wanted to discuss the section about binding and ES6 class syntax with you.

Perhaps we could turn this issue into a general "Event Handling" changes issue?

Regarding the binding/ES6 class section, I feel it goes a little too deep into the whole context/binding/ES6 issue for an "Event Handling" document, covering stuff like property initializers and whatnot. I think it'd be better if there was a separate document deep diving into the ES6/bind issue that we could briefly reference in the "Event Handling" doc.

[lacker]

I think adding more content to the event-handling doc to account for common cases would be useful. It's not a very long document right now, just 140 lines, so structurally I don't think it's time to split it up into multiple documents. I wish it did not have to go so deep into binding, but I don't think you can really handle events using best practices without learning everything in that document, and the goal of that doc is to explain event-handling best practices. The annoying truth is that you do need to learn about binding to handle events well in React.

[Frazer]

Thanks for the suggestion @hedgerh .

Back to my original question:

<button onClick={this.openSlideshow.bind(this, 'due')}>Due</button
<button onClick={this.openSlideshow.bind(this, 'all')}>All</button>

  openSlideshow(viewingWhat){
    this.props.toggleSlideView(viewingWhat);
  }

If I wanted to handle the event in openSlideShow, it isn't clear how. Perhaps:

  openSlideshow(viewingWhat, event){
    console.log(viewingWhat);
    console.log(e.target.name)
  }

It's not clear where to put the event parameter.

[hedgerh]

I'm on mobile so I can't do code blocks but

If you used this.openSlideshow.bind(this, 'due') as your handler, the
second param would be the event.

I prefer to do this with my handler, which dan mentioned:

onClick={(e) => { this.openSlideshow(e, 'due') } and make e the first param
of the method

On Wed, Nov 16, 2016 at 3:33 PM Frazer Kirkman notifications@github.com
wrote:

Thanks for the suggestion @hedgerh https://github.com/hedgerh .

Back to my original question:

<button onClick={this.openSlideshow.bind(this, 'due')}>Due</button
<button onClick={this.openSlideshow.bind(this, 'all')}>All

openSlideshow(viewingWhat){
this.props.toggleSlideView(viewingWhat);
}

If I wanted to handle the event in openSlideShow, it isn't clear how.
Perhaps:

openSlideshow(viewingWhat, event){
console.log(viewingWhat);
console.log(e.target.name)
}

It's not clear where to put the event parameter.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#8065 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACaEeJS3SwQ4uCRRwYF9vfOFTzOQkeQQks5q-2iWgaJpZM4KeLYX
.

[hedgerh]

@Frazer, hey I'm home on my computer. Let me know if that answered your question, bc I'm not 100% sure I did.

[Frazer]

@hedgerh yes, that answered perfectly. I guess I think both of these examples should be in the documentation.

[hedgerh]

@Frazer, yeah I'll add them. I'm curious, what do you think about using the name attribute on the element instead of using an extra param to identify which button was clicked?

<button name='due' onClick={this.openSlideshow}>Due</button>
<button name='all' onClick={this.openSlideshow}>All</button>

openSlideshow (e) {
  this.props.toggleSlideView(e.target.name)
}

[lacker]

To me these seem like handy tricks rather than a necessary thing to learn. For example if you only had the knowledge in the docs, you could just make a shim function like

openSlideshow(what) {
  ...
}

openDueSlideshow() {
  this.openSlideshow('due');
}

openAllSlideshow() {
  this.openSlideshow('all');
}

// Then later in render:

<button onClick={this.openDueSlideshow}>Due</button>
<button onClick={this.openAllSlideshow}>All</button>

What's wrong with that? Well, it's a couple extra lines of code. But it totally works. So these suggestions are nice to know but pretty noncritical if you can't figure it out yourself with your knowledge of binding.

Basically we should only include an example for something when it's something people need to learn, either to be using "best practices" or just to get a feature to work at all. If it's just a small improvement then it's ok if we leave it out of our docs.

[hedgerh]

@lacker - Consider if, instead of having 2 static buttons, the buttons are created dynamically.

I think it's worth documenting in the Event Handling section. I've had to use this technique in the past, and I can think of a few use cases that require identifying which child was clicked/changed.

[lacker]

Ehhhhhh OK I guess that's fair. Does bind have any advantage over arrow functions here? I can't think of any and if it's all the same then I would rather nudge people away from bind.

[hedgerh]

I can't think of any, either. I think that's the way to go.

On Wed, Nov 16, 2016 at 6:27 PM Kevin Lacker notifications@github.com
wrote:

Ehhhhhh OK I guess that's fair. Does bind have any advantage over arrow
functions here? I can't think of any and if it's all the same then I would
rather nudge people away from bind.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8065 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACaEeNPkZDdseqGMNr_4ssjNBhMACLceks5q-5FdgaJpZM4KeLYX
.

[Frazer]

showing how to pass parameters to a function is essential to my way of thinking about functions.

Is it true that {(e) => {someFunc(e,x)} } creates a copy of someFunc each time? and .bind does not? Is that one advantage of .bind?

[hedgerh]

Hmm, I think it may actually be the opposite. The bind function executes on every render, returning your new someFunc with your this context bound and the first param applied. The arrow function isn't evaluated until the button is clicked, so I don't think there is any issue there. Can someone correct me if im wrong?

[lacker]

Both the bind and the arrow-function ways for binding a parameter in create a new function every time. The repetitive shim function does not. Basically any call to bind and every use of => creates a new function - the only way around that while still binding this to the callback is calling bind in the constructor.

Thinking about it more, there is not a crisp, clear pattern of "what is the right thing to do", and if we suggest a few different patterns without having a strong recommendation we are just muddling the issue. I think we should just punt and not advocate one particular way of doing this in our docs.

[hedgerh]

Hmm, but don't we cover a few different approaches for dealing with the binding issues when using ES6 classes?

[lacker]

Yeah, it's definitely a grey area. I feel like it's not really clarifying to add more examples of ways to bind in more-complex scenarios, though - at some point you just need to tell people, you need to learn what binding is to build more complicated things. I think we're at that point - if you're going past the few scenarios in the docs, you should just read up on binding and think for yourself.

[hedgerh]

Honestly, I'd be on board with just mentioning the arrow approach and calling it a day. We aren't necessarily saying that's the better way to do it, and if someone knows they can partially apply an extra param with the bind approach already, they shouldn't really be thrown through a loop by this.

We could even say, "if you need to pass an extra parameter to your event handler, one way you can do this..."

So we suggest a method, but aren't necessarily advocating for it

[bvaughn]

Thank you for filing this issue! 😄

The documentation and source code for reactjs.org now lives in a different repository: reactjs/reactjs.org. (For more info on why we made this move, see issue #11075.)

I already created an issue in the new repo that relates to this, so let's move the discussion there! 😄

reactjs/react.dev/issues/75