fix(card-template-editor): dismiss keyboard before 'discard changes'

AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt

@@ -341,12 +341,14 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
         }
     }
 
-    override fun onOptionsItemSelected(item: MenuItem): Boolean {
-        if (item.itemId == android.R.id.home) {
-            onBackPressedDispatcher.onBackPressed()
-            return true
+    override fun onActionBarBackPressed(): Boolean {
+        // not the back dispatcher: its callback is disabled while the keyboard is open
+        if (noteTypeHasChanged()) {
+            showDiscardChangesDialog()
+        } else {
+            finish()
         }
-        return super.onOptionsItemSelected(item)
+        return true
     }
 
     /**
@@ -395,15 +397,34 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
         if (startingOrdId != -1) {
             mainBinding.cardTemplateEditorPager.setCurrentItem(startingOrdId, animationDisabled())
         }
+        updateDiscardChangesCallback()
     }
 
     fun noteTypeHasChanged(): Boolean {
+        // insets and 'Up' can both arrive before the note type is loaded: answer them without
+        // opening the collection
+        val tempNoteType = tempNoteType ?: return false
         val oldNoteType: NotetypeJson? = getColUnsafe.notetypes.get(noteTypeId)
-        return tempNoteType != null && tempNoteType!!.notetype.toString() != oldNoteType.toString()
+        return tempNoteType.notetype.toString() != oldNoteType.toString()
     }
 
-    private fun enableDiscardChangesDialog() {
-        displayDiscardChangesCallback.isEnabled = noteTypeHasChanged()
+    /**
+     * Whether a software keyboard is open, which 'back' dismisses.
+     *
+     * The template tabs are hidden when the keyboard is open, so 'back' should reveal them.
+     */
+    private val isKeyboardOpen: Boolean
+        get() =
+            currentFragment
+                ?.takeIf { it.view != null }
+                ?.binding
+                ?.bottomNavigation
+                ?.isVisible == false
+
+    /** Updates [displayDiscardChangesCallback]. Call when the edits or the keyboard change. */
+    private fun updateDiscardChangesCallback() {
+        // 'back' dismisses the keyboard, revealing the tabs again
+        displayDiscardChangesCallback.isEnabled = !isKeyboardOpen && noteTypeHasChanged()
     }
 
     private fun showDiscardChangesDialog() =
@@ -448,7 +469,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
 
         // Deck Override can change from "on" <-> "off"
         invalidateOptionsMenu()
-        enableDiscardChangesDialog()
+        updateDiscardChangesCallback()
     }
 
     override fun onKeyUp(
@@ -524,13 +545,8 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
 
     @get:VisibleForTesting
     val currentFragment: CardTemplateFragment?
-        get() =
-            try {
-                supportFragmentManager.findFragmentByTag("f" + ord) as CardTemplateFragment?
-            } catch (e: Exception) {
-                Timber.w("Failed to get current fragment")
-                null
-            }
+        get() = templateFragment(ord)
+
     // ----------------------------------------------------------------------------
     // INNER CLASSES
     // ----------------------------------------------------------------------------
@@ -741,7 +757,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
                             }
                         refreshFragmentRunnable = updateRunnable
                         refreshFragmentHandler.postDelayed(updateRunnable, REFRESH_PREVIEW_DELAY)
-                        templateEditor.enableDiscardChangesDialog()
+                        templateEditor.updateDiscardChangesCallback()
                     }
 
                     override fun beforeTextChanged(
@@ -768,6 +784,8 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
                 // Hide the template tabs to make room for a full software keyboard. A physical
                 // keyboard can report a visible IME with only a navigation strip (or zero height).
                 binding.bottomNavigation.isVisible = insets.getInsets(ime()).bottom <= binding.bottomNavigation.minimumHeight
+                // only the selected page; a page reapplies insets on attach, so none is missed
+                if (isCurrentPage) templateEditor.updateDiscardChangesCallback()
                 // When fragmented, the activity insets the editor pane instead.
                 // The bottom navigation insets itself, so it is not padded here.
                 if (!templateEditor.fragmented) {
@@ -916,7 +934,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
                 existingNames = existingNames,
             ) { newName ->
                 template.name = newName.value
-                templateEditor.enableDiscardChangesDialog()
+                templateEditor.updateDiscardChangesCallback()
                 Timber.i("updated card template name")
                 Timber.d("updated name of template %d to '%s'", ordinal, newName)
 
@@ -1413,7 +1431,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
             val currentTemplate = getCurrentTemplate()
             if (currentTemplate != null) {
                 result.applyTo(currentTemplate)
-                templateEditor.enableDiscardChangesDialog()
+                templateEditor.updateDiscardChangesCallback()
             }
         }
 
@@ -1499,7 +1517,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
             try {
                 templateEditor.getColUnsafe.modSchema(check = true)
                 schemaChangingAction.run()
-                templateEditor.enableDiscardChangesDialog()
+                templateEditor.updateDiscardChangesCallback()
                 templateEditor.loadTemplatePreviewerFragmentIfFragmented()
             } catch (e: ConfirmModSchemaException) {
                 e.log()
@@ -1509,7 +1527,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
                     Runnable {
                         templateEditor.getColUnsafe.modSchema(check = false)
                         schemaChangingAction.run()
-                        templateEditor.enableDiscardChangesDialog()
+                        templateEditor.updateDiscardChangesCallback()
                         templateEditor.dismissAllDialogFragments()
                     }
                 val cancel = Runnable { templateEditor.dismissAllDialogFragments() }

AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt

@@ -8,6 +8,7 @@ import android.content.DialogInterface
 import android.content.Intent
 import android.os.Bundle
 import android.os.Looper
+import android.view.KeyEvent
 import android.view.View
 import android.widget.EditText
 import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -24,7 +25,9 @@ import com.ichi2.anki.previewer.CardViewerActivity
 import com.ichi2.anki.previewer.TemplatePreviewerFragment
 import com.ichi2.anki.scheduling.selectTab
 import com.ichi2.testutils.assertFalse
+import com.ichi2.testutils.dispatchInsets
 import com.ichi2.testutils.withSplitPaneUi
+import com.ichi2.utils.dp
 import org.hamcrest.CoreMatchers.equalTo
 import org.hamcrest.MatcherAssert
 import org.hamcrest.MatcherAssert.assertThat
@@ -684,6 +687,169 @@ class CardTemplateEditorTest : RobolectricTest() {
         MatcherAssert.assertThat("Deck ID element should be null", template.jsonObject["did"], Matchers.equalTo(JSONObject.NULL))
     }
 
+    @Test
+    fun `editing with the keyboard open leaves Back to the IME - Issue 21807`() {
+        withCardTemplateEditor {
+            dispatchInsets(imeBottom = 240.dp)
+            editText.text.append("edited")
+
+            assertTrue("The template has unsaved changes", noteTypeHasChanged())
+            assertFalse("Back should dismiss the keyboard", onBackPressedDispatcher.hasEnabledCallbacks())
+
+            dispatchInsets()
+            onBackPressedDispatcher.onBackPressed()
+            assertEquals("Back should now confirm discarding edits", "Discard changes?", getAlertDialogText(true))
+            assertFalse("Edits must not be discarded without confirmation", isFinishing)
+        }
+    }
+
+    @Test
+    fun `opening the keyboard after editing leaves Back to the IME - Issue 21807`() {
+        withCardTemplateEditor {
+            editText.text.append("edited")
+            assertTrue("Back should confirm discarding edits", onBackPressedDispatcher.hasEnabledCallbacks())
+
+            dispatchInsets(imeBottom = 240.dp)
+
+            assertFalse("Back should dismiss the keyboard", onBackPressedDispatcher.hasEnabledCallbacks())
+        }
+    }
+
+    /**
+     * Guards the fix for 21807: a physical keyboard reports a visible IME, but does not hide the
+     * tabs, and 'back' does not dismiss it.
+     *
+     * @see CardTemplateEditorScreenshotTest.PhysicalKeyboardIme
+     */
+    @Test
+    fun `back confirms discarding edits with a physical keyboard`() {
+        fun CardTemplateEditor.assertBackConfirmsDiscardingEdits() {
+            editText.text.append("edited")
+
+            assertTrue("Back should confirm discarding edits", onBackPressedDispatcher.hasEnabledCallbacks())
+            onBackPressedDispatcher.onBackPressed()
+
+            assertEquals("Back should confirm discarding edits", "Discard changes?", getAlertDialogText(true))
+            assertFalse("Edits must not be discarded without confirmation", isFinishing)
+            // dismiss: the next case would otherwise assert against this dialog if it showed none
+            clickAlertDialogButton(DialogInterface.BUTTON_NEGATIVE, false)
+        }
+
+        // 3-button navigation reports a visible IME with no height
+        withCardTemplateEditor {
+            dispatchInsets(navBarRight = 48.dp, imeVisible = true)
+            assertBackConfirmsDiscardingEdits()
+        }
+
+        // gesture navigation shows only a 48dp strip
+        withCardTemplateEditor {
+            dispatchInsets(navBarBottom = 24.dp, imeBottom = 48.dp)
+            assertBackConfirmsDiscardingEdits()
+        }
+    }
+
+    /**
+     * 'back' is driven by the selected page's tabs, so it must still follow the keyboard once
+     * the pager moves to another card.
+     */
+    @Test
+    fun `Back follows the keyboard after selecting another card - Issue 21807`() {
+        withCardTemplateEditor(col.notetypes.basicAndReversed) {
+            editText.text.append("edited")
+            selectTab(1)
+            advanceRobolectricLooper()
+
+            dispatchInsets(imeBottom = 240.dp)
+            assertFalse("Back should dismiss the keyboard", onBackPressedDispatcher.hasEnabledCallbacks())
+
+            dispatchInsets()
+            assertTrue("Back should confirm discarding edits", onBackPressedDispatcher.hasEnabledCallbacks())
+        }
+    }
+
+    /** Deleting a card type recreates the pages with new adapter IDs. */
+    @Test
+    fun `Back follows the keyboard after deleting a card type - Issue 21807`() {
+        withCardTemplateEditor(col.notetypes.basicAndReversed) {
+            assertTrue("Unable to delete the card type", shadowOf(this).clickMenuItem(R.id.action_delete))
+            advanceRobolectricLooper()
+            clickAlertDialogButton(DialogInterface.BUTTON_POSITIVE, true)
+            advanceRobolectricLooper()
+            assertEquals("One card type should remain", 1, tempNoteType!!.templateCount)
+            assertTrue("The deletion is unsaved", noteTypeHasChanged())
+
+            dispatchInsets(imeBottom = 240.dp)
+            assertEquals("The keyboard should hide the tabs", View.GONE, findViewById<View>(R.id.bottom_navigation).visibility)
+            assertFalse("Back should dismiss the keyboard", onBackPressedDispatcher.hasEnabledCallbacks())
+
+            dispatchInsets()
+            onBackPressedDispatcher.onBackPressed()
+            assertEquals("Back should now confirm discarding the deletion", "Discard changes?", getAlertDialogText(true))
+            assertFalse("The deletion must not be discarded without confirmation", isFinishing)
+        }
+    }
+
+    @Test
+    fun `Ctrl+2 switches to the back template after deleting a card type`() {
+        withCardTemplateEditor(col.notetypes.basicAndReversed) {
+            assertTrue("Unable to delete the card type", shadowOf(this).clickMenuItem(R.id.action_delete))
+            advanceRobolectricLooper()
+            clickAlertDialogButton(DialogInterface.BUTTON_POSITIVE, true)
+            advanceRobolectricLooper()
+            assertEquals("One card type should remain", 1, tempNoteType!!.templateCount)
+
+            val template = tempNoteType!!.getTemplate(0)
+            assertEquals("The front template should be selected initially", template.qfmt, editText.text.toString())
+
+            onKeyUp(KeyEvent.KEYCODE_2, KeyEvent(0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_2, 0, KeyEvent.META_CTRL_ON))
+
+            assertEquals("Ctrl+2 should show the back template", template.afmt, editText.text.toString())
+        }
+    }
+
+    /** The tabs live in the fragment, which is present in both layouts */
+    @Test
+    fun `editing with the keyboard open leaves Back to the IME - tablet ui - Issue 21807`() =
+        withSplitPaneUi {
+            withCardTemplateEditor(col.notetypes.basicAndReversed) {
+                editText.text.append("edited")
+                assertTrue("Back should confirm discarding edits", onBackPressedDispatcher.hasEnabledCallbacks())
+
+                dispatchInsets(imeBottom = 240.dp)
+
+                assertFalse("Back should dismiss the keyboard", onBackPressedDispatcher.hasEnabledCallbacks())
+            }
+        }
+
+    @Test
+    fun `toolbar navigation still confirms discarding edits with the keyboard open`() {
+        withCardTemplateEditor {
+            dispatchInsets(imeBottom = 240.dp)
+            editText.text.append("edited")
+
+            assertTrue("Unable to click?", shadowOf(this).clickMenuItem(android.R.id.home))
+
+            assertEquals("The toolbar should confirm discarding edits", "Discard changes?", getAlertDialogText(true))
+            assertFalse("Edits must not be discarded without confirmation", isFinishing)
+        }
+    }
+
+    @Test
+    fun `closing the keyboard after reverting edits allows Back to exit`() {
+        withCardTemplateEditor {
+            val original = editText.text.toString()
+            dispatchInsets(imeBottom = 240.dp)
+            editText.text.append("edited")
+            editText.setText(original)
+
+            dispatchInsets()
+            assertFalse("No discard confirmation is needed", onBackPressedDispatcher.hasEnabledCallbacks())
+            onBackPressedDispatcher.onBackPressed()
+
+            assertTrue("Back should close the unchanged editor", isFinishing)
+        }
+    }
+
     @Test
     fun `ensure 'Discard changes' dialog is enabled after note type changes - Issue 18518`() {
         fun assertDiscardChangesDialogShown(shadowEditor: ShadowActivity) {