Some keys are no longer showing in the Footer

[davep]

Given this code in Textual v0.15.1:

from textual.app     import App, ComposeResult
from textual.widgets import Header, Footer
from textual.binding import Binding

class MissingFooterStuffApp( App[ None ] ):

    BINDINGS = [
        Binding( "up", "oops(0)", "Up" ),
        Binding( "down", "oops(1)", "Down" ),
        Binding( "left", "oops(2)", "Left" ),
        Binding( "right", "oops(3)", "Right" ),
        Binding( "ctrl+up", "oops(4)", "Up" ),
        Binding( "ctrl+down", "oops(5)", "Down" ),
        Binding( "ctrl+left", "oops(6)", "Left" ),
        Binding( "ctrl+right", "oops(7)", "Right" ),
        Binding( "space", "oops(8)", "Space" ),
        Binding( "enter", "oops(9)", "Space" ),
        Binding( "q", "oops(10)", "Q" ),
    ]

    def compose( self ) -> ComposeResult:
        yield Header()
        yield Footer()

    def action_oops( self, _ ):
        self.app.exit()

if __name__ == "__main__":
    MissingFooterStuffApp().run()

while all the bindings work, not all of them are showing in the Footer any more. The above isn't an exhaustive list (I'm told that Home and End aren't showing up either).

Fairly sure these were working okay until recently.

Thanks to DigitalSeraphim on Discord for alerting us to that.

[digitalseraphim]

This was due to the swapping of the order of "precedence" in the "binding chain" applied in 32d18a

Other namespace bindings (in the case of "up", it's the "screen") are overwriting the App defined bindings if the key matches.

As this was obviously an intentional change, I'll leave it to the main team to fix 😄

[willmcgugan]

Probably a dupe of #2219

[davep]

Looking at this a little further, the issue here seems to come down to doing things on the application vs, say, doing them on screen or a widget. To narrow it down to a simple example, here's an app where we bind Up on the app:

from textual.app      import App, ComposeResult
from textual.binding  import Binding
from textual.reactive import reactive
from textual.widgets  import Header, Footer, Label

class FooterBindingOnApp( App[ None ] ):

    BINDINGS = [
        Binding( "up", "up", "Press to get the highest score! (App)" ),
    ]

    score: reactive[int] = reactive(0)

    def compose( self ) -> ComposeResult:
        yield Header()
        yield Label()
        yield Footer()

    def action_up( self ) -> None:
        self.score += 1
        self.query_one( Label ).update( f"Score: {self.score}" )

if __name__ == "__main__":
    FooterBindingOnApp().run()

As reported here, when we run it the binding works, but we don't get the help we want in the footer.

However, if we don't use the default screen, but instead create our own screen and move all the work on to that, the binding works and we get the help:

from textual.app      import App, ComposeResult
from textual.binding  import Binding
from textual.reactive import reactive
from textual.screen   import Screen
from textual.widgets  import Header, Footer, Label

class Game( Screen ):

    BINDINGS = [
        Binding( "up", "up", "Press to get the highest score! (Screen)" ),
    ]

    score: reactive[int] = reactive(0)

    def compose( self ) -> ComposeResult:
        yield Header()
        yield Label()
        yield Footer()

    def action_up( self ) -> None:
        self.score += 1
        self.query_one( Label ).update( f"Score: {self.score}" )

class FooterBindingOnScreen( App[ None ] ):

    def on_mount( self ) -> None:
        self.push_screen( Game() )

if __name__ == "__main__":
    FooterBindingOnScreen().run()

This actually makes sense. Screen inherits from Widget and Widget defines the navigation keys as show=False. When doing the binding on the application the default Screen will "win" in terms of getting the description and, more importantly, the show status of the binding. On the other hand, if the binding is done on a custom screen it will win over the bindings inherited from Widget.

I think the conclusion here is that this is working as intended; albeit it's a little surprising as it's easy to forget that when you're doing work on your App-derived class, you have to take the effects of the default screen that's used for App.screen into account.

This very much feels like one of those times where, unless you're doing the Textual equivalent of a one-liner, don't use the default screen; create and use your own screen and do the work on that (as it will result in less surprising behaviour).

Tagging @rodrigogiraoserrao and @darrenburns in on this too due to #2219 as I'm inclined to close this as "works as intended", but I'd like everyone to have one last think over it before I do so.

(and longer-term I think we might want to think about adding something to the documentation that encourages using a non-default screen for all but the most trivial of applications).

[rodrigogiraoserrao]

Intuition often seems to lean on "everything that is custom should overwrite everything that is default".
We just need to determine whether we want to be strictly consistent in this case or whether we want to go with intuition.

I'm fine with either.

If we change nothing, maybe I'd add a note somewhere in the docs about this gotcha.

[davep]

Intuition often seems to lean on "everything that is custom should overwrite everything that is default".

This is where it gets tricky, and where the default screen often causes confusion. Nothing is overriding the default here (in the app version). The defaults are on the screen, inherited from Widget. Nothing in that chain is overwriting them.

[rodrigogiraoserrao]

Maybe I misunderstood. By default, a binding is shown, right? So, the app binding for up in your app example should be shown. But it is not, because it is getting the show=False from the default screen, right? That's what I understood from your explanation...

And that's what I am saying is counter-intuitive. How come a default setting is overriding our custom, explicit binding?

[davep]

You've got it right: I'm just emphasising that it's the whole business of the default screen that is counterintuitive, not the bindings. Having a default screen is incredibly useful but also causes people to conflate the app class and the screen class; and that things are a lot easier to reason about if you make your app as minimal as possible and work on your own screen.

[digitalseraphim]

To me the "inversion" of the "binding override tower" is confusing. I guess I imagine the events "trickling down" to individual widgets, allowing each parent to "capture" the event before it goes to it's children. I'm not exactly sure why I think of it this way, and can't remember how any other framework I've used worked in this situation. I can definitely think of some situations where I wouldn't want this though, so, I guess it makes sense this way as well.

My app doesn't need scrolling anywhere, so it would be nice if I could specify that I didn't want a scrolling screen. But that doesn't seem possible, given that Screen extends Widget, which "unilaterally" binds to the movement keys. Which is where the "custom overriding default" actually works. However, the example above, with the custom Screen only workw until you have a focused widget within that screen because the bindings start at the focused widget and work "up" the stack to the screen and app. (Just add a yield Input() and click in it to see that happen)

To me, application and screen bindings should override everything. App should override screen, because screens change, but "application" level bindings should be "universal".

[davep]

Right, good points, and we can see the core of what's going on here and how it gets in the way. The plan is to move the scrollable support code out of Widget and into a child class of Widget that will be the base class for scrollable containers.

I'll spin up a new issue for that, but keep this open for now and reference it.

[rodrigogiraoserrao]

You've got it right: I'm just emphasising that it's the whole business of the default screen that is counterintuitive [...]

Right, to me it doesn't make much sense that the default screen is going to interfere with the app's bindings, that's all. 🤷
I get why it happens. I just don't like it.
Especially given that the app binding still works, which makes it look like it is broken.

[davep]

Opened #2331 and #2332 as issues that will feed back into here (hope to tackle them tomorrow).

[github-actions]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.