-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
Make caption dialog non-cancellable (issue #4939) #4942
base: develop
Are you sure you want to change the base?
Conversation
This is a good start, but I think we will still need another dialog to prevent accidental cancel with the cancel button |
this is what introduced it svenjacobs@9e66ccf, and the only change there that looks like it had something to do with |
So instead of making it non-cancellable, we need a warning dialog when text was entered and someone either presses Cancel, dismissed the dialog by clicking outside or uses the back navigation. |
No, the non-cancellable thing makes sense and can stay. Just add another confirmation to the cancel button. |
So are you fine with the back navigation not doing anything when this dialog is opened? |
Well we don't want accidental cancels, so yes? Back button should either do nothing, or show the warning dialog as well. But definitely not close the dialog silently. |
Sorry guys but I currently struggle with this a bit as I have only coded in Jetpack Compose for the past few years and it seems I forgot how to handle I tried to implement the |
Yes probably |
A few observations:
However: I would say that we could stay with normal UI behavior here:
|
@svenjacobs Do you still want to work on this? If not I can just merge it like this and do the remaining stuff myself. |
Sorry, some other stuff caught up. Please go ahead and merge it. I think you will solve this in half the time than me struggling with the legacy Android view system 😉 |
I just made the dialog non-cancellable. Actually I don't know why it was cancellable when there also is
dialog?.setCanceledOnTouchOutside(false)
Fixes #4939