Skip to content

Conversation

willmcgugan
Copy link
Member

Fixes #6127

@TomJGooding
Copy link
Collaborator

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
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
Collaborator

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
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
Copy link
Collaborator

TomJGooding commented Sep 29, 2025

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