From 8273f78c099e95204fd75d7eb43af103bd15489f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 1 Apr 2023 18:21:07 +0100 Subject: [PATCH 1/8] Fix issue with modals --- CHANGELOG.md | 6 +++++- src/textual/app.py | 40 ++++++++++++++++++---------------------- src/textual/dom.py | 5 +++++ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65364b1d21..4987a8491c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed bindings persistance https://github.com/Textualize/textual/issues/1613 -- The `Markdown` widget now auto-increments ordered lists https://github.com/Textualize/textual/issues/2002 +- Fixed modal bindings https://github.com/Textualize/textual/issues/2194 + +### Changed + +- - The `Markdown` widget now auto-increments ordered lists https://github.com/Textualize/textual/issues/2002 ## [0.17.1] - 2023-03-30 diff --git a/src/textual/app.py b/src/textual/app.py index 17c75ffb57..2d13a0f625 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -1961,38 +1961,34 @@ def bell(self) -> None: @property def _binding_chain(self) -> list[tuple[DOMNode, Bindings]]: """Get a chain of nodes and bindings to consider. + If no widget is focused, returns the bindings from both the screen and the app level bindings. Otherwise, combines all the bindings from the currently focused node up the DOM to the root App. - - Returns: - List of DOM nodes and their bindings. """ focused = self.focused namespace_bindings: list[tuple[DOMNode, Bindings]] - screen = self.screen if focused is None: - if screen.is_modal: - namespace_bindings = [ - (self.screen, self.screen._bindings), - ] - else: - namespace_bindings = [ - (self.screen, self.screen._bindings), - (self, self._bindings), - ] + namespace_bindings = [ + (self.screen, self.screen._bindings), + (self, self._bindings), + ] else: - if screen.is_modal: - namespace_bindings = [ - (node, node._bindings) for node in focused.ancestors - ] - else: - namespace_bindings = [ - (node, node._bindings) for node in focused.ancestors_with_self - ] + namespace_bindings = [ + (node, node._bindings) for node in focused.ancestors_with_self + ] return namespace_bindings + @property + def _modal_binding_chain(self) -> list[tuple[DOMNode, Bindings]]: + """The binding chain, ignoring everything before the last modal.""" + binding_chain = self._binding_chain + for index, (node, _bindings) in enumerate(binding_chain): + if node.is_modal: + return binding_chain[:index] + return binding_chain + async def check_bindings(self, key: str, priority: bool = False) -> bool: """Handle a key press. @@ -2004,7 +2000,7 @@ async def check_bindings(self, key: str, priority: bool = False) -> bool: True if the key was handled by a binding, otherwise False """ for namespace, bindings in ( - reversed(self._binding_chain) if priority else self._binding_chain + reversed(self._binding_chain) if priority else self._modal_binding_chain ): binding = bindings.keys.get(key) if binding is not None and binding.priority == priority: diff --git a/src/textual/dom.py b/src/textual/dom.py index 7eca75d885..04309bb6fa 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -208,6 +208,11 @@ def auto_refresh(self, interval: float | None) -> None: ) self._auto_refresh = interval + @property + def is_modal(self) -> bool: + """Is the node a modal?""" + return False + def _automatic_refresh(self) -> None: """Perform an automatic refresh (set with auto_refresh property).""" self.refresh() From 241d6c989d97eb355b44440b1967fa22dab5c906 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 1 Apr 2023 18:25:39 +0100 Subject: [PATCH 2/8] changelog --- CHANGELOG.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4987a8491c..7410d74603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,16 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## Unreleased +## [0.17.2] - Unreleased -### Fixed +### [Fixed] - Fixed bindings persistance https://github.com/Textualize/textual/issues/1613 +- The `Markdown` widget now auto-increments ordered lists https://github.com/Textualize/textual/issues/2002 - Fixed modal bindings https://github.com/Textualize/textual/issues/2194 -### Changed - -- - The `Markdown` widget now auto-increments ordered lists https://github.com/Textualize/textual/issues/2002 ## [0.17.1] - 2023-03-30 @@ -691,6 +689,9 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040 - New handler system for messages that doesn't require inheritance - Improved traceback handling +[0.17.2]: https://github.com/Textualize/textual/compare/v0.17.1...v0.17.2 +[0.17.1]: https://github.com/Textualize/textual/compare/v0.17.0...v0.17.1 +[0.17.0]: https://github.com/Textualize/textual/compare/v0.16.0...v0.17.0 [0.16.0]: https://github.com/Textualize/textual/compare/v0.15.1...v0.16.0 [0.15.1]: https://github.com/Textualize/textual/compare/v0.15.0...v0.15.1 [0.15.0]: https://github.com/Textualize/textual/compare/v0.14.0...v0.15.0 From b48178b80d4876e8641201a2b9d77a1f77e2d18b Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 1 Apr 2023 18:34:51 +0100 Subject: [PATCH 3/8] fix binding on button --- CHANGELOG.md | 2 +- src/textual/widgets/_button.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7410d74603..5d342bcd06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed bindings persistance https://github.com/Textualize/textual/issues/1613 - The `Markdown` widget now auto-increments ordered lists https://github.com/Textualize/textual/issues/2002 - Fixed modal bindings https://github.com/Textualize/textual/issues/2194 - +- Fix binding enter to active button https://github.com/Textualize/textual/issues/2194 ## [0.17.1] - 2023-03-30 diff --git a/src/textual/widgets/_button.py b/src/textual/widgets/_button.py index dfa1867398..6c8faa8546 100644 --- a/src/textual/widgets/_button.py +++ b/src/textual/widgets/_button.py @@ -7,6 +7,7 @@ from typing_extensions import Literal, Self from .. import events +from ..binding import Binding from ..css._error_tools import friendly_list from ..message import Message from ..reactive import reactive @@ -145,6 +146,8 @@ class Button(Static, can_focus=True): """ + BINDINGS = [Binding("enter", "press", "Press Button", show=False)] + ACTIVE_EFFECT_DURATION = 0.3 """When buttons are clicked they get the `-active` class for this duration (in seconds)""" @@ -252,10 +255,9 @@ def _start_active_affect(self) -> None: self.ACTIVE_EFFECT_DURATION, partial(self.remove_class, "-active") ) - async def _on_key(self, event: events.Key) -> None: - if event.key == "enter" and not self.disabled: - self._start_active_affect() - self.post_message(Button.Pressed(self)) + def action_press(self) -> None: + """Activate a press if""" + self.press() @classmethod def success( From 11c3790ebaead53f7d56994844db6e57d4a97fe6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 1 Apr 2023 18:48:03 +0100 Subject: [PATCH 4/8] binding tweak --- src/textual/app.py | 8 ++------ src/textual/screen.py | 6 ++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 2d13a0f625..104dd97695 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -279,11 +279,7 @@ class App(Generic[ReturnType], DOMNode): also the `sub_title` attribute. """ - BINDINGS = [ - Binding("ctrl+c", "quit", "Quit", show=False, priority=True), - Binding("tab", "focus_next", "Focus Next", show=False), - Binding("shift+tab", "focus_previous", "Focus Previous", show=False), - ] + BINDINGS = [Binding("ctrl+c", "quit", "Quit", show=False, priority=True)] title: Reactive[str] = Reactive("", compute=False) sub_title: Reactive[str] = Reactive("", compute=False) @@ -1984,7 +1980,7 @@ def _binding_chain(self) -> list[tuple[DOMNode, Bindings]]: def _modal_binding_chain(self) -> list[tuple[DOMNode, Bindings]]: """The binding chain, ignoring everything before the last modal.""" binding_chain = self._binding_chain - for index, (node, _bindings) in enumerate(binding_chain): + for index, (node, _bindings) in enumerate(binding_chain, 1): if node.is_modal: return binding_chain[:index] return binding_chain diff --git a/src/textual/screen.py b/src/textual/screen.py index cb769a4988..170c00e2d8 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -11,6 +11,7 @@ from ._compositor import Compositor, MapGeometry from ._context import visible_screen_stack from ._types import CallbackType +from .binding import Binding from .css.match import match from .css.parse import parse_selectors from .css.query import QueryType @@ -45,6 +46,11 @@ class Screen(Widget): stack_updates: Reactive[int] = Reactive(0, repaint=False) """An integer that updates when the screen is resumed.""" + BINDINGS = [ + Binding("tab", "focus_next", "Focus Next", show=False), + Binding("shift+tab", "focus_previous", "Focus Previous", show=False), + ] + def __init__( self, name: str | None = None, From efa4e9a129f20d119a20947e47dbf58e4821524b Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 1 Apr 2023 18:51:07 +0100 Subject: [PATCH 5/8] changelog --- CHANGELOG.md | 4 ++++ tests/test_binding_inheritance.py | 12 +++--------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d342bcd06..626e5608de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed modal bindings https://github.com/Textualize/textual/issues/2194 - Fix binding enter to active button https://github.com/Textualize/textual/issues/2194 +### [Changed] + +- tab and shift+tab are now defined on Screen. + ## [0.17.1] - 2023-03-30 ### Fixed diff --git a/tests/test_binding_inheritance.py b/tests/test_binding_inheritance.py index 5afb5e5311..e5545dc3cb 100644 --- a/tests/test_binding_inheritance.py +++ b/tests/test_binding_inheritance.py @@ -39,10 +39,8 @@ class NoBindings(App[None]): async def test_just_app_no_bindings() -> None: """An app with no bindings should have no bindings, other than ctrl+c.""" async with NoBindings().run_test() as pilot: - assert list(pilot.app._bindings.keys.keys()) == ["ctrl+c", "tab", "shift+tab"] + assert list(pilot.app._bindings.keys.keys()) == ["ctrl+c"] assert pilot.app._bindings.get_key("ctrl+c").priority is True - assert pilot.app._bindings.get_key("tab").priority is False - assert pilot.app._bindings.get_key("shift+tab").priority is False ############################################################################## @@ -63,9 +61,7 @@ class AlphaBinding(App[None]): async def test_just_app_alpha_binding() -> None: """An app with a single binding should have just the one binding.""" async with AlphaBinding().run_test() as pilot: - assert sorted(pilot.app._bindings.keys.keys()) == sorted( - ["ctrl+c", "tab", "shift+tab", "a"] - ) + assert sorted(pilot.app._bindings.keys.keys()) == sorted(["ctrl+c", "a"]) assert pilot.app._bindings.get_key("ctrl+c").priority is True assert pilot.app._bindings.get_key("a").priority is True @@ -87,9 +83,7 @@ class LowAlphaBinding(App[None]): async def test_just_app_low_priority_alpha_binding() -> None: """An app with a single low-priority binding should have just the one binding.""" async with LowAlphaBinding().run_test() as pilot: - assert sorted(pilot.app._bindings.keys.keys()) == sorted( - ["ctrl+c", "tab", "shift+tab", "a"] - ) + assert sorted(pilot.app._bindings.keys.keys()) == sorted(["ctrl+c", "a"]) assert pilot.app._bindings.get_key("ctrl+c").priority is True assert pilot.app._bindings.get_key("a").priority is False From 2ba48051e65d857abe54e78bfb814c4647dce7c9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Apr 2023 08:43:02 +0100 Subject: [PATCH 6/8] snapshots --- .../__snapshots__/test_snapshots.ambr | 322 ++++++++++++++++++ .../snapshot_apps/modal_screen_bindings.py | 34 ++ tests/snapshot_tests/test_snapshots.py | 17 + 3 files changed, 373 insertions(+) create mode 100644 tests/snapshot_tests/snapshot_apps/modal_screen_bindings.py diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 1ed1df73c8..52e1523c0b 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -16790,6 +16790,328 @@ ''' # --- +# name: test_modal_dialog_bindings + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ModalApp + + + + + + + + + + ⭘ModalApp + Hello + + + + + + + + + + + + + + + + + + + + + +  ⏎  Open Dialog  + + + + + ''' +# --- +# name: test_modal_dialog_bindings_input + ''' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ModalApp + + + + + + + + + + DialogModalApp + ▊▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▎ + ▊hi!▎ + ▊▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▎ + ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔ + OK + ▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ + + + + + + + + + + + + + + + + +  ⏎  Open Dialog  + + + + + ''' +# --- # name: test_multiple_css ''' diff --git a/tests/snapshot_tests/snapshot_apps/modal_screen_bindings.py b/tests/snapshot_tests/snapshot_apps/modal_screen_bindings.py new file mode 100644 index 0000000000..df6010d1ce --- /dev/null +++ b/tests/snapshot_tests/snapshot_apps/modal_screen_bindings.py @@ -0,0 +1,34 @@ +from textual.app import App, ComposeResult +from textual.screen import Screen, ModalScreen +from textual.widgets import Button, Footer, Header, Label, Input + + +class Dialog(ModalScreen): + def compose(self) -> ComposeResult: + yield Label("Dialog") + yield Input() + yield Button("OK", id="ok") + + def on_button_pressed(self, event: Button.Pressed) -> None: + if event.button.id == "ok": + self.app.pop_screen() + + def on_input_submitted(self, event: Input.Submitted) -> None: + self.app.pop_screen() # Never gets here + + +class ModalApp(App): + BINDINGS = [("enter", "open_dialog", "Open Dialog")] + + def compose(self) -> ComposeResult: + yield Header() + yield Label("Hello") + yield Footer() + + def action_open_dialog(self) -> None: + self.push_screen(Dialog()) + + +if __name__ == "__main__": + app = ModalApp() + app.run() diff --git a/tests/snapshot_tests/test_snapshots.py b/tests/snapshot_tests/test_snapshots.py index a329811d09..33373e6588 100644 --- a/tests/snapshot_tests/test_snapshots.py +++ b/tests/snapshot_tests/test_snapshots.py @@ -192,6 +192,7 @@ def test_option_list(snap_compare): assert snap_compare(WIDGET_EXAMPLES_DIR / "option_list_options.py") assert snap_compare(WIDGET_EXAMPLES_DIR / "option_list_tables.py") + # --- CSS properties --- # We have a canonical example for each CSS property that is shown in their docs. # If any of these change, something has likely broken, so snapshot each of them. @@ -364,3 +365,19 @@ async def run_before(pilot): def test_layer_fix(snap_compare): # Check https://github.com/Textualize/textual/issues/1358 assert snap_compare(SNAPSHOT_APPS_DIR / "layer_fix.py", press=["d"]) + + +def test_modal_dialog_bindings_input(snap_compare): + # Check https://github.com/Textualize/textual/issues/2194 + assert snap_compare( + SNAPSHOT_APPS_DIR / "modal_screen_bindings.py", + press=["enter", "tab", "h", "!", "left", "i", "tab"], + ) + + +def test_modal_dialog_bindings(snap_compare): + # Check https://github.com/Textualize/textual/issues/2194 + assert snap_compare( + SNAPSHOT_APPS_DIR / "modal_screen_bindings.py", + press=["enter", "tab", "h", "i", "tab", "enter"], + ) From fdb885233fda5f16f9ac501b79084a2f10763783 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Apr 2023 08:43:59 +0100 Subject: [PATCH 7/8] version bump --- CHANGELOG.md | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 626e5608de..f32359a8ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [0.17.2] - Unreleased +## [0.17.2] - 2023-04-02 ### [Fixed] diff --git a/pyproject.toml b/pyproject.toml index 7820894aee..35747c4461 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "textual" -version = "0.17.1" +version = "0.17.2" homepage = "https://github.com/Textualize/textual" description = "Modern Text User Interface framework" authors = ["Will McGugan "] From 80d9681cafb83c9f2b801ef1c854a7ff91bd6f7f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Apr 2023 08:47:33 +0100 Subject: [PATCH 8/8] docstring --- src/textual/widgets/_button.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/widgets/_button.py b/src/textual/widgets/_button.py index 6c8faa8546..281b3c1169 100644 --- a/src/textual/widgets/_button.py +++ b/src/textual/widgets/_button.py @@ -256,7 +256,7 @@ def _start_active_affect(self) -> None: ) def action_press(self) -> None: - """Activate a press if""" + """Activate a press of the button.""" self.press() @classmethod