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

Screen.dismiss doesn't call the callback function if it's called with no args #4790

Closed
darrenburns opened this issue Jul 22, 2024 · 2 comments · Fixed by #4795
Closed

Screen.dismiss doesn't call the callback function if it's called with no args #4790

darrenburns opened this issue Jul 22, 2024 · 2 comments · Fixed by #4795
Labels
bug Something isn't working

Comments

@darrenburns
Copy link
Member

darrenburns commented Jul 22, 2024

e.g. screen.dismiss()

or in a Binding Binding(..., action="screen.dismiss", ...).

Confirmed with this MRE:

from textual.app import App, ComposeResult
from textual.screen import Screen
from textual.widgets import Static


class MyScreen(Screen):
    BINDINGS = [("space", "screen.dismiss()", "Dismiss")]

    def compose(self) -> ComposeResult:
        yield Static("Hello")


class MyApp(App):
    def on_mount(self) -> None:
        def callback(sd):
            self.notify("Callback")

        self.push_screen(MyScreen(), callback)

    def compose(self) -> ComposeResult:
        yield Static("Default Screen")


if __name__ == "__main__":
    app = MyApp()
    app.run()
@darrenburns darrenburns added the bug Something isn't working label Jul 22, 2024
@willmcgugan
Copy link
Collaborator

This does seem like it was by design.

From the dismiss docstring:

   If `result` is provided and a callback was set when the screen was [pushed][textual.app.App.push_screen], then
  the callback will be invoked with `result`.

I think the rationale might have been that the callback exists to receive the argument to dismiss(). And if you don't supply one, then you don't need the callback.

On reflection supplying a callback and then discarding it feels like a wart.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants