textual icon indicating copy to clipboard operation
textual copied to clipboard

`Hide` event not working as advertised?

Open davep opened this issue 9 months ago • 9 comments

As I read the docs for Hide, it's my understanding that this code should show show and hide notifications as I hit the spacebar:

from textual.app import App, ComposeResult
from textual.widgets import Label

class ShowHideLabel(Label):

    def on_show(self) -> None:
        self.notify("Show!")

    def on_hide(self) -> None:
        self.notify("Hide!")

class ShowHideApp(App[None]):

    BINDINGS = [("space", "toggle")]

    def compose(self) -> ComposeResult:
        yield ShowHideLabel("Here I am")

    def action_toggle(self) -> None:
        self.query_one(ShowHideLabel).visible = not self.query_one(ShowHideLabel).visible

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

but I only ever see show events.

davep avatar Oct 04 '23 20:10 davep

I think the root of the problem might be here following commit a40300a. If I've understood correctly, won't hidden_widgets be empty?

https://github.com/Textualize/textual/blob/3aaecd39200c2251cb9af8e450c5291839638b7f/src/textual/_compositor.py#L361-L368

It looks like the Hide messages are posted by the screen here:

https://github.com/Textualize/textual/blob/3aaecd39200c2251cb9af8e450c5291839638b7f/src/textual/screen.py#L741-L746

TomJGooding avatar Oct 05 '23 16:10 TomJGooding

I think so, yes. Widgets with visible = False are still considered in the layout, but filtered out somewhere else.

willmcgugan avatar Oct 05 '23 16:10 willmcgugan

Well I've confirmed it wasn't that commit as changing back to visible_only=True doesn't work either.

I think best left to the experts, but it was interesting digging deeper into Textual!

TomJGooding avatar Oct 05 '23 17:10 TomJGooding

I think what is happening is that the sets for shown / hidden / resized ignore the visible property.

It would be nice if setting visible=False did produce a Hide in the same way as setting display=False. But only if it can be done in a way that doesn't impact performance too much.

Worth investigating.

willmcgugan avatar Oct 11 '23 10:10 willmcgugan

We need to discuss what the Hide event should do.

Right now, the docs says a Hide event will fire when a widget moves off-screen, which feels very wrong to me.

I think Hide should only fire when visibility or display is changed.

darrenburns avatar Nov 06 '23 10:11 darrenburns

The Hide event is sent when the widget was in the previous layout, but not in the new one. Essentially the logical inverse of the Show event.

visibility is a bit of a special case. If visibility is False it is still considered to be in the layout, but never actually drawn.

The logic is somewhat determined by how the compositor is implemented. It uses set operations to very efficiently figure out what is shown / hidden.

I'd be interested in understanding why you think it should work differently. Let's have a chat about it tomorrow!

willmcgugan avatar Nov 07 '23 15:11 willmcgugan

@darrenburns Does this need any work?

willmcgugan avatar Nov 21 '23 16:11 willmcgugan

@willmcgugan Yes, the example in the original post was never fixed. I started "fixing" it, but wasn't sure about the exact intended behaviour. The reason I was unsure was because of all the ways all the ways a widget could go from being visible to not visible - being hidden behind a docked widget, offset pushing it off screen, hidden behind an overlay widget, modal screen etc (also wasn't sure if the compositor handle all of these cases in the hidden/visible sets).

This behaviour of a Hide event firing in these situations also felt "wrong" to me in that it was too magical. I expected the Hide event to only fire when visible/display was explicitly set via code, rather than it possibly being sent by some other widget which happens to be rendered above it, hiding it from view.

Given all of this it felt better to raise it for discussion instead of guessing.

darrenburns avatar Nov 21 '23 16:11 darrenburns

Going to park this for now. I think there may be two concepts that are being conflated: Events when widgets come in to the viewport, and events when something is toggled. Until we have a use case we can consider, it is hard to know what the behaviour shoulld be.

willmcgugan avatar Nov 21 '23 17:11 willmcgugan