Textualize/textual

Potential bug with bindings not appearing after 0.55.0

Closed this issue · 4 comments

I'm naming this issue as "potential" since I don't know what is the expected behavior, but since there was nothing about it in the release note (and this change is breaking), wanted to show it.

For me, it looks like a bug because things specified at the Screen level have a higher priority than at the App level (I mean even without explicitly setting the priority=True)

Appears after 0.55.0, specifically talking, I'm suspecting this change:

Fix priority bindings not appearing in footer when key clashes with focused widget #4342

Consider such an MRE:

from __future__ import annotations

from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.screen import Screen
from textual.widgets import Footer, Static


class ScreenWithNoBindingVisible(Screen):
    def compose(self) -> ComposeResult:
        yield Static(self.__class__.__name__)
        yield Footer()


class ScreenThatShowedBindingBefore_0_55_0(ScreenWithNoBindingVisible):
    BINDINGS = [
        Binding("f1", "notify", "Hey, I was visible before 0.55.0!"),
    ]


class BindingOverrideApp(App):
    BINDINGS = [
        Binding("f1", "notify", show=False),
        Binding("p", "pop_screen", "Pop screen"),
    ]

    def on_mount(self) -> None:
        self.push_screen(ScreenWithNoBindingVisible())
        self.push_screen(ScreenThatShowedBindingBefore_0_55_0())

    def action_notify(self):
        self.notify("F1 pressed!")


BindingOverrideApp().run()

Pre 0.55.0 (tested on 0.54.0):
image

On 0.55.0:
image

Of course it's fixable by changing the following line like:

Binding("f1", "notify", "Hey, I was visible before 0.55.0!", priority=True),

but this is what's this issue about - previously it worked like a charm without any explicit setting.
I'm curious what you think, is this a bug or is it the default behavior from now on?

From my side - previous behavior was expected - if I disabled something globally, then I could enable it on some screen/widget (which is a more local setting) just by overriding BINDINGS with show=True. Worked with no surprises.

We found the following entry in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

Yeah, I think my fix in the linked PR had this as an unintended side effect.

I can confirm that I notice this as well. There has been a breaking of how Bindings are handled. My Bindings visible in the Footer are supposed to automatically change depending on what Screen/ Mode I'm on, Now, the Bindings don't change at all, or change for some Bindings, not others

Don't forget to star the repository!

Follow @textualizeio for Textual updates.