Skip to content

Conversation

@PhilLab
Copy link
Contributor

@PhilLab PhilLab commented Dec 15, 2025

  • Tests written, or not not needed

Before dismissing the dialog, we need to wait for a potentially pending transaction. As showing the dialog also includes the dismissing of prior instances, we need to wait there as well.
Both is needed to satisfy the test case added in the previous commit.

Otherwise, the dialog might be shown after it was meant to be dismissed already. This issue was observed when testing RemoveFilesDialogFragment's removeFiles() and also sporadically "in the wild" when using the app productively

If you want to see for yourself, first checkout the test commit and see the test fail. Then checkout the fix commit and see the test succeed

@PhilLab PhilLab force-pushed the loading_dialog_timing_issue branch from 9deca52 to 8e549a1 Compare December 16, 2025 17:22
This timing issue was reproducible when testing
RemoveFilesDialogFragment#removeFiles and sporadically "in the wild".

However, no solution offered so far

Signed-off-by: Philipp Hasper <vcs@hasper.info>
@PhilLab PhilLab force-pushed the loading_dialog_timing_issue branch from 8e549a1 to 4dcbad8 Compare December 16, 2025 17:24
Before dismissing the dialog, we need to wait for a potentially pending
transaction. As showing the dialog also includes the dismissing of prior
instances, we need to wait there as well.
Both is needed to satisfy the test case added in the previous commit.

Otherwise, the dialog might be shown after it was meant to be dismissed
already. This issue was observed when testing RemoveFilesDialogFragment's
removeFiles() and also sporadically "in the wild".

Signed-off-by: Philipp Hasper <vcs@hasper.info>
@PhilLab PhilLab changed the title Illustrating timing issue with FileActivity#dismissLoadingDialog Fixing timing issue with FileActivity's loading dialog show and dismiss Dec 16, 2025
@PhilLab PhilLab marked this pull request as ready for review December 16, 2025 17:27
@PhilLab
Copy link
Contributor Author

PhilLab commented Dec 16, 2025

@alperozturk96 can you review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants