diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f4edcf95d..fb1ad4ed9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Changed + +- Breaking change: standard keyboard scrollable navigation bindings have been moved off `Widget` and onto a new base class for scrollable containers (see also below addition) https://github.com/Textualize/textual/issues/2332 +- `ScrollView` now inherits from `ScrollableContainer` rather than `Widget` https://github.com/Textualize/textual/issues/2332 +- Containers no longer inherit any bindings from `Widget` https://github.com/Textualize/textual/issues/2331 + +### Added + +- Added `ScrollableContainer`; a container class that binds the common navigation keys to scroll actions (see also above breaking change) https://github.com/Textualize/textual/issues/2332 + ### Fixed - Fixed dark mode toggles in a "child" screen not updating a "parent" screen https://github.com/Textualize/textual/issues/1999 diff --git a/src/textual/containers.py b/src/textual/containers.py index 9d6c35b7f6..54aa36d507 100644 --- a/src/textual/containers.py +++ b/src/textual/containers.py @@ -3,7 +3,11 @@ """ +from __future__ import annotations +from typing import ClassVar + +from .binding import Binding, BindingType from .widget import Widget @@ -19,7 +23,35 @@ class Container(Widget): """ -class Vertical(Widget): +class ScrollableContainer(Widget, inherit_bindings=False): + """Base container widget that binds navigation keys for scrolling.""" + + BINDINGS: ClassVar[list[BindingType]] = [ + Binding("up", "scroll_up", "Scroll Up", show=False), + Binding("down", "scroll_down", "Scroll Down", show=False), + Binding("left", "scroll_left", "Scroll Up", show=False), + Binding("right", "scroll_right", "Scroll Right", show=False), + Binding("home", "scroll_home", "Scroll Home", show=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), + ] + """Keyboard bindings for scrollable containers. + + | Key(s) | Description | + | :- | :- | + | up | Scroll up, if vertical scrolling is available. | + | down | Scroll down, if vertical scrolling is available. | + | left | Scroll left, if horizontal scrolling is available. | + | right | Scroll right, if horizontal scrolling is available. | + | home | Scroll to the home position, if scrolling is available. | + | 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. | + """ + + +class Vertical(Widget, inherit_bindings=False): """A container which arranges children vertically.""" DEFAULT_CSS = """ @@ -31,7 +63,7 @@ class Vertical(Widget): """ -class VerticalScroll(Widget, can_focus=True): +class VerticalScroll(ScrollableContainer, can_focus=True): """A container which arranges children vertically, with an automatic vertical scrollbar.""" DEFAULT_CSS = """ @@ -43,7 +75,7 @@ class VerticalScroll(Widget, can_focus=True): """ -class Horizontal(Widget): +class Horizontal(Widget, inherit_bindings=False): """A container which arranges children horizontally.""" DEFAULT_CSS = """ @@ -55,7 +87,7 @@ class Horizontal(Widget): """ -class HorizontalScroll(Widget, can_focus=True): +class HorizontalScroll(ScrollableContainer, can_focus=True): """A container which arranges children horizontally, with an automatic horizontal scrollbar.""" DEFAULT_CSS = """ @@ -67,7 +99,7 @@ class HorizontalScroll(Widget, can_focus=True): """ -class Center(Widget): +class Center(Widget, inherit_bindings=False): """A container which centers children horizontally.""" DEFAULT_CSS = """ @@ -79,7 +111,7 @@ class Center(Widget): """ -class Middle(Widget): +class Middle(Widget, inherit_bindings=False): """A container which aligns children vertically in the middle.""" DEFAULT_CSS = """ @@ -91,7 +123,7 @@ class Middle(Widget): """ -class Grid(Widget): +class Grid(Widget, inherit_bindings=False): """A container with grid alignment.""" DEFAULT_CSS = """ @@ -102,7 +134,7 @@ class Grid(Widget): """ -class Content(Widget, can_focus=True, can_focus_children=False): +class Content(Widget, can_focus=True, can_focus_children=False, inherit_bindings=False): """A container for content such as text.""" DEFAULT_CSS = """ diff --git a/src/textual/demo.py b/src/textual/demo.py index 3cb55b0273..750ffe079b 100644 --- a/src/textual/demo.py +++ b/src/textual/demo.py @@ -14,7 +14,7 @@ from textual.app import App, ComposeResult from textual.binding import Binding -from textual.containers import Container, Horizontal +from textual.containers import Container, Horizontal, ScrollableContainer from textual.reactive import reactive from textual.widgets import ( Button, @@ -189,7 +189,7 @@ """ -class Body(Container): +class Body(ScrollableContainer): pass diff --git a/src/textual/scroll_view.py b/src/textual/scroll_view.py index e791471e53..46dd934cb4 100644 --- a/src/textual/scroll_view.py +++ b/src/textual/scroll_view.py @@ -7,13 +7,13 @@ from rich.console import RenderableType +from .containers import ScrollableContainer from .geometry import Size -from .widget import Widget -class ScrollView(Widget): +class ScrollView(ScrollableContainer): """ - A base class for a Widget that handles it's own scrolling (i.e. doesn't rely + A base class for a Widget that handles its own scrolling (i.e. doesn't rely on the compositor to render children). """ diff --git a/src/textual/widget.py b/src/textual/widget.py index 501de9b600..bfc242f910 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -54,7 +54,6 @@ from ._styles_cache import StylesCache from .actions import SkipAction from .await_remove import AwaitRemove -from .binding import Binding from .box_model import BoxModel from .css.query import NoMatches, WrongType from .css.scalar import ScalarOffset @@ -237,17 +236,6 @@ class Widget(DOMNode): """ - BINDINGS = [ - Binding("up", "scroll_up", "Scroll Up", show=False), - Binding("down", "scroll_down", "Scroll Down", show=False), - Binding("left", "scroll_left", "Scroll Up", show=False), - Binding("right", "scroll_right", "Scroll Right", show=False), - Binding("home", "scroll_home", "Scroll Home", show=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), - ] - DEFAULT_CSS = """ Widget{ scrollbar-background: $panel-darken-1; diff --git a/tests/test_binding_inheritance.py b/tests/test_binding_inheritance.py index e5545dc3cb..6e4c59b0f0 100644 --- a/tests/test_binding_inheritance.py +++ b/tests/test_binding_inheritance.py @@ -47,9 +47,8 @@ async def test_just_app_no_bindings() -> None: # An application with a single alpha binding. # # Sticking with just an app and the default screen: this configuration has a -# BINDINGS on the app itself, and simply binds the letter a -- in other -# words avoiding anything to do with movement keys. The result should be -# that we see the letter a, ctrl+c, and nothing else. +# BINDINGS on the app itself, and simply binds the letter a. The result +# should be that we see the letter a, ctrl+c, and nothing else. class AlphaBinding(App[None]): @@ -114,22 +113,13 @@ def on_mount(self) -> None: async def test_app_screen_with_bindings() -> None: """Test a screen with a single key binding defined.""" async with AppWithScreenThatHasABinding().run_test() as pilot: - # The screen will contain all of the movement keys, because it - # inherits from Widget. That's fine. Let's check they're there, but - # also let's check that they all have a non-priority binding. - assert all( - pilot.app.screen._bindings.get_key(key).priority is False - for key in MOVEMENT_KEYS - ) - # Let's also check that the 'a' key is there, and it *is* a priority - # binding. assert pilot.app.screen._bindings.get_key("a").priority is True ############################################################################## # A non-default screen with a single low-priority alpha key binding. # -# As above, but because Screen sets akk keys as high priority by default, we +# As above, but because Screen sets all keys as high priority by default, we # want to be sure that if we set our keys in our subclass as low priority as # default, they come through as such. @@ -152,13 +142,7 @@ def on_mount(self) -> None: async def test_app_screen_with_low_bindings() -> None: """Test a screen with a single low-priority key binding defined.""" async with AppWithScreenThatHasALowBinding().run_test() as pilot: - # Screens inherit from Widget which means they get movement keys - # too, so let's ensure they're all in there, along with our own key, - # and that everyone is low-priority. - assert all( - pilot.app.screen._bindings.get_key(key).priority is False - for key in ["a", *MOVEMENT_KEYS] - ) + assert pilot.app.screen._bindings.get_key("a").priority is False ##############################################################################