Skip to content

fix for progress bar crash#6139

Merged
willmcgugan merged 23 commits into
mainfrom
fix-progress-bar-crash
Sep 29, 2025
Merged

fix for progress bar crash#6139
willmcgugan merged 23 commits into
mainfrom
fix-progress-bar-crash

Conversation

@willmcgugan

Copy link
Copy Markdown
Member

Fixes #6127

@TomJGooding

Copy link
Copy Markdown
Contributor

Pilot.click will return True if the initial mouse down is on the specified target

Doesn't this risk false positives? For example, if start clicking a button but it somehow moves before mouse up, this wouldn't actually produce a click in the app?

@willmcgugan

Copy link
Copy Markdown
Member Author

Pilot.click will return True if the initial mouse down is on the specified target

Doesn't this risk false positives? For example, if start clicking a button but it somehow moves before mouse up, this wouldn't actually produce a click in the app?

The purpose of that parameter is just to tell the pilot where to click. The return value is a sanity check to confirm that the widget was at least under the "mouse" when the call started.

@TomJGooding

Copy link
Copy Markdown
Contributor

The purpose of that parameter is just to tell the pilot where to click.

But a click is actually more than just a mouse down, right?

Here's an (admittedly contrived!) example. I would expect this to fail since the button is never actually clicked, but if I've understood this change correctly, the check would now confusingly pass?

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


class Example(Static, can_focus=True):
    DEFAULT_CSS = """
    Example {
        color: $text-accent;
        margin: 1 0;
        &:focus { text-style: bold underline; }
    }
    """

    def on_blur(self) -> None:
        self.display = False


class ExampleApp(App):
    click_count = 0

    def compose(self) -> ComposeResult:
        yield Example("Click the button")
        yield Button()

    def on_button_clicked(self) -> None:
        self.click_count += 1


async def test_example_app():
    app = ExampleApp()
    async with app.run_test() as pilot:
        # This SHOULD fail
        assert await pilot.click(Button)
        assert app.click_count == 1


if __name__ == "__main__":
    app = ExampleApp()
    app.run()

@willmcgugan

Copy link
Copy Markdown
Member Author

But a click is actually more than just a mouse down, right?

Yes.

The pilot.click call returns True because there was a Button there to be clicked. That's all the return value confirms.

Don't think of it as delivering a Click event to a widget. It's more that the user clicked that button. Even if the button moves or disappears after the mouse down, from the user's POV they still clicked it.

When you write a test, you still have to confirm it had the intended effect.

It's always failing on Windows btw. Do you have any insight why? I want to solve it without an arbitrary pause.

@TomJGooding

TomJGooding commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

Okay, perhaps I've misunderstood the purpose then, But my reason for questioning this change is because the current behaviour is what helped me understand these flaky tests recently involving the footer.

Sorry, no idea why it always seems to fail on Windows.

@willmcgugan willmcgugan merged commit 4e09a26 into main Sep 29, 2025
23 checks passed
@willmcgugan willmcgugan deleted the fix-progress-bar-crash branch September 29, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProgressBar can be caused to crash with CSS since 5.0.0

2 participants