Skip to content

Fix ExportReady dialog async handling#21018

Open
lukstbit wants to merge 2 commits into
ankidroid:mainfrom
lukstbit:refactor_export_dialog_handler_code
Open

Fix ExportReady dialog async handling#21018
lukstbit wants to merge 2 commits into
ankidroid:mainfrom
lukstbit:refactor_export_dialog_handler_code

Conversation

@lukstbit
Copy link
Copy Markdown
Member

Purpose / Description

Fixes(IMO) the issue where the ExportReadyDialog that was shown in response to an export request was sometimes shown(and crashed) when the original activity was sent to the background and a new one was at the top.

I didn't figure out how this happens but you can kind of simulate this with the changes below in BackendExporting.kt:

diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt
index d2c0b2f682..b691344af6 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt
@@ -13,6 +13,7 @@
  */
 package com.ichi2.anki
 
+import android.content.Intent
 import anki.import_export.ExportLimit
 import com.ichi2.anki.CollectionManager.TR
 import com.ichi2.anki.CollectionManager.withCol
@@ -20,6 +21,7 @@ import com.ichi2.anki.libanki.Collection
 import com.ichi2.anki.libanki.exportAnkiPackage
 import com.ichi2.anki.libanki.exportCardsCsv
 import com.ichi2.anki.libanki.exportNotesCsv
+import kotlinx.coroutines.delay
 import net.ankiweb.rsdroid.exceptions.BackendInvalidInputException
 
 fun AnkiActivity.exportApkgPackage(
@@ -41,6 +43,8 @@ fun AnkiActivity.exportApkgPackage(
                 exportAnkiPackage(exportPath, withScheduling, withDeckConfigs, withMedia, limit, legacy)
             }
         }
+        startActivity(Intent(this@exportApkgPackage, CardBrowser::class.java))
+        delay(2000)
         ankiBaseViewModel.registerExportReadyRequest(exportPath)
     }
 }

and using the export apkg feature.

Note: I added the new AnkiViewModel in a base package, didn't figure out a better location, feel free to propose a different place.

Fixes

How Has This Been Tested?

Checked exporting things, ran tests.

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

Copy link
Copy Markdown
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 request for changes is one consideration on showSimpleNotification, as I don't think the VIewModel will be retained when starting a new activity.

We'll be tackling base and where the dialogs go in the next few months in the multimodule re-architecture work, so I'm happy with this for the moment.


100% support in general - Message has been awful since I joined. I love this.

The following is a total nitpick which keeps AnkiActivity lean. Kill it with fire if you don't like it.

  • moves implementation into ExportReadyDialog
    • This would be an extension on FragmentActivity, but showSimpleNotification needs thought
  • uses filterNotNull() so we don't have a null parameter
  • AnkiViewModel -> ExportReadyViewModel
  • Copyright change is optional, your discretion
Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt	(revision 61aa721b323fde311628d402e4d32bc9e0e3a7d5)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt	(date 1778543194703)
@@ -63,7 +63,7 @@
 import com.ichi2.anki.android.input.ShortcutGroup
 import com.ichi2.anki.android.input.ShortcutGroupProvider
 import com.ichi2.anki.android.input.shortcut
-import com.ichi2.anki.base.AnkiViewModel
+import com.ichi2.anki.dialogs.viewmodel.ExportReadyViewModel
 import com.ichi2.anki.common.annotations.LegacyNotifications
 import com.ichi2.anki.common.annotations.NeedsTest
 import com.ichi2.anki.common.crashreporting.CrashReportService
@@ -75,9 +75,9 @@
 import com.ichi2.anki.dialogs.DatabaseErrorDialog.CustomExceptionData
 import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType
 import com.ichi2.anki.dialogs.DialogHandler
-import com.ichi2.anki.dialogs.ExportReadyDialog
 import com.ichi2.anki.dialogs.ExportReadyDialog.Companion.ARG_SHARE_AS_TEXT
 import com.ichi2.anki.dialogs.ExportReadyDialog.Companion.KEY_EXPORT_PATH
+import com.ichi2.anki.dialogs.handleExportReadyRequest
 import com.ichi2.anki.dialogs.ExportReadyDialog.Companion.REQUEST_EXPORT_SAVE
 import com.ichi2.anki.dialogs.ExportReadyDialog.Companion.REQUEST_EXPORT_SHARE
 import com.ichi2.anki.dialogs.SimpleMessageDialog
@@ -96,13 +96,12 @@
 import com.ichi2.themes.Themes
 import com.ichi2.utils.AdaptionUtil
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.io.File
 import java.io.FileOutputStream
-import kotlin.onFailure
-import kotlin.runCatching
 import androidx.browser.customtabs.CustomTabsIntent.Builder as CustomTabsIntentBuilder
 
 @UiThread
@@ -111,7 +110,7 @@
 ) : AppCompatActivity(contentLayoutId ?: 0),
     ShortcutGroupProvider,
     AnkiActivityProvider {
-    val ankiBaseViewModel by viewModels<AnkiViewModel>()
+    val exportReadyViewModel by viewModels<ExportReadyViewModel>()
 
     /**
      * Receiver that informs us when a broadcast listen in [broadcastsActions] is received.
@@ -170,7 +169,9 @@
 
         lifecycleScope.launch {
             repeatOnLifecycle(Lifecycle.State.RESUMED) {
-                ankiBaseViewModel.exportReadyDestination.collect(::handleExportReadyRequest)
+                exportReadyViewModel.exportReadyDestination.filterNotNull().collect { params ->
+                    handleExportReadyRequest(exportReadyViewModel, params)
+                }
             }
         }
 
@@ -723,35 +724,6 @@
         super.onSaveInstanceState(outState)
     }
 
-    private fun handleExportReadyRequest(params: AnkiViewModel.ExportReadyParams?) {
-        // null means there's no pending export ready dialog
-        if (params == null) return
-        runCatching {
-            Timber.d("Trying to show the export ready dialog...")
-            val dialog = ExportReadyDialog.newInstance(params.exportPath, params.asText)
-            dialog.show(supportFragmentManager, "ExportReadyDialog")
-        }.onFailure { exception ->
-            if (exception is IllegalStateException) {
-                Timber.w(
-                    exception,
-                    "failed to show fragment, activity is likely paused. Sending notification",
-                )
-                // showing the dialog has failed so we show this notification, this request to show
-                // the dialog is stored in the ViewModel and will be shown when the activity resumes
-                showSimpleNotification(
-                    getString(R.string.export_ready_title),
-                    null,
-                    Channel.GENERAL,
-                )
-            } else {
-                throw exception
-            }
-        }.onSuccess {
-            Timber.d("Showing the export ready was successful, clearing any stored references...")
-            ankiBaseViewModel.clearExportReadyRequest()
-        }
-    }
-
     @NeedsTest("#20993 verify that the proper mime type is used for the share intent")
     private fun shareFile(
         path: String,
Index: AnkiDroid/src/main/java/com/ichi2/anki/base/AnkiViewModel.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/base/AnkiViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/viewmodel/ExportReadyViewModel.kt
rename from AnkiDroid/src/main/java/com/ichi2/anki/base/AnkiViewModel.kt
rename to AnkiDroid/src/main/java/com/ichi2/anki/dialogs/viewmodel/ExportReadyViewModel.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/base/AnkiViewModel.kt	(revision 61aa721b323fde311628d402e4d32bc9e0e3a7d5)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/viewmodel/ExportReadyViewModel.kt	(date 1778543194689)
@@ -1,31 +1,17 @@
-/*
- * Copyright (c) 2026 lukstbit <lukstbit@users.noreply.github.com>
- *
- * This program is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free Software
- * Foundation; either version 3 of the License, or (at your option) any later
- * version.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
- * PARTICULAR PURPOSE. See the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see <http://www.gnu.org/licenses/>.
- */
+// SPDX-FileCopyrightText: 2026 lukstbit <lukstbit@users.noreply.github.com>
+// SPDX-License-Identifier: GPL-3.0-or-later
 
-package com.ichi2.anki.base
+package com.ichi2.anki.dialogs.viewmodel
 
 import android.os.Parcelable
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.ViewModel
-import com.ichi2.anki.AnkiActivity
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.parcelize.Parcelize
 import timber.log.Timber
 
-/** [ViewModel] used by the base [AnkiActivity] class */
-class AnkiViewModel(
+/** [androidx.lifecycle.ViewModel] used by the base [com.ichi2.anki.AnkiActivity] class */
+class ExportReadyViewModel(
     private val savedStateHandle: SavedStateHandle,
 ) : ViewModel() {
     val exportReadyDestination: StateFlow<ExportReadyParams?>
@@ -35,12 +21,12 @@
         exportPath: String,
         asText: Boolean = false,
     ) {
-        Timber.d("Export ready dialog requested")
+        Timber.Forest.d("Export ready dialog requested")
         savedStateHandle[ARG_EXPORT_READY_PARAMS] = ExportReadyParams(exportPath, asText)
     }
 
     fun clearExportReadyRequest() {
-        Timber.d("Clearing export ready dialog request")
+        Timber.Forest.d("Clearing export ready dialog request")
         savedStateHandle[ARG_EXPORT_READY_PARAMS] = null
     }
 
@@ -53,4 +39,4 @@
     companion object {
         private const val ARG_EXPORT_READY_PARAMS = "arg_export_ready_params"
     }
-}
+}
\ No newline at end of file
Index: AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ExportReadyDialog.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ExportReadyDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ExportReadyDialog.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ExportReadyDialog.kt	(revision 61aa721b323fde311628d402e4d32bc9e0e3a7d5)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/ExportReadyDialog.kt	(date 1778543194681)
@@ -20,9 +20,13 @@
 import androidx.appcompat.app.AlertDialog
 import androidx.core.os.bundleOf
 import androidx.fragment.app.DialogFragment
+import com.ichi2.anki.AnkiActivity
+import com.ichi2.anki.Channel
 import com.ichi2.anki.R
+import com.ichi2.anki.dialogs.viewmodel.ExportReadyViewModel
 import com.ichi2.utils.negativeButton
 import com.ichi2.utils.positiveButton
+import timber.log.Timber
 
 class ExportReadyDialog : DialogFragment() {
     private val exportPath
@@ -71,3 +75,34 @@
         }
     }
 }
+
+internal fun AnkiActivity.handleExportReadyRequest(
+    viewModel: ExportReadyViewModel,
+    params: ExportReadyViewModel.ExportReadyParams,
+) {
+    runCatching {
+        Timber.d("Trying to show the export ready dialog...")
+        val dialog = ExportReadyDialog.newInstance(params.exportPath, params.asText)
+        dialog.show(supportFragmentManager, "ExportReadyDialog")
+    }.onFailure { exception ->
+        if (exception is IllegalStateException) {
+            Timber.w(
+                exception,
+                "failed to show fragment, activity is likely paused. Sending notification",
+            )
+            // showing the dialog has failed so we show this notification, this request to show
+            // the dialog is stored in the ViewModel and will be shown when the activity resumes
+            showSimpleNotification(
+                getString(R.string.export_ready_title),
+                null,
+                Channel.GENERAL,
+            )
+        } else {
+            throw exception
+        }
+    }.onSuccess {
+        Timber.d("Showing the export ready was successful, clearing any stored references...")
+        viewModel.clearExportReadyRequest()
+    }
+}
+
Index: AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt	(revision 61aa721b323fde311628d402e4d32bc9e0e3a7d5)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendExporting.kt	(date 1778541955399)
@@ -39,7 +39,7 @@
         withProgress(extractProgress = onProgress) {
             withCol { exportAnkiPackage(exportPath, withScheduling, withDeckConfigs, withMedia, limit, legacy) }
         }
-        ankiBaseViewModel.registerExportReadyRequest(exportPath)
+        exportReadyViewModel.registerExportReadyRequest(exportPath)
     }
 }
 
@@ -57,7 +57,7 @@
         withProgress(extractProgress = onProgress) {
             withCol { exportCollectionPackage(exportPath, withMedia, legacy) }
         }
-        ankiBaseViewModel.registerExportReadyRequest(exportPath)
+        exportReadyViewModel.registerExportReadyRequest(exportPath)
     }
 }
 
@@ -89,7 +89,7 @@
                 )
             }
         }
-        ankiBaseViewModel.registerExportReadyRequest(exportPath, asText = true)
+        exportReadyViewModel.registerExportReadyRequest(exportPath, asText = true)
     }
 }
 
@@ -109,7 +109,7 @@
                 exportCardsCsv(exportPath, withHtml, limit)
             }
         }
-        ankiBaseViewModel.registerExportReadyRequest(exportPath, asText = true)
+        exportReadyViewModel.registerExportReadyRequest(exportPath, asText = true)
     }
 }

Comment on lines +741 to +745
showSimpleNotification(
getString(R.string.export_ready_title),
null,
Channel.GENERAL,
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐉 showSimpleNotification uses an intent to launch the DeckPicker when tapped. This won't work for other activities

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, I missed this completely, also had the notifications disabled on the app on the emulator 😕 .

This would complicate the implementation quite a bit and I'm not sure about the value at this moment. Would you accept to eliminate the notification for now and create an issue for it(current code I pushed)?

With Gsoc we are going to change the UI and my understanding is that this will leave us with just one activity(so a simplification for this issue)?!

Also, looking at the export UI we probably should refactor the extra step, looks verbose to create a temp file with the export and after this ask the user what he wants to do with it.

Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt Outdated
Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt Outdated
Comment thread AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt Outdated
@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label May 11, 2026
Comment thread AnkiDroid/src/main/java/com/ichi2/anki/base/AnkiViewModel.kt Outdated
@lukstbit lukstbit force-pushed the refactor_export_dialog_handler_code branch from 61aa721 to 97ea1fc Compare May 15, 2026 08:32
lukstbit added 2 commits May 15, 2026 11:35
Switch to using a ViewModel in the base class AnkiActivity which
manages the exporting code(as we have two activities that can export,
DeckPicker and CardBrowser).

Instead of trying to show the dialog and on failure post a message on
the main Looper, the request to show the dialog is registered in the
ViewModel of AnkiActivity and from there it's passed to the actual
activity.

If showing the dialog works then we clear the request, if not, the
dialog data is still stored in the ViewModel and will be proposed
again to the activity once it comes back.
Replaces old generic copyright with new version.
Adds copyright for both developers that produces the modern version
of the file.
@lukstbit lukstbit force-pushed the refactor_export_dialog_handler_code branch from 97ea1fc to 1348d24 Compare May 15, 2026 08:35
@lukstbit
Copy link
Copy Markdown
Member Author

@david-allison See the second commit message about the copyright.

Copy link
Copy Markdown
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.

Looks awesome, thanks!

Could you split out the Todo into an issue: it'll likely be a common pattern

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author Needs Review labels May 15, 2026
* Restoration + handling is performed in [AnkiActivity.onResume].
* It is assumed that the [DeckPicker] will be the inheritor of AnkiActivity at this time.
* As this is provided as the intent from [AnkiActivity.showSimpleNotification]
* As this is provided as the intent from [AnkiActivity.showExportReadyNotification]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AnkiActivity.showExportReadyNotification doesn't exist. Should this still reference showSimpleNotification, or should the entire sentence be removed since the export notification was dropped in this PR?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants