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

Widget.loading can cause widgets to disappear #3935

Closed
davep opened this issue Dec 29, 2023 · 5 comments · Fixed by #5079
Closed

Widget.loading can cause widgets to disappear #3935

davep opened this issue Dec 29, 2023 · 5 comments · Fixed by #5079
Labels
bug Something isn't working documentation Improvements or additions to documentation Task

Comments

@davep
Copy link
Contributor

davep commented Dec 29, 2023

This might be a case of Widget.loading needing some more work, or the documentation for the property needing expanding, but right now using the widget loading property can have results that are counter what you'd expect given the documentation:

If set to True this widget will temporarily be replaced with a loading indicator.

This would suggest that a loading indicator would appear in a space the same dimension as the widget it's overlaying. With some widgets, that isn't the case. As a sample:

from textual.app import App, ComposeResult
from textual.widgets import Static, Label, Footer, Button

class Fixed(Label):

    DEFAULT_CSS = """
    Fixed {
        width: 1fr;
        height: 10;
        background: teal;
    }
    """

class LabelLoadingTestApp(App[None]):

    BINDINGS = [
        ("space", "toggle", "Toggle the loading indicator"),
    ]

    def compose(self) -> ComposeResult:
        yield Label()
        yield Label("This is a test")
        yield Static()
        yield Static("This is a test")
        yield Footer()
        yield Button("")
        yield Button("Foo")
        yield Fixed("This is fixed")

    def action_toggle(self) -> None:
        for widget in self.query("Screen > *").results():
            widget.loading = not widget.loading

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

without loading, the screen looks like this:

Screenshot 2023-12-29 at 16 30 30

when you toggle on loading, it looks like this:

Screenshot 2023-12-29 at 16 30 34

@davep davep added bug Something isn't working documentation Improvements or additions to documentation Task labels Dec 29, 2023
@TomJGooding
Copy link
Contributor

For anyone who might stumble across this issue, you can workaround this by adding a min width/height to your widgets, for example:

class LabelLoadingTestApp(App[None]):
    CSS = """
    Static, Label {
        min-height: 1;
        min-width: 9;
    }

    Button {
        min-height: 3;
    }
    """

@willmcgugan
Copy link
Collaborator

The loading indicator works with containers, but when applied to simple widgets it transforms them into containers that don't render because they have no dimensions.

I don't think there is an easy fix for this one. We likely need some additional mechanism to change how a widget is rendered without changing its dimensions.

Worth doing, but not a small job.

Parking for now.

@darrenburns
Copy link
Member

After seeing #4958 and digging into this a bit in the past, I'm getting the feeling that more generally we need an approach to loading indicators that doesn't "turn a widget into a container" or interfere with a widget's children in any way.

As an author of a container widget, I don't want to worry about suddenly receiving a new "child" widget of a totally unexpected type.

@willmcgugan
Copy link
Collaborator

I came to much the same conclusion. We need a mechanism that says render this widget in place of or on top of this other widget.

Copy link

github-actions bot commented Oct 2, 2024

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 documentation Improvements or additions to documentation Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants