From e75f784b2c788f95e398821266fcaab0f79aa12f Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 13 Dec 2022 10:12:16 +0000 Subject: [PATCH] Add a test for a screen binding movement, wrapping a focusable widget This is the heart of the issue introduced by https://github.com/Textualize/textual/pull/1170/commits/b48a1402b8103ca16d5e338538620e9e08fb2c0e and which is being investigated in https://github.com/Textualize/textual/issues/1343 -- the child widget can be focused, but (as far as the author of the code is concerned) it has no bindings. Bindings for movement-oriented keys exist on the screen which composes up the widget into it. Up until 0.5.0 this worked just fine. As of 0.6.0, because binding inheritance was introduced, the bindings for movement that live at the `Widget` level cause the widget that has no bindings to appear to have bindings. While this can potentially be worked around with the use of inherit_bindings, this isn't a very satisfying solution and also breaks the rule of least astonishment. This test is going to be key to all of this. This is the test that should be made to work without breaking any of the other currently-passing tests. --- tests/test_binding_inheritance.py | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/test_binding_inheritance.py b/tests/test_binding_inheritance.py index 9d7bbd363..ab6b675af 100644 --- a/tests/test_binding_inheritance.py +++ b/tests/test_binding_inheritance.py @@ -164,3 +164,42 @@ async def test_focused_child_widget_with_movement_bindings() -> None: async with AppWithWidgetWithBindings().run_test() as pilot: await pilot.press("x", *MOVEMENT_KEYS, "x") assert pilot.app.pressed_keys == ["x", *[f"locally_{key}" for key in MOVEMENT_KEYS], "x"] + +############################################################################## +class FocusableWidgetWithNoBindings(Static, can_focus=True): + """A widget that can receive focus but has no bindings.""" + +class ScreenWithMovementBindings(Screen): + """A screen that binds keys, including movement keys.""" + + BINDINGS = [ + Binding("x", "record('x')", "x"), + *[Binding(key, f"screen_record('{key}')", key) for key in MOVEMENT_KEYS] + ] + + async def action_screen_record(self, key: str) -> None: + # Sneaky forward reference. Just for the purposes of testing. + await self.app.action_record(f"screen_{key}") + + def compose(self) -> ComposeResult: + yield FocusableWidgetWithNoBindings() + + def on_mount(self) -> None: + self.query_one(FocusableWidgetWithNoBindings).focus() + +class AppWithScreenWithBindingsWidgetNoBindings(AppKeyRecorder): + """An app with a non-default screen that handles movement key bindings.""" + + SCREENS = {"main":ScreenWithMovementBindings} + + def on_mount(self) -> None: + self.push_screen("main") + +@pytest.mark.xfail( + reason="Movement keys never make it to the screen with such bindings due to key inheritance and pre-bound movement keys [issue#1343]" +) +async def test_focused_child_widget_with_movement_bindings_on_screen() -> None: + """A focused child widget, with movement bindings in the screen, should trigger screen actions.""" + async with AppWithScreenWithBindingsWidgetNoBindings().run_test() as pilot: + await pilot.press("x", *MOVEMENT_KEYS, "x") + assert pilot.app.pressed_keys == ["x", *[f"locally_{key}" for key in MOVEMENT_KEYS], "x"]