Redundant calls in Table. #954
Conversation
…k` are executed multiple times.
…deselect all selected checkbox after click outside the table.
|
@mkucharz You're right. I skipped over the table-row onClick handler. The original intent was to not have handlers on both the row and the cells -- looks like I never went back to update the table-row. I suspect that the number of rows will be orders of magnitude smaller than the number of cells so there may not be much benefit to removing the onClick handler in table-row and calling the _handleRowClick from _handleCellClick. Thoughts? |
|
@jkruder In general I think the most useful and universal way to solve it is to combine those two calls - always send in addition cell name. You can always get it's number from |
|
@mkucharz I like the separation of the two events; your handlers are going to be doing one type of processing as opposed to containing logic to handle row events and then handle cell events. Personal preference. If we do stick with separate row/cell events I would like to remove the separate click handler on the table-row and just leverage the cell event. @hai-cea Any preference as to how we handle row vs cell events? |
|
@jkruder sounds good, I'm ok with your idea. |
|
@jkruder Doesn't the cell click event automatically bubble up to the row? It would seem that manually calling onRowClick inside onCellClick unnecessary because of event bubbling. |
|
@hai-cea I would remove the click handler from the row entirely. No point in registering a handler on the row if we have them on each cell. So I would modify this PR so that the click event on the table-row is removed and the click event on the cell is propagated to the row inside table-row. |
|
Sounds good 👍 |
|
@mkucharz Do you have a status update on this? |
|
Resolved in #1087 |
zannager
added
the
component: table
Those calls are redundant, with them
onRowSelectionandonCellClickare executed multiple times.