Skip to content

Fix progress dialog outside touch #20210

Closed
Alok-Silswal wants to merge 0 commit intoankidroid:mainfrom
Alok-Silswal:fix-progress-dialog-outside-touch-
Closed

Fix progress dialog outside touch #20210
Alok-Silswal wants to merge 0 commit intoankidroid:mainfrom
Alok-Silswal:fix-progress-dialog-outside-touch-

Conversation

@Alok-Silswal
Copy link
Contributor

@Alok-Silswal Alok-Silswal commented Jan 24, 2026

Purpose / Description

During a forced one-way / full sync, the progress dialog can currently be dismissed by tapping on the dimmed background (i.e. Outside the box).

Fixes

Approach

  • Added Cancel button to manually abort the full sync.
  • Canellation logic now handles disabling of outside touch.

How Has This Been Tested?

Tested on my phone (Android 14, Xiaomi / HyperOS) using a debug build.

Outside touch no longer cancels syncing.

Videos attached as evidence.

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings) (N/A – no visual UI changes)
  • UI Changes: You have tested your change using the [Google Accessibility Scanner] (N/A – no UI changes)(https://play.google.com/store/apps/details?id=com.google.android.apps.accessibility.auditor)

Before this change:

Outside_Touch_BEFORE.mp4

After this change:

Outside_Touch_AFTER.mp4

@welcome
Copy link

welcome bot commented Jan 24, 2026

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backend offers cancellation, but as far as I understand it, this PR removes the only way to access it (maybe the back button?)

You haven't shown evidence of testing for this change.


The other syncing dialogs have a 'cancel' button [sorry for the bad screensnip], is there a reason you didn't add this in?

Image

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Jan 29, 2026
@Alok-Silswal
Copy link
Contributor Author

Alok-Silswal commented Feb 6, 2026

This change only disables cancellation via outside-touch. It does not change setCancelable(onCancel != null), so cancellation via the Back button remains available and continues to trigger cancelSync() as before.

The backend cancellation path is unchanged and no code regarding this has been touched; only accidental dismissal via background taps is prevented.

setCanceledOnTouchOutside(false) specifically controls outside-touch behavior without affecting other cancellation mechanisms.

Reference: https://developer.android.com/reference/android/app/Dialog#setCanceledOnTouchOutside(boolean)


The other syncing dialogs have a 'cancel' button [sorry for the bad screensnip], is there a reason you didn't add this in?

I intentionally avoided adding new UI here to keep the change minimal.

Other sync dialogs do include a Cancel button, but the forced full-sync dialog currently relies on the Back button for cancellation, and this PR preserves that existing behavior rather than introducing a new interaction.


If adding an explicit Cancel button here would be preferable for consistency, I will do the needful accordingly.

@Alok-Silswal Alok-Silswal marked this pull request as draft February 24, 2026 05:22
@Alok-Silswal Alok-Silswal marked this pull request as ready for review February 24, 2026 13:49
@Alok-Silswal
Copy link
Contributor Author

You haven't shown evidence of testing for this change.

I have updated the PR to include the evidence of testing (i.e., screen recording).


The backend offers cancellation, but as far as I understand it, this PR removes the only way to access it (maybe the back button?)

Backend cancellation can still be done via the back button, as shown in the video below:

Cancellation.via.back.working.evidence.mp4

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dialog must have an explicit cancel button if you're going to remove the 'cancel on touch outside' functionality

There's a lot of code changes here, and a lot of parameters, an additional refactor to define 'options' for withProgress may be useful

@Alok-Silswal
Copy link
Contributor Author

Alok-Silswal commented Mar 5, 2026

There's a lot of code changes here, and a lot of parameters

Yes, code changes are there. But as far as the parameter is concerned, I have added only one parameter per function to disable outside touch.

For the changes, these are small recurring additions only. Nothing significant has been changed.

@Alok-Silswal
Copy link
Contributor Author

an additional refactor to define 'options' for withProgress may be useful

Yes, it may be useful if

  • there are several parameters as options, or
  • for future reference to know that this is a default value

But for only one parameter, I will prefer to keep it as a direct argument to keep the modification minimal

@david-allison How shall I go about it?

@Alok-Silswal
Copy link
Contributor Author

The dialog must have an explicit cancel button if you're going to remove the 'cancel on touch outside' functionality

Done!

Added.CANCEL.button.while.full.sync.mp4

@david-allison
Copy link
Member

This seems overly complicated

There are only 3 usages of manualCancelButton, all sync related:

Screenshot 2026-03-08 at 09 15 41

2 of which set cancelOnTouchOutside = false


For consistency:

  • remove the cancelOnTouchOutside parameter
  • keep the two new 'cancel' buttons
  • inside withProgressDialog
                if (manualCancelButton != null) {
                    setCancelable(false)
+                   setCanceledOnTouchOutside(false)

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, cheers

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. and removed Needs Author Reply Waiting for a reply from the original author Needs Review labels Mar 8, 2026
@david-allison
Copy link
Member

optional: This will be easier to merge if you

  • squash merge
  • provide a succinct commit title and description

@Alok-Silswal
Copy link
Contributor Author

Yes! I am doing that only...

@Alok-Silswal Alok-Silswal force-pushed the fix-progress-dialog-outside-touch- branch from 9f417c1 to 216101a Compare March 8, 2026 17:40
@Alok-Silswal
Copy link
Contributor Author

Alok-Silswal commented Mar 8, 2026

I earlier (maybe a month ago), by mistake ran git pull origin main on this branch due to which un-related commits messed up my branch.

While trying to do squash and merge, I by mistake did something that closed this PR and now I am not able to re-open it.

optional: This will be easier to merge if you

  • squash merge
  • provide a succinct commit title and description

Either way, with those commits this would have not been possible.

@Alok-Silswal
Copy link
Contributor Author

Alok-Silswal commented Mar 8, 2026

@david-allison I have created #20420 PR that do the same cleanly with the aforementioned feedback and same logic.

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

Labels

Needs Second Approval Has one approval, one more approval to merge squash-merge A squash & force push is required. The PR author may do this to speed up the merge process.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tapping outside of the progress box during a forced, one-way sync cancels the transfer

2 participants