Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent events when revealing a widget #2582

Closed
davep opened this issue May 16, 2023 · 4 comments
Closed

Inconsistent events when revealing a widget #2582

davep opened this issue May 16, 2023 · 4 comments
Assignees
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented May 16, 2023

This is related to #2557 in that it's something noticed while trying to get to the core of what was going on in that issue. The code that follows is a little convoluted, but it's done in a way to try and mimic how Select works. While possibly not the root of the issue, the idea here is that a widget is shown/hidden by having the classes of its container manipulated (this is how Select works). It might be that this aspect of the issue can be eliminated.

Also note that this isn't being recorded for a deep dive right now, this is more a case of recording a set of symptoms and some code for reproducing them, and then it can be looked at later.

So, the code:

from textual.app        import App, ComposeResult
from textual.containers import Vertical, Horizontal
from textual.widgets    import OptionList, Button, TextLog, Label

class Wrapper( Vertical ):
    pass

class ReportingLabel( Label ):

    def _on_show( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_show - {self.size}")

    def _on_resize( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_resize - {self.size}")

    def _on_hide( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_hide - {self.size}")

class ReportingList( OptionList ):

    def _on_show( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_show - {self.size}")

    def _on_resize( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_resize - {self.size}")

    def _on_hide( self ) -> None:
        self.app.screen.query_one(TextLog).write(f"on_hide - {self.size}")

class ShowByParentClassApp( App[ None ] ):

    CSS = """
    Wrapper > Widget {
        width: 1fr;
        display: none;
        height: auto;
    }

    Wrapper.show > Widget {
        display: block;
    }
    """

    def compose( self ) -> ComposeResult:
        with Horizontal():
            with Vertical():
                yield Button( "Toggle Option List", id="list" )
                with Wrapper( id="target-list" ):
                    yield ReportingList( "Foo", "Bar", "Baz", "Wibble", "Wobble" )
            with Vertical():
                yield Button( "Toggle Label", id="label" )
                with Wrapper( id="target-label" ):
                    yield ReportingLabel( "This is a label" )
            yield TextLog()

    def on_button_pressed( self, event: Button.Pressed ) -> None:
        self.query_one( f"#target-{event.button.id}", Wrapper ).toggle_class( "show" )

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

This will give a display with two buttons and a TextLog. If either button is pressed I think you'd expect to see an on_show event for the related just-shown widget.

On first startup, if you press the button to show a label, the Label widget gets an on_show and on_resize event. Every toggle after that also gets the same pair of events (hiding it never gets a on_hide, see #2574).

Again, on first startup, if you press the button to show a list, the OptionList widget gets an on_resize event, but surprisingly no on_show. Every toggle after that gets an on_show and on_resize pair of events (again, no on_hide for the reasons mentioned above).

I've not dived deeper into this than to document the above (and to test similar issues within Select itself). The only observation I've got right now is that Label inherits from Static, which inherits from Widget. OptionList on the other hand goes via ScrollView, which does have a bit of code pertaining to sizes. I don't know that this is relevant here but it does seem to be the one obvious difference between the two sets of results.

@davep davep added bug Something isn't working Task labels May 16, 2023
davep added a commit to davep/textual that referenced this issue May 16, 2023
While the fix for Textualize#2557 likely isn't *the* fix (see Textualize#2582 for some context
around that), it is a fix that works for now. As such, with the change,
there was a double attempt to refresh the content tracking in the clearing
of options in the OptionList, which shouldn't be necessary.

This removes that.
@willmcgugan
Copy link
Collaborator

There have been some related fixes since this issue. It may be working as expected now.

@davep davep self-assigned this May 31, 2023
@davep
Copy link
Contributor Author

davep commented May 31, 2023

Grabbing to recheck the above code.

@davep
Copy link
Contributor Author

davep commented May 31, 2023

Tested with main as of 4ff1d18 (2023-05-31T14:00+0100) and now all working as expected.

Closing as fixed by recent changes.

@davep davep closed this as completed May 31, 2023
@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Task
Projects
None yet
Development

No branches or pull requests

2 participants