Skip to content

Commit

Permalink
NF: Remove setTitle in AbstractFlashCardViewer
Browse files Browse the repository at this point in the history
It probably should not be `NF`, but here it is. Because each time that
`setTitle` is called, `updateActionBar` is called immediately after and change
the title anyway.

In practice `setTitle` was setting a wrong title in the Reviewer, as it was
using current deck instead of the name of the current card.

Worse, while moving access to `col` to coroutine, I realized that this created
a race condition, where the title changed. That was catched by automated test,
so that's great actually.

We can also decide that actually, we wanted to keep `Preview` as title all the
time, and current behavior is a bug. In which case I suggest we move
`updateActionBar` to `Reviewer` and ensure it does not deal with title anymore.
  • Loading branch information
Arthur-Milchior authored and mikehardy committed Jun 22, 2023
1 parent ca18b6a commit 0b4c6cb
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,6 @@ abstract class AbstractFlashcardViewer :
refreshActionBar()
}

protected abstract fun setTitle()

// Finish initializing the activity after the collection has been correctly loaded
public override fun onCollectionLoaded(col: Collection) {
super.onCollectionLoaded(col)
Expand Down Expand Up @@ -601,7 +599,6 @@ abstract class AbstractFlashcardViewer :
registerExternalStorageListener()
restoreCollectionPreferences(col)
initLayout()
setTitle()
mHtmlGenerator = createInstance(this, typeAnswer!!, mBaseUrl!!)

// Initialize text-to-speech. This is an asynchronous operation.
Expand Down Expand Up @@ -631,7 +628,6 @@ abstract class AbstractFlashcardViewer :
}
automaticAnswer.enable()
// Reset the activity title
setTitle()
updateActionBar()
selectNavigationItem(-1)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ open class CardTemplatePreviewer : AbstractFlashcardViewer() {
closeCardTemplatePreviewer()
}

override fun setTitle() {
if (supportActionBar != null) {
supportActionBar?.setTitle(R.string.preview_title)
}
}

override fun initLayout() {
super.initLayout()
topBarLayout!!.visibility = View.GONE
Expand Down
4 changes: 0 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/anki/Previewer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,6 @@ class Previewer : AbstractFlashcardViewer() {
throw IllegalStateException("newCardList was empty")
}

override fun setTitle() {
supportActionBar!!.setTitle(R.string.preview_title)
}

override fun initLayout() {
super.initLayout()
topBarLayout!!.visibility = View.GONE
Expand Down
12 changes: 0 additions & 12 deletions AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,6 @@ open class Reviewer :
col.sched.deferReset()
}

override fun setTitle() {
val title: String = if (colIsOpen()) {
Decks.basename(col.decks.current().getString("name"))
} else {
Timber.e("Could not set title in reviewer because collection closed")
""
}
supportActionBar!!.title = title
super.setTitle(title)
supportActionBar!!.subtitle = ""
}

override fun getContentViewAttr(fullscreenMode: FullScreenMode): Int {
return when (fullscreenMode) {
FullScreenMode.BUTTONS_ONLY -> R.layout.reviewer_fullscreen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,6 @@ class AbstractFlashcardViewerCommandTest : RobolectricTest() {
currentCardOverride = card
}

override fun setTitle() {
// Intentionally blank
}

override fun performReload() {
// intentionally blank
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ class AbstractFlashcardViewerKeyboardInputTest : RobolectricTest() {
}
}

override fun setTitle() {
// required for interface. Intentionally left blank
}

fun focusTextField() {
mFocusTextField = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class AbstractFlashcardViewerTest : RobolectricTest() {
class NonAbstractFlashcardViewer : AbstractFlashcardViewer() {
var answered: Int? = null
private var mLastTime = 0
override fun setTitle() {}
override fun performReload() {
// intentionally blank
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import org.mockito.ArgumentMatchers
import org.mockito.Mockito
import org.mockito.kotlin.whenever
import timber.log.Timber
import java.lang.Exception

@RunWith(AndroidJUnit4::class)
class ReviewerKeyboardInputTest : RobolectricTest() {
Expand Down Expand Up @@ -330,11 +329,6 @@ class ReviewerKeyboardInputTest : RobolectricTest() {
whenever(keyEvent.isAltPressed).thenReturn(false)
return keyEvent
}

override fun setTitle() {
// required for interface. Intentionally left blank
}

fun focusTextField(): KeyboardInputTestReviewer {
mFocusTextField = true
return this
Expand Down

0 comments on commit 0b4c6cb

Please sign in to comment.