Initial hover_template function #6580
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
marimo/_plugins/ui/_impl/table.py
Outdated
| total_rows: Union[int, Literal["too_many"]] | ||
| cell_styles: Optional[CellStyles] = None | ||
| # Mapping of rowId -> columnName -> hover text (plain string) | ||
| cell_hover_texts: Optional[dict[RowId, dict[ColumnName, str]]] = None |
There was a problem hiding this comment.
maybe: Optional[dict[RowId, dict[ColumnName, Optional[str]]]] to allow for empty hover for cells
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new callable hover template feature for tables, providing fine-grained control over cell hover text as an alternative to the existing string-based row-level hover templates.
- Adds support for callable hover templates that compute plain-text hover titles for individual cells
- Extends table functionality to handle per-cell hover text computation and display
- Implements comprehensive test coverage for the new hover functionality
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
marimo/_plugins/ui/_impl/table.py |
Core implementation of callable hover templates with cell-level text computation |
frontend/src/plugins/impl/DataTablePlugin.tsx |
Frontend integration for cell hover texts with type definitions and data flow |
frontend/src/components/data-table/data-table.tsx |
Table component integration with new hover text feature |
frontend/src/components/data-table/renderers.tsx |
Cell renderer updates to display hover titles |
frontend/src/components/data-table/cell-hover-text/types.ts |
TypeScript type definitions for cell hover text functionality |
frontend/src/components/data-table/cell-hover-text/feature.ts |
React Table feature implementation for cell hover text |
tests/_plugins/ui/_impl/test_table.py |
Comprehensive test suite for the new hover functionality |
examples/ui/table.py |
Example demonstrating the new callable hover template usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if args.sort and (self._style_cell or self._hover_cell): | ||
| LOGGER.warning( | ||
| "Cell styling is not correctly applied when table rows are sorted" | ||
| "Cell styling/hover may not be correctly applied when table rows are sorted" | ||
| ) |
There was a problem hiding this comment.
[nitpick] The warning message combines both styling and hover functionality, but they are separate features. Consider logging separate, more specific warnings for each feature when applicable.
| @@ -0,0 +1,40 @@ | |||
| /* Copyright 2024 Marimo. All rights reserved. */ | |||
| "use no memo"; | |||
There was a problem hiding this comment.
The directive should be 'use no memo' but appears to have a spacing issue. It should be written as a proper directive comment.
| "use no memo"; | |
| // use no memo |
2 participants
📝 Summary
Provides an alternative to
hover_templatefor tables.The main gripe with the template is that is was for the entire row and that turned out to be not flexible enough.
🔍 Description of Changes
Similar to
style_cellwe accept a callable to construct the hover text for a cell.📋 Checklist