From 61d0543fac26e53faae5eb4c6fd00b77ddc7d151 Mon Sep 17 00:00:00 2001 From: Johan Forsberg Date: Sat, 8 Jun 2024 12:09:08 +0200 Subject: [PATCH 1/7] Account for scrollbar in DataTable page-up/down --- src/textual/widgets/_data_table.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index 824c280156..8097297a03 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -2476,7 +2476,9 @@ def action_page_down(self) -> None: """Move the cursor one page down.""" self._set_hover_cursor(False) if self.show_cursor and self.cursor_type in ("cell", "row"): - height = self.size.height - (self.header_height if self.show_header else 0) + height = self.scrollable_content_region.height - ( + self.header_height if self.show_header else 0 + ) # Determine how many rows constitutes a "page" offset = 0 @@ -2498,7 +2500,9 @@ def action_page_up(self) -> None: """Move the cursor one page up.""" self._set_hover_cursor(False) if self.show_cursor and self.cursor_type in ("cell", "row"): - height = self.size.height - (self.header_height if self.show_header else 0) + height = self.scrollable_content_region.height - ( + self.header_height if self.show_header else 0 + ) # Determine how many rows constitutes a "page" offset = 0 From c31f51cd8c754d6ee4409563c4dd06242c528dc5 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Mon, 10 Jun 2024 15:29:04 +0100 Subject: [PATCH 2/7] UX matches expectation for pageup/pagedown --- src/textual/widgets/_data_table.py | 38 +++++++++++++++++------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index 8097297a03..dd36fc4a62 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -755,7 +755,7 @@ def _y_offsets(self) -> list[tuple[RowKey, int]]: y-coordinate, we can index into this list to find which row that y-coordinate lands on, and the y-offset *within* that row. The length of the returned list is therefore the total height of all rows within the DataTable.""" - y_offsets = [] + y_offsets: list[tuple[RowKey, int]] = [] if self._update_count in self._offset_cache: y_offsets = self._offset_cache[self._update_count] else: @@ -1096,6 +1096,7 @@ def watch_cursor_coordinate( elif self.cursor_type == "column": self.refresh_column(old_coordinate.column) self._highlight_column(new_coordinate.column) + if self._require_update_dimensions: self.call_after_refresh(self._scroll_cursor_into_view) else: @@ -1107,6 +1108,7 @@ def move_cursor( row: int | None = None, column: int | None = None, animate: bool = False, + scroll: bool = True, ) -> None: """Move the cursor to the given position. @@ -1123,6 +1125,7 @@ def move_cursor( row: The new row to move the cursor to. column: The new column to move the cursor to. animate: Whether to animate the change of coordinates. + scroll: Scroll the cursor into view after moving. """ cursor_row, cursor_column = self.cursor_coordinate @@ -1138,10 +1141,11 @@ def move_cursor( # of rows then tried to immediately move the cursor. # We do this before setting `cursor_coordinate` because its watcher will also # schedule a call to `_scroll_cursor_into_view` without optionally animating. - if self._require_update_dimensions: - self.call_after_refresh(self._scroll_cursor_into_view, animate=animate) - else: - self._scroll_cursor_into_view(animate=animate) + if scroll: + if self._require_update_dimensions: + self.call_after_refresh(self._scroll_cursor_into_view, animate=animate) + else: + self._scroll_cursor_into_view(animate=animate) self.cursor_coordinate = destination @@ -2422,7 +2426,7 @@ def _scroll_cursor_into_view(self, animate: bool = False) -> None: else: region = self._get_cell_region(self.cursor_coordinate) - self.scroll_to_region(region, animate=animate, spacing=fixed_offset) + self.scroll_to_region(region, animate=animate, spacing=fixed_offset, force=True) def _set_hover_cursor(self, active: bool) -> None: """Set whether the hover cursor (the faint cursor you see when you @@ -2445,7 +2449,7 @@ def _set_hover_cursor(self, active: bool) -> None: async def _on_click(self, event: events.Click) -> None: self._set_hover_cursor(True) meta = event.style.meta - if not "row" in meta or not "column" in meta: + if "row" not in meta or "column" not in meta: return row_index = meta["row"] @@ -2483,16 +2487,16 @@ def action_page_down(self) -> None: # Determine how many rows constitutes a "page" offset = 0 rows_to_scroll = 0 - row_index, column_index = self.cursor_coordinate + row_index, _ = self.cursor_coordinate for ordered_row in self.ordered_rows[row_index:]: offset += ordered_row.height + rows_to_scroll += 1 if offset > height: break - rows_to_scroll += 1 - self.cursor_coordinate = Coordinate( - row_index + rows_to_scroll - 1, column_index - ) + target_row = row_index + rows_to_scroll - 1 + self.scroll_relative(y=height, animate=False, force=True) + self.move_cursor(row=target_row, scroll=False) else: super().action_page_down() @@ -2507,16 +2511,16 @@ def action_page_up(self) -> None: # Determine how many rows constitutes a "page" offset = 0 rows_to_scroll = 0 - row_index, column_index = self.cursor_coordinate + row_index, _ = self.cursor_coordinate for ordered_row in self.ordered_rows[: row_index + 1]: offset += ordered_row.height + rows_to_scroll += 1 if offset > height: break - rows_to_scroll += 1 - self.cursor_coordinate = Coordinate( - row_index - rows_to_scroll + 1, column_index - ) + target_row = row_index - rows_to_scroll + 1 + self.scroll_relative(y=-height, animate=False) + self.move_cursor(row=target_row, scroll=False) else: super().action_page_up() From 1f6ead2957a2be5f0380b6fc292304699131f53c Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Mon, 10 Jun 2024 16:12:38 +0100 Subject: [PATCH 3/7] Add extra data table bindings --- src/textual/widgets/_data_table.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index 824c280156..5f72791bc5 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -216,12 +216,14 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True): BINDINGS: ClassVar[list[BindingType]] = [ Binding("enter", "select_cursor", "Select", show=False), - Binding("up", "cursor_up", "Cursor Up", show=False), - Binding("down", "cursor_down", "Cursor Down", show=False), - Binding("right", "cursor_right", "Cursor Right", show=False), - Binding("left", "cursor_left", "Cursor Left", show=False), + Binding("up,k", "cursor_up", "Cursor Up", show=False), + Binding("down,j", "cursor_down", "Cursor Down", show=False), + Binding("right,l", "cursor_right", "Cursor Right", show=False), + Binding("left,h", "cursor_left", "Cursor Left", show=False), Binding("pageup", "page_up", "Page Up", show=False), Binding("pagedown", "page_down", "Page Down", show=False), + Binding("g,home", "scroll_home", "Home", show=False), + Binding("G,end", "scroll_end", "End", show=False), ] """ | Key(s) | Description | From b69f2ce246291c4b7307681d5274f659921a6b52 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Mon, 10 Jun 2024 17:20:58 +0100 Subject: [PATCH 4/7] Page left and right --- src/textual/containers.py | 4 ++ src/textual/widget.py | 12 ++++++ src/textual/widgets/_data_table.py | 60 ++++++++++++++++++++++++------ 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/textual/containers.py b/src/textual/containers.py index c8fe96194e..54b9749681 100644 --- a/src/textual/containers.py +++ b/src/textual/containers.py @@ -47,6 +47,8 @@ class ScrollableContainer(Widget, can_focus=True, inherit_bindings=False): Binding("end", "scroll_end", "Scroll End", show=False), Binding("pageup", "page_up", "Page Up", show=False), Binding("pagedown", "page_down", "Page Down", show=False), + Binding("ctrl+pageup", "page_left", "Page Left", show=False), + Binding("ctrl+pagedown", "page_right", "Page Right", show=False), ] """Keyboard bindings for scrollable containers. @@ -60,6 +62,8 @@ class ScrollableContainer(Widget, can_focus=True, inherit_bindings=False): | end | Scroll to the end position, if scrolling is available. | | pageup | Scroll up one page, if vertical scrolling is available. | | pagedown | Scroll down one page, if vertical scrolling is available. | + | ctrl+pageup | Scroll left one page, if horizontal scrolling is available. | + | ctrl+pagedown | Scroll right one page, if horizontal scrolling is available. | """ diff --git a/src/textual/widget.py b/src/textual/widget.py index fa50805fbf..b1321e378a 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -3890,6 +3890,18 @@ def action_page_up(self) -> None: self._clear_anchor() self.scroll_page_up() + def action_page_left(self) -> None: + if not self.allow_horizontal_scroll: + raise SkipAction() + self._clear_anchor() + self.scroll_page_left() + + def action_page_right(self) -> None: + if not self.allow_horizontal_scroll: + raise SkipAction() + self._clear_anchor() + self.scroll_page_right() + def notify( self, message: str, diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index 6075eb6b7a..9eb71ed37d 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -222,17 +222,25 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True): Binding("left,h", "cursor_left", "Cursor Left", show=False), Binding("pageup", "page_up", "Page Up", show=False), Binding("pagedown", "page_down", "Page Down", show=False), - Binding("g,home", "scroll_home", "Home", show=False), - Binding("G,end", "scroll_end", "End", show=False), + Binding("g", "scroll_top", "Top", show=False), + Binding("G", "scroll_bottom", "Bottom", show=False), + Binding("home", "scroll_home", "Home", show=False), + Binding("end", "scroll_end", "End", show=False), ] """ | Key(s) | Description | | :- | :- | | enter | Select cells under the cursor. | - | up | Move the cursor up. | - | down | Move the cursor down. | - | right | Move the cursor right. | - | left | Move the cursor left. | + | up,k | Move the cursor up. | + | down,j | Move the cursor down. | + | right,l | Move the cursor right. | + | left,h | Move the cursor left. | + | pageup | Move one page up. | + | pagedown | Move one page down. | + | g | Move to the top. | + | G | Move to the bottom. | + | home | Move to the home position (leftmost column). | + | end | Move to the end position (rightmost column). | """ COMPONENT_CLASSES: ClassVar[set[str]] = { @@ -2526,26 +2534,54 @@ def action_page_up(self) -> None: else: super().action_page_up() - def action_scroll_home(self) -> None: - """Scroll to the top of the data table.""" + def action_page_left(self) -> None: + """Move the cursor one page left.""" + self._set_hover_cursor(False) + super().scroll_page_left() + + def action_page_right(self) -> None: + """Move the cursor one page right.""" + self._set_hover_cursor(False) + super().scroll_page_right() + + def action_scroll_top(self) -> None: + """Move the cursor and scroll to the top.""" self._set_hover_cursor(False) cursor_type = self.cursor_type if self.show_cursor and (cursor_type == "cell" or cursor_type == "row"): - row_index, column_index = self.cursor_coordinate + _, column_index = self.cursor_coordinate self.cursor_coordinate = Coordinate(0, column_index) else: super().action_scroll_home() - def action_scroll_end(self) -> None: - """Scroll to the bottom of the data table.""" + def action_scroll_bottom(self) -> None: + """Move the cursor and scroll to the bottom.""" self._set_hover_cursor(False) cursor_type = self.cursor_type if self.show_cursor and (cursor_type == "cell" or cursor_type == "row"): - row_index, column_index = self.cursor_coordinate + _, column_index = self.cursor_coordinate self.cursor_coordinate = Coordinate(self.row_count - 1, column_index) else: super().action_scroll_end() + def action_scroll_home(self) -> None: + """Move the cursor and scroll to the leftmost column.""" + self._set_hover_cursor(False) + cursor_type = self.cursor_type + if self.show_cursor and (cursor_type == "cell" or cursor_type == "column"): + self.move_cursor(column=0) + else: + self.scroll_x = 0 + + def action_scroll_end(self) -> None: + """Move the cursor and scroll to the rightmost column.""" + self._set_hover_cursor(False) + cursor_type = self.cursor_type + if self.show_cursor and (cursor_type == "cell" or cursor_type == "column"): + self.move_cursor(column=len(self.columns) - 1) + else: + self.scroll_x = self.max_scroll_x + def action_cursor_up(self) -> None: self._set_hover_cursor(False) cursor_type = self.cursor_type From 96c3fceddeaad1315f027ed2c3ac5f2e07f8be28 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Tue, 11 Jun 2024 12:52:48 +0100 Subject: [PATCH 5/7] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74d9e0128b..774b370fed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added support for Kitty's key protocol https://github.com/Textualize/textual/pull/4631 +### Fixed + +- Fixed pageup/pagedown behavior in DataTable https://github.com/Textualize/textual/pull/4633 + ## [0.66.0] - 2024-06-08 ### Changed From 7991457f5568db8d692c566e74f93c91fdc242c2 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Tue, 11 Jun 2024 13:04:49 +0100 Subject: [PATCH 6/7] Updating a test for new behaviour --- tests/test_data_table.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_data_table.py b/tests/test_data_table.py index abd886f31d..1320d853f0 100644 --- a/tests/test_data_table.py +++ b/tests/test_data_table.py @@ -192,11 +192,11 @@ async def test_cursor_movement_with_home_pagedown_etc(show_header): await pilot.pause() assert table.cursor_coordinate == Coordinate(0, 1) - await pilot.press("end") + await pilot.press("home") await pilot.pause() - assert table.cursor_coordinate == Coordinate(2, 1) + assert table.cursor_coordinate == Coordinate(0, 0) - await pilot.press("home") + await pilot.press("end") await pilot.pause() assert table.cursor_coordinate == Coordinate(0, 1) From 7a64a2a7328cd71d466ae8384a937a1d3b6b6878 Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Tue, 11 Jun 2024 13:07:32 +0100 Subject: [PATCH 7/7] Update CHANGELOG --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 774b370fed..2be8613262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Added support for Kitty's key protocol https://github.com/Textualize/textual/pull/4631 +- `ctrl+pageup`/`ctrl+pagedown` will scroll page left/right in DataTable https://github.com/Textualize/textual/pull/4633 +- `g`/`G` will scroll to the top/bottom of the DataTable https://github.com/Textualize/textual/pull/4633 +- Added simple `hjkl` key bindings to move the cursor in DataTable https://github.com/Textualize/textual/pull/4633 + +### Changed + +- `home` and `end` now works horizontally instead of vertically in DataTable https://github.com/Textualize/textual/pull/4633 ### Fixed