Conversation
There was a problem hiding this comment.
Pull request overview
Extends DataTable row-cursor rendering to fill any extra horizontal space and changes click-selection so that “selected” events are only emitted when clicking an already-highlighted cell/row/column.
Changes:
- Extend row cursor highlight to the full widget width by styling the “remaining space” area and widening row refresh regions.
- Change click behavior so selection events only fire on a click that re-clicks the current highlight.
- Update tests, snapshots, changelog, and bump version to 7.5.0.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/textual/widgets/_data_table.py |
Extends row cursor styling into remaining space; adjusts click-selection behavior; updates hover handling. |
tests/test_data_table.py |
Updates click tests to require a second click for selection; adds focus config. |
tests/snapshot_tests/__snapshots__/test_snapshots/test_datatable_row_cursor_render.svg |
Snapshot update reflecting new full-width row cursor rendering and style IDs. |
tests/snapshot_tests/__snapshots__/test_snapshots/test_datatable_hot_reloading.svg |
Snapshot update reflecting new rendering/style IDs. |
pyproject.toml |
Version bump to 7.5.0. |
CHANGELOG.md |
Adds 7.5.0 entry documenting the cursor extension and click-selection change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| table.add_row("123") | ||
| table.add_row("456") | ||
| await pilot.click(offset=Offset(1, 2)) | ||
| await pilot.click(offset=Offset(1, 2)) |
There was a problem hiding this comment.
The test description says clicking emits both ColumnHighlighted and ColumnSelected, but the test now requires a second click to emit ColumnSelected. Please update the docstring/comment to reflect the new behavior.
| extend_style += Style.from_meta( | ||
| {"row": row_index, "column": 0, "out_of_bounds": True} | ||
| ) |
There was a problem hiding this comment.
The padding segment for the “extended” area is tagged with meta {"row": row_index, "column": 0, "out_of_bounds": True}. Because _on_click only checks for presence of row/column, clicking in the extended whitespace will be interpreted as a click on column 0 (and on the header row will incorrectly fire HeaderSelected for column 0). Consider not emitting row/column meta for out-of-bounds padding, or set a sentinel and explicitly handle it in the click logic (e.g., treat it as a row-click when cursor_type == "row", otherwise ignore).
| extend_style += Style.from_meta( | |
| {"row": row_index, "column": 0, "out_of_bounds": True} | |
| ) | |
| extend_style += Style.from_meta({"out_of_bounds": True}) |
| ) | ||
| remaining_space = max(0, widget_width - table_width) | ||
| background_color = self.background_colors[1] |
There was a problem hiding this comment.
remaining_space is computed from widget_width - table_width, but table_width currently doesn’t include the fixed column widths (only columns from fixed_columns: plus the row-label width). When fixed_columns > 0 this overestimates padding and relies on later trimming, which adds unnecessary work per rendered line. Consider basing the calculation on the full table width (row labels + all columns) or otherwise accounting for fixed columns.
| self.post_message(message) | ||
| elif self.show_cursor and self.cursor_type != "none": | ||
| # Only post selection events if there is a visible row/col/cell cursor. | ||
| self.cursor_coordinate = Coordinate(row_index, column_index) | ||
| self._post_selected_message() | ||
| new_coordinate = Coordinate(row_index, column_index) | ||
| highlight_click = new_coordinate == self.cursor_coordinate | ||
| self.cursor_coordinate = new_coordinate | ||
| if highlight_click: | ||
| self._post_selected_message() |
There was a problem hiding this comment.
The new click-selection logic doesn’t account for the out_of_bounds padding segments added during rendering. Since those segments include row/column meta, clicks in the extended whitespace can be misinterpreted as clicks on column 0 / header column 0. Add an out_of_bounds check here and decide whether to ignore the click or map it appropriately (e.g., treat it as a row click when cursor_type == "row").
|
|
||
| await pilot.click(offset=Offset(1, 2)) | ||
| assert app.message_names == ["RowHighlighted", "RowHighlighted", "RowSelected"] | ||
| print(repr(app.message_names)) |
There was a problem hiding this comment.
The debug print(repr(app.message_names)) will pollute test output and can cause noisy CI logs. Please remove it (or replace with an assertion/message in the failing assert if you need diagnostics).
| print(repr(app.message_names)) |
| table.add_row("123") | ||
| row_key = table.add_row("456") | ||
| await pilot.click(offset=Offset(1, 2)) | ||
| await pilot.click(offset=Offset(1, 2)) |
There was a problem hiding this comment.
This test’s docstring/commentary says a single click emits both highlight + select, but the behavior under test now requires a second click on the already-highlighted cell to emit CellSelected. Please update the docstring/comment to match the new interaction.
| table.add_row("123") | ||
| row_key = table.add_row("456") | ||
|
|
||
| await pilot.click(offset=Offset(1, 2)) |
There was a problem hiding this comment.
The test description says clicking emits both RowHighlighted and RowSelected, but the test now asserts selection only on a second click (after the row is already highlighted). Please update the docstring/comment to reflect the new behavior.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Extend the row cursor to the full width.