refactor: centralize grid cell event handlers#12138
Conversation
Replace the per-cell inline tooltip listeners and the per-column `keydown` binding with shared bound methods on the grid. Cells now delegate `keydown` to their column via `column.__onCellKeyDown`, so the selection column's `_onCellKeyDown` is renamed to `__onCellKeyDown`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
vursen
changed the title
|
/code-review |
vaadin-review-bot
left a comment
vaadin-review-bot
left a comment
There was a problem hiding this comment.
Reviewed the changes — left 1 comment.
| Finding | |
|---|---|
| ⚡ | Mouse listeners are now registered on touch devices, where the base code skipped them entirely. |
| this._hideTooltip(true); | ||
| }); | ||
| } | ||
| cell.addEventListener('mouseenter', this.__onCellMouseEnter); |
There was a problem hiding this comment.
⚡ Mouse listeners are now registered on touch devices, where the base code skipped them entirely.
In the base code, _createCell attached mouseenter/mouseleave/mousedown listeners only inside if (!isAndroid && !isIOS), so on Android and iOS no mouse listeners were attached at all. The refactor moved that guard from registration into each handler body (__onCellMouseEnter/Leave/Down), so _createCell now unconditionally calls cell.addEventListener three times per cell on every platform.
On touch devices this adds three registered listeners per cell that immediately return without doing anything. Behavior is unchanged, but the listeners are pure overhead. Keeping the if (!isAndroid && !isIOS) check at registration time — while still using the shared bound methods — avoids attaching dead listeners and lets each handler drop its now-redundant internal platform check.
vaadin-grid-mixin.js:405 · efficiency · confirmed
There was a problem hiding this comment.
It's intentional. It allows us to reuse these handlers.
4 participants
The grid attaches cell event listeners in different places and in different ways: the tooltip mouse listeners are created as new closures for every cell in
_createCell, while thekeydownlistener is bound per column and attached in two separate places in__initRow, but only for columns that define_onCellKeyDown.This moves all cell event handling to the grid itself:
__onCellMouseEnter,__onCellMouseLeave,__onCellMouseDown,__onCellKeyDown) that are attached once per cell in_createCell.keydown, the cell delegates to its column throughcolumn.__onCellKeyDown, so the per-column listener setup in__initRowis no longer needed. The selection column's_onCellKeyDownis renamed to__onCellKeyDownto match.This also prepares for rendering header and footer rows declaratively with Lit, where the same handler references can be used directly in the template.
Extracted from #12134
Part of #10789