Fixes #17864 : Instant card, Double Pop of Discard Dialog Box issue by MritunjayTiwari14 · Pull Request #17871 · ankidroid/Anki-Android

MritunjayTiwari14 · 2025-01-25T15:07:00Z

Purpose / Description

#17864 : Instant card, Double Pop of Discard Dialog Box issue

Fixes

Fixes Instant Card: Double "discard changes" window appears on Android "Back" swipe gesture #17864

Approach

using flag

How Has This Been Tested?

simply perform the task

Checklist

Please, go through these checks before submitting the PR.

[ x] 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
[x ] 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)
UI Changes: You have tested your change using the Google Accessibility Scanner

…x issue

MritunjayTiwari14 · 2025-01-25T15:08:44Z

@david-allison I need some guidance on how to fix the swipe gesture with back button which simultaneously pop and closes the discard dialog box without any interaction with the user

david-allison · 2025-01-25T15:23:16Z

@MritunjayTiwari14 I'm marked as busy on GitHub and won't be able to help, sorry

criticalAY · 2025-01-26T16:04:21Z

AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

+//    private val dialogBackCallback =
+//        object : OnBackPressedCallback(false) {
+//            override fun handleOnBackPressed() {
+//                showDiscardChangesDialog()
+//            }
+//        }
+
+    private var isDialogVisible = false
+


Heya! thanks for the contribution

Why this comment?
And how does it solve the issue, did you inspect the root cause?

Hello!, Sorry i forgot to remove it. I kept it as a backup and to analyze it while i wrote the new code with Flag to avoid repeated call to the Function to Display Discard Dialog Box.

…False

MritunjayTiwari14 · 2025-02-07T03:58:13Z

Can someone Please Review it? @david-allison

david-allison · 2025-02-07T04:00:22Z

Is there a simpler way to do this? This feels over complicated and likely to lead to bugs, especially for something which is such a common occurrence

MritunjayTiwari14 · 2025-02-07T04:02:25Z

Is there a simpler way to do this? This feels over complicated and likely to lead to bugs, especially for something which is such a common occurrence

Alright, looking into it!

criticalAY · 2025-02-07T22:17:23Z

AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

+                Timber.i("InstantNoteEditorActivity:: OK button pressed to confirm discard changes")
+                finish()
+            }.setOnDismissListener {
+                // Used Dismiss Listener to Update the Value of Flag each time the Dialog is Dismissed


Remove comment

criticalAY · 2025-02-07T22:17:34Z

AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

+                // Used Dismiss Listener to Update the Value of Flag each time the Dialog is Dismissed
+                isDialogVisible = false
+            }
+        isDialogVisible = true // Updating Flag as Dialog is Visible


Okay removing comments!

criticalAY · 2025-02-07T22:18:01Z

AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

-        }
+        DiscardChangesDialog
+            .showDialog(this) {
+                Timber.i("InstantNoteEditorActivity:: OK button pressed to confirm discard changes")


Either d or remove it

Okay, i will d it.

criticalAY · 2025-02-07T22:18:15Z

AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

+    private var isDialogVisible = false // Created a New Flag To Handle Double Pop up of Discard Dialog
+
    private val dialogBackCallback =
        object : OnBackPressedCallback(false) {
            override fun handleOnBackPressed() {
-                showDiscardChangesDialog()
+                if (!isDialogVisible) { // Show Discard Dialog only when it is not Visible to avoid Duplication
+                    showDiscardChangesDialog()
+                }


remove comments

criticalAY · 2025-02-07T22:20:42Z

I see there are two PR for the same issue @david-allison I am closing the latter one

…vtiy File

MritunjayTiwari14 · 2025-02-08T13:49:41Z

@criticalAY Put the SetCancelable() directly into the Activity File rather than for Whole Alertbuilder Dialog, It should be more optimized as we are changing for this Activity only and will not interfere in other places where the DiscardChangeDialog is being used otherwise.

criticalAY

Quoting David:

Is there a simpler way to do this? This feels over complicated and likely to lead to bugs, especially for something which is such a common occurrence

Please see if we can do better here

MritunjayTiwari14 · 2025-02-09T14:39:04Z

@criticalAY Please Review, the best right now we can do is to remove the Flag and use isShowing(), other than that i have searched for many forms for hours and mostly say that to handle double back press issue, setCancellable() is a standard method.

MritunjayTiwari14 · 2025-02-19T09:57:36Z

Hello???, Can anyone Please Review this? @david-allison

david-allison · 2025-02-19T16:04:52Z

@MritunjayTiwari14 My review comment still stands.

Look into the following code, and modify it such that system swipe gestures are handled as they are by other dialogs.

The problem is the root activity, not the 'discard changes' dialog

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/instantnoteeditor/InstantNoteEditorActivity.kt

Lines 590 to 599 in e9f193d

    
           private fun View.userClickOutsideDialog(exclude: View) { 
        
               setOnTouchListener { _, event -> 
        
                   if (event.action != MotionEvent.ACTION_DOWN) return@setOnTouchListener false 
        
                   if (exclude.rawHitTest(event)) { 
        
                       return@setOnTouchListener false 
        
                   } 
        
                   this@InstantNoteEditorActivity.onBackPressedDispatcher.onBackPressed() 
        
                   false 
        
               } 
        
           }

MritunjayTiwari14 · 2025-02-20T14:38:07Z

Thank you for Your Direction, i was able to analyze within moments that the onBackPressed() should be called only when the motion event detects an Action.UP not Action.DOWN and that was the reason all these bugs were arising.

@david-allison Please have a review over the code base. I reverted all the changes and updated the code with the simple fix.

MritunjayTiwari14 · 2025-02-26T14:09:27Z

Can you please check it? @david-allison

MritunjayTiwari14 · 2025-03-16T05:12:27Z

@Arthur-Milchior Can you please have a review on this?

david-allison · 2025-06-03T03:54:28Z

At an initial glance, this doesn't seem to take into account my comment:

Look into the following code, and modify it such that system swipe gestures are handled as they are by other dialogs.

github-actions · 2025-06-17T04:43:12Z

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

Fixes ankidroid#17864 : Instant card, Double Pop of Discard Dialog Bo…

3e25c58

…x issue

criticalAY self-requested a review January 26, 2025 15:18

criticalAY requested changes Jan 26, 2025

View reviewed changes

MritunjayTiwari14 added 2 commits February 4, 2025 18:24

Removed the Unneccesary Comments

1589b61

Fixed Double Pop up using Dismiss Listener and Setted Cancellable to …

34a3549

…False

MritunjayTiwari14 requested a review from criticalAY February 6, 2025 04:46

criticalAY requested changes Feb 7, 2025

View reviewed changes

criticalAY mentioned this pull request Feb 7, 2025

Prevent double Discard Changes dialog on Android back swipe gesture #17907

Closed

4 tasks

MritunjayTiwari14 added 2 commits February 8, 2025 19:13

Removed Comments

0bc022c

Removed SetCancelable from DiscardChangeDialog.kt and Added into Acti…

b378121

…vtiy File

MritunjayTiwari14 requested a review from criticalAY February 8, 2025 13:47

criticalAY reviewed Feb 8, 2025

View reviewed changes

Removed Use of Flag to Track Multiple Dialog

e9f193d

MritunjayTiwari14 requested a review from criticalAY February 9, 2025 14:40

david-allison added the Needs Author Reply Waiting for a reply from the original author label Feb 19, 2025

Reverted all the Old Changes and Applied the New Once

42d6845

MritunjayTiwari14 mentioned this pull request Mar 5, 2025

Instant Card: Double "discard changes" window appears on Android "Back" swipe gesture #17864

Closed

david-allison removed the Needs Author Reply Waiting for a reply from the original author label Mar 5, 2025

david-allison added the Needs Review label Mar 5, 2025

BrayanDSO added the Instant Editor label Mar 7, 2025

david-allison added the Needs Author Reply Waiting for a reply from the original author label Jun 3, 2025

github-actions bot added the Stale label Jun 17, 2025

github-actions bot closed this Jun 24, 2025

Uh oh!

Conversation

MritunjayTiwari14 commented Jan 25, 2025

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Uh oh!

MritunjayTiwari14 commented Jan 25, 2025

Uh oh!

david-allison commented Jan 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MritunjayTiwari14 commented Feb 7, 2025

Uh oh!

david-allison commented Feb 7, 2025

Uh oh!

MritunjayTiwari14 commented Feb 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalAY commented Feb 7, 2025

Uh oh!

MritunjayTiwari14 commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

criticalAY left a comment

Choose a reason for hiding this comment

Uh oh!

MritunjayTiwari14 commented Feb 9, 2025

Uh oh!

MritunjayTiwari14 commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

david-allison commented Feb 19, 2025

Uh oh!

MritunjayTiwari14 commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MritunjayTiwari14 commented Feb 26, 2025

Uh oh!

MritunjayTiwari14 commented Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

david-allison commented Jun 3, 2025

Uh oh!

github-actions bot commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MritunjayTiwari14 commented Feb 8, 2025 •

edited

Loading

MritunjayTiwari14 commented Feb 19, 2025 •

edited

Loading

MritunjayTiwari14 commented Feb 20, 2025 •

edited

Loading

MritunjayTiwari14 commented Mar 16, 2025 •

edited

Loading