[Table] Send event object after click, hover, hoverOut on cell. #3002
Conversation
|
@MaronMariusz That sounds like a good idea to propagate the |
| @@ -384,16 +384,16 @@ const TableBody = React.createClass({ | |||
|
|
|||
| _onCellClick(e, rowNumber, columnNumber) { | |||
| e.stopPropagation(); | |||
| if (this.props.onCellClick) this.props.onCellClick(rowNumber, this._getColumnId(columnNumber)); | |||
| if (this.props.onCellClick) this.props.onCellClick(rowNumber, this._getColumnId(columnNumber), e); | |||
There was a problem hiding this comment.
I think that it would be better to use event instead of e.
That would also be good to have the event as the first argument. However, using event as the first parameters would introduce a breaking change.
There was a problem hiding this comment.
Hi, thanks for reply.
event instead of e sounds better you are right, but what I have seen in this component author used just e as variable name so I wanted to keep with this idea.
Moreover I agree that moving this param on the first place can break something so that's why I pushed it as the last item ;)
Personally it helps me so much in order to make some event delegations :)
Cheers!
There was a problem hiding this comment.
what I have seen in this component author used just e as variable name so I wanted to keep with this idea.
I understand, we are slowing rolling out from e to event.
Moreover I agree that moving this param on the first place can break something so that's why I pushed it as the last item ;)
Don't do it in this PR. That would be a breaking change.
|
@MaronMariusz Could you take into account my comments? I think that we can merge this PR then. |
oliviertassinari
added
PR: needs revision
MaronMariusz
force-pushed
the
Table-onCellEventObject
branch
from
1efacee
to
2bed696
Compare
modified: src/table/table-body.jsx modified: src/table/table.jsx
MaronMariusz
force-pushed
the
Table-onCellEventObject
branch
from
2bed696
to
118efbe
Compare
|
Hi, I changed e to event and we can |
oliviertassinari
removed
the
PR: needs revision
|
@MaronMariusz That looks good, thanks! |
[Table] Send event object after click, hover, hoverOut on cell.
zannager
added
the
component: table
Since I couldn't find onClick event on my table I made something own. If you like it, merge it ;) Cheers!
(if you have some solution to make this PR useless let me know!)