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

Using dismiss in call_after_refresh now raises an error. #5049

Closed
JoeZiminski opened this issue Sep 24, 2024 · 4 comments
Closed

Using dismiss in call_after_refresh now raises an error. #5049

JoeZiminski opened this issue Sep 24, 2024 · 4 comments

Comments

@JoeZiminski
Copy link

JoeZiminski commented Sep 24, 2024

I just wanted to highlight change in behaviour that I could not see documented in the change log. Previously I was calling call_after_refresh(lambda: self.dismiss(True)) within a ModalScreen. Now, this gives an error:

ScreenError: Can't await screen.dismiss() from the screen's message handler; try removing the await

I think because of a new check where this error is raised. I found another workaround to acheive what I wanted, but thought I'd raise this in case 1) this change in behaviour was not intended 2) an improved error mesage could be given. It took me a while to track it down, if suitable, if the error could indicate not to use self.dismiss in call_after_refresh it might be useful.

Sorry I cannot give clear versions on which this works / does not work. But it is OK in 0.71.0 but not in most recent version.

@TomJGooding
Copy link
Contributor

TomJGooding commented Sep 24, 2024

I'm not sure what may have changed, but why use a lambda rather than call_after_refresh(self.dismiss, True)?

EDIT: Never mind, after taking a closer look I think this changed in #4789 with the warning below added to the dismiss docs. But I'm also struggling to understand this warning and the error message in the context of dismissing a screen after it has refreshed.

Textual will raise a ScreenError if you await the return value from a message handler on the Screen being dismissed. If you want to dismiss the current screen, you can call self.dismiss() without awaiting.

@Textualize Textualize deleted a comment from github-actions bot Sep 25, 2024
@willmcgugan
Copy link
Collaborator

call_after_refresh will execute the callback within the screen's message pump. If it returns something awaitable (which dismiss does) then it will await it.

The error is because it would end up waiting for itself to complete. i.e. a deadlock.

it is an edge case. Can't think of any way to auto-detect that right now.

However, I suspect it isn't necessary. From a quick look at your project I gather you want a refresh before the UI freezes. You almost certainly want to use workers to prevent freezing.

I suspect that if you were to add @work to your button handler, that would prevent freezing and negate the need for call_after_refresh.

@JoeZiminski
Copy link
Author

JoeZiminski commented Sep 26, 2024

Thanks a lot @TomJGooding @willmcgugan, thanks both! That all makes sense, thanks for the tip on using workers, looking forward to trying that out! I guess this will also allow the use of data loader widget which I was very keen on. Cheers!

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

No branches or pull requests

3 participants