Fix issue with modals (#2195)

* Fix issue with modals

* changelog

* fix binding on button

* binding tweak

* changelog

* snapshots

* version bump

Author: willmcgugan

CHANGELOG.md

@@ -5,12 +5,18 @@ 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] - 2023-04-02
 
-### 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
+- 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
 
@@ -687,6 +693,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

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 <[email protected]>"]

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)
@@ -1961,38 +1957,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, 1):
+            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 +1996,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:

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()

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,

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(