feat(table): styled tooltip for mo.ui.table hover_template - #9780
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
1 issue found across 6 files
Architecture diagram
sequenceDiagram
participant User
participant Cell as Table Cell (td)
participant Hook as useTableHoverTooltip()
participant Content as computeCellTooltipContent()
participant Tooltip as HoverTooltip Component
participant Radix as Radix Tooltip System
participant Scroll as Scroll Container
Note over User,Scroll: Cell hover triggers tooltip flow
User->>Cell: mouseOver (buttons=0)
Cell->>Hook: handleCellMouseOver(event, cell)
Hook->>Content: computeCellTooltipContent(cell, hoverTemplate)
alt Cell-level hover title exists
Content-->>Hook: cellTitle string
else Row-level string template (e.g. "{{first}} {{last}}")
Content->>Content: applyHoverTemplate(template, visible cells)
Content-->>Hook: resolved string
else No template or title
Content-->>Hook: undefined
Hook-->>Cell: hideTooltip() → return early
end
Note over Hook: Start 400ms delay timer
Hook->>Hook: setTimeout(showFor, 400)
alt Mouse leaves before 400ms
User->>Cell: mouseLeave
Cell->>Hook: handleCellMouseLeave()
Hook->>Hook: clearTimeout + setTooltipState(null)
else Timer fires
Hook->>Hook: showFor(target, content)
Hook->>Hook: capture cell getBoundingClientRect()
Hook-->>Tooltip: { rect, content }
Tooltip->>Tooltip: Position invisible trigger at cell rect
Tooltip->>Radix: open=true (controlled)
Radix-->>User: Render styled tooltip content
alt Scroll event occurs
User->>Scroll: scroll
Scroll->>Hook: hideTooltip()
Hook->>Tooltip: onClose()
Tooltip->>Radix: open=false
end
end
Note over User,Radix: Keyboard focus also opens tooltip (parity)
User->>Cell: focus (Tab / click)
Cell->>Hook: handleCellFocus(event, cell)
Hook->>Content: computeCellTooltipContent(cell, hoverTemplate)
alt Content available
Hook->>Hook: showFor(target, content) (no delay)
Hook-->>Tooltip: { rect, content }
Tooltip->>Radix: open=true
end
User->>Cell: blur
Cell->>Hook: handleCellBlur()
Hook->>Tooltip: onClose()
Note over User,Radix: Drag selection suppresses tooltip
User->>Cell: mouseDown (buttons != 0)
Cell->>Hook: hideTooltip() immediately
Hook->>Tooltip: onClose()
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@kirangadhave I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Pull request overview
This PR replaces mo.ui.table’s native HTML title tooltips (from hover_template and per-cell hover text) with a single controlled Radix-based styled tooltip per table, improving visual consistency and avoiding per-cell tooltip component overhead.
Changes:
- Removes native
titleattributes from table rows/cells and wires hover/focus events into a shared hover-tooltip controller. - Adds a new hover-tooltip module that resolves tooltip content (cell-level overrides row template) and renders a single positioned Radix tooltip.
- Updates and adds tests to validate tooltip content resolution and that styled tooltip content appears on hover.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/data-table/renderers.tsx | Removes title= usage and integrates the new hover-tooltip hook + single tooltip instance. |
| frontend/src/components/data-table/hover-tooltip/use-table-hover-tooltip.ts | Implements tooltip state management, delay, and hide/show behavior for hover/focus. |
| frontend/src/components/data-table/hover-tooltip/hover-tooltip.tsx | Renders the single controlled Radix tooltip anchored to a fixed-position rect. |
| frontend/src/components/data-table/hover-tooltip/content.ts | Extracts template application and cell-vs-row tooltip content resolution. |
| frontend/src/components/data-table/hover-tooltip/tests/content.test.ts | Unit tests for template substitution and content precedence. |
| frontend/src/components/data-table/tests/data-table.test.tsx | Updates tests to assert styled tooltip rendering (instead of title attributes). |
There was a problem hiding this comment.
3 issues found across 7 files
Architecture diagram
sequenceDiagram
participant User as User
participant Table as DataTableBody
participant Hook as useTableHoverTooltip
participant Content as computeCellTooltipContent
participant Tooltip as HoverTooltip Component
participant Radix as Radix Tooltip
Note over User,Tooltip: NEW: Styled tooltip flow replaces native title attribute
User->>Table: hovers over cell
Table->>Hook: handleCellMouseOver(event, cell)
alt Mouse button pressed (drag)
Hook->>Hook: suppress (e.buttons !== 0)
else No mouse button
Hook->>Content: computeCellTooltipContent(cell, hoverTemplate)
alt Cell-level hover title exists
Content-->>Hook: cell.getHoverTitle() content
else Cell-level title absent, row template exists
Content->>Content: applyHoverTemplate(template, visible cells)
Content-->>Hook: resolved template string
else No content available
Content-->>Hook: undefined/empty
Hook->>Hook: hideTooltip() immediately
end
opt Content exists
Hook->>Hook: set 400ms timeout
Hook->>Tooltip: showFor(target rect, content)
end
end
Note over User,Tooltip: Keyboard focus parity (replaces native title behavior)
User->>Table: focuses cell (Tab)
Table->>Hook: handleCellFocus(event, cell)
Hook->>Content: computeCellTooltipContent(cell, hoverTemplate)
Content-->>Hook: content
Hook->>Tooltip: showFor(rect, content) (no delay)
User->>Table: blurs cell
Table->>Hook: handleCellBlur()
Hook->>Tooltip: hideTooltip()
Note over Tooltip,Radix: Single controlled tooltip repositioned to hovered cell
Tooltip->>Radix: renders TooltipRoot with state
Radix->>Tooltip: TooltipTrigger at cell's getBoundingClientRect()
Tooltip->>Radix: TooltipContent with resolved text
Radix-->>User: displays styled tooltip
Note over Table,Hook: Scroll/gesture suppression
User->>Table: scrolls table
Table->>Hook: scroll event listener
Hook->>Tooltip: hideTooltip()
User->>Table: mouseDown (range select)
Table->>Hook: hideTooltip() immediately
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Dismiss on window scroll (capture) and resize, not just the table's own container. Link the focused cell to the tooltip content via aria-describedby so screen readers announce it, restoring native title parity.
A focus-triggered tooltip could be overwritten by a stale mouse-hover timeout firing after the delay; clear the timer on focus.
Focus fires for click and drag-select too; track pointer state so only keyboard focus shows the tooltip, preserving the selection-drag suppression.
1bf2779 to
f65064d
Compare
Summary
Replaces the native HTML
titletooltip used bymo.ui.table'shover_templatewith marimo's existing radix styled tooltip, for both modes: the row-level string template ("{{first}} {{last}}") and the per-cell callable. Hover text is now consistent with the rest of the table UI and can support richer content in the future.Approach
A single controlled radix tooltip per table, not one per cell. A small hook tracks
{ rect, content }of the hovered cell (piggybacking the existing cellonMouseOver), and one positioned,pointer-events-nonetrigger is anchored to that rect. Cost is O(1) in tooltip components regardless of cell count, which avoids the per-cell wrapping perf concern.Changes
data-table/hover-tooltip/:content.ts(template resolution, cell-level wins over row-level),use-table-hover-tooltip.ts(hover state, 400ms delay, drag/scroll suppression, focus/blur parity),hover-tooltip.tsx(the single controlled tooltip).renderers.tsx: removed the two nativetitle=props and the inlineapplyHoverTemplate; wired the hook and render<HoverTooltip>once.Closes MO-5746