diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt index 23cb83d638b7..60cbd62fb7f4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt @@ -343,12 +343,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 } /** @@ -397,15 +399,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() = @@ -450,7 +471,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi // Deck Override can change from "on" <-> "off" invalidateOptionsMenu() - enableDiscardChangesDialog() + updateDiscardChangesCallback() } override fun onKeyUp( @@ -526,13 +547,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 // ---------------------------------------------------------------------------- @@ -743,7 +759,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( @@ -775,6 +791,8 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi view.updatePadding( bottom = if (softwareKeyboardVisible) bottomInset else 0, ) + // 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. if (!templateEditor.fragmented) { val bars = insets.getInsets(systemBars() or displayCutout()) @@ -921,7 +939,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) @@ -1418,7 +1436,7 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi val currentTemplate = getCurrentTemplate() if (currentTemplate != null) { result.applyTo(currentTemplate) - templateEditor.enableDiscardChangesDialog() + templateEditor.updateDiscardChangesCallback() } } @@ -1504,7 +1522,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() @@ -1514,7 +1532,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() } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt index 5b6010644e7e..aa71df082215 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt +++ b/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(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) {