From 98099e2bd8036389df86b5b791d9b75867d2785b Mon Sep 17 00:00:00 2001 From: farees Date: Mon, 25 Jan 2021 01:12:25 +0530 Subject: [PATCH 01/14] Fixed Revealing Second hint --- .../state/StatePlayerRecyclerViewAssembler.kt | 2 +- .../app/player/state/StateFragmentTest.kt | 96 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 578cd23ff3c..978f86f80e8 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -1376,7 +1376,7 @@ class StatePlayerRecyclerViewAssembler private constructor( } } else { // See if the learner's new wrong answer justifies showing a hint. - if (isFirstHint) { + if (isFirstHint && nextUnrevealedHintIndex.hintIndex == 0) { if (wrongAnswerCount > 1) { // If more than one answer has been submitted and no hint has yet been shown, show a // hint immediately since the learner is probably stuck. diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt index 4db6235e830..b39d6df957d 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt @@ -20,6 +20,7 @@ import androidx.test.espresso.action.GeneralLocation import androidx.test.espresso.action.Press import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.action.ViewActions.closeSoftKeyboard +import androidx.test.espresso.action.ViewActions.pressBack import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.contrib.RecyclerViewActions.scrollToHolder @@ -103,6 +104,7 @@ import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.loguploader.LogUploadWorkerModule import org.oppia.android.domain.oppialogger.loguploader.WorkManagerConfigurationModule import org.oppia.android.domain.question.QuestionModule +import org.oppia.android.domain.topic.FRACTIONS_EXPLORATION_ID_0 import org.oppia.android.domain.topic.FRACTIONS_EXPLORATION_ID_1 import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_0 @@ -905,6 +907,100 @@ class StateFragmentTest { } } + @Test + fun testStateFragment_forTwoWrongAnswers_revealHint() { + launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { + startPlayingExploration() + clickContinueInteractionButton() + clickContinueInteractionButton() + clickContinueInteractionButton() + selectMultipleChoiceOption(optionPosition = 2) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + onView(withId(R.id.hints_and_solution_fragment_container)).check( + matches( + not(isDisplayed()) + ) + ) + selectMultipleChoiceOption(optionPosition = 1) + selectMultipleChoiceOption(optionPosition = 1) + onView(withId(R.id.hints_and_solution_fragment_container)).check( + matches( + isDisplayed() + ) + ) + } + } + + @Test + fun testStateFragment_forTwoWrongAnswers_revealHint_dotDisappears() { + launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { + startPlayingExploration() + clickContinueInteractionButton() + clickContinueInteractionButton() + clickContinueInteractionButton() + selectMultipleChoiceOption(optionPosition = 2) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + + selectMultipleChoiceOption(optionPosition = 1) + selectMultipleChoiceOption(optionPosition = 1) + onView(withId(R.id.dot_hint)).check( + matches( + isDisplayed() + ) + ) + onView(withId(R.id.hints_and_solution_fragment_container)).perform(click()) + onView(withId(R.id.reveal_hint_button)).perform(click()) + onView(isRoot()).perform(pressBack()) + onView(withId(R.id.dot_hint)).check( + matches( + not(isDisplayed()) + ) + ) + } + } + + @Test + fun testStateFragment_forTwoWrongAnswers_revealHint_configurationChange_newHintNotRevealed() { + launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { + startPlayingExploration() + clickContinueInteractionButton() + clickContinueInteractionButton() + clickContinueInteractionButton() + selectMultipleChoiceOption(optionPosition = 2) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + selectMultipleChoiceOption(optionPosition = 1) + clickContinueNavigationButton() + + selectMultipleChoiceOption(optionPosition = 1) + selectMultipleChoiceOption(optionPosition = 1) + onView(withId(R.id.dot_hint)).check( + matches( + isDisplayed() + ) + ) + onView(withId(R.id.hints_and_solution_fragment_container)).perform(click()) + onView(withId(R.id.reveal_hint_button)).perform(click()) + onView(isRoot()).perform(pressBack()) + rotateToLandscape() + onView(withId(R.id.dot_hint)).check( + matches( + not(isDisplayed()) + ) + ) + } + } + + @Test fun testStateFragment_forMisconception_showsLinkTextForConceptCard() { launchForExploration(FRACTIONS_EXPLORATION_ID_1).use { From eb761b7cd70641fc950ed3b030a88b1ae93b858c Mon Sep 17 00:00:00 2001 From: farees Date: Mon, 25 Jan 2021 01:19:06 +0530 Subject: [PATCH 02/14] ktlint fix --- .../java/org/oppia/android/app/player/state/StateFragmentTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt index b39d6df957d..b74bcc3a8e9 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt @@ -1000,7 +1000,6 @@ class StateFragmentTest { } } - @Test fun testStateFragment_forMisconception_showsLinkTextForConceptCard() { launchForExploration(FRACTIONS_EXPLORATION_ID_1).use { From 54c9e9e2de4c52084358ac0d8ee82648f63ad86c Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 5 Feb 2021 21:59:39 +0530 Subject: [PATCH 03/14] making HintHandler life cycle aware --- .../android/app/player/state/StateFragment.kt | 8 ++++- .../player/state/StateFragmentPresenter.kt | 12 +++++++- .../state/StatePlayerRecyclerViewAssembler.kt | 29 +++++++++++++++---- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt index 1eff9e90ff0..211732cfdcb 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt @@ -82,10 +82,16 @@ class StateFragment : internalProfileId, topicId, storyId, - explorationId + explorationId, + savedInstanceState ) } + override fun onSaveInstanceState(outState: Bundle) { + super.onSaveInstanceState(outState) + stateFragmentPresenter.handleOnSaveInstanceState(outState) + } + override fun onAnswerReadyForSubmission(answer: UserAnswer) { stateFragmentPresenter.handleAnswerReadyForSubmission(answer) } diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt index 43737cab4be..21710bca62f 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt @@ -1,6 +1,7 @@ package org.oppia.android.app.player.state import android.content.Context +import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -88,13 +89,18 @@ class StateFragmentPresenter @Inject constructor( internalProfileId: Int, topicId: String, storyId: String, - explorationId: String + explorationId: String, + savedInstanceState: Bundle? ): View? { profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build() this.topicId = topicId this.storyId = storyId this.explorationId = explorationId + savedInstanceState?.let { + recyclerViewAssembler.restoreState(savedInstanceState) + } + binding = StateFragmentBinding.inflate( inflater, container, @@ -145,6 +151,10 @@ class StateFragmentPresenter @Inject constructor( return binding.root } + fun handleOnSaveInstanceState(bundle: Bundle) { + recyclerViewAssembler.saveState(bundle) + } + fun handleAnswerReadyForSubmission(answer: UserAnswer) { // An interaction has indicated that an answer is ready for submission. handleSubmitAnswer(answer) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 978f86f80e8..c4ed7fbdb81 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -1,5 +1,6 @@ package org.oppia.android.app.player.state +import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.animation.AccelerateInterpolator @@ -92,6 +93,10 @@ private typealias AudioUiManagerRetriever = () -> AudioUiManager? /** The fragment tag corresponding to the concept card dialog fragment. */ const val CONCEPT_CARD_DIALOG_FRAGMENT_TAG = "CONCEPT_CARD_FRAGMENT" +const val KEY_TRACKED_WRONG_ANSWER_COUNT = "TRACKED_WRONG_ANSWER_COUNT" +const val KEY_PREVIOUS_HELP_INDEX = "PREVIOUS_HELP_INDEX" +const val KEY_HINT_SEQUENCE_NUMBER = "HINT_SEQUENCE_NUMBER" +const val KEY_IS_HINT_VISIBLE_IN_LATEST_STATE = "IS_HINT_VISIBLE_IN_LATEST_STATE" /** * An assembler for generating the list of view models to bind to the state player recycler view. @@ -150,6 +155,20 @@ class StatePlayerRecyclerViewAssembler private constructor( val isCorrectAnswer = ObservableField(false) + fun saveState(bundle: Bundle) { + bundle.putInt(KEY_TRACKED_WRONG_ANSWER_COUNT, hintHandler.trackedWrongAnswerCount) + // bundle.putSerializable(KEY_PREVIOUS_HELP_INDEX, hintHandler.previousHelpIndex as Serializable) + bundle.putInt(KEY_HINT_SEQUENCE_NUMBER, hintHandler.hintSequenceNumber) + bundle.putBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE, hintHandler.isHintVisibleInLatestState) + } + + fun restoreState(bundle: Bundle) { + hintHandler.trackedWrongAnswerCount = bundle.getInt(KEY_TRACKED_WRONG_ANSWER_COUNT) + // hintHandler.previousHelpIndex = bundle.get(KEY_PREVIOUS_HELP_INDEX) as HelpIndex + hintHandler.hintSequenceNumber = bundle.getInt(KEY_HINT_SEQUENCE_NUMBER) + hintHandler.isHintVisibleInLatestState = bundle.getBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE) + } + private val lifecycleSafeTimerFactory = LifecycleSafeTimerFactory(backgroundCoroutineDispatcher) private val hintHandler = HintHandler( @@ -1309,10 +1328,10 @@ class StatePlayerRecyclerViewAssembler private constructor( private val delayShowAdditionalHintsMs: Long, private val delayShowAdditionalHintsFromWrongAnswerMs: Long ) { - private var trackedWrongAnswerCount = 0 - private var previousHelpIndex: HelpIndex = HelpIndex.getDefaultInstance() - private var hintSequenceNumber = 0 - private var isHintVisibleInLatestState = false + var trackedWrongAnswerCount = 0 + var previousHelpIndex: HelpIndex = HelpIndex.getDefaultInstance() + var hintSequenceNumber = 0 + var isHintVisibleInLatestState = false /** Resets this handler to prepare it for a new state, cancelling any pending hints. */ fun reset() { @@ -1376,7 +1395,7 @@ class StatePlayerRecyclerViewAssembler private constructor( } } else { // See if the learner's new wrong answer justifies showing a hint. - if (isFirstHint && nextUnrevealedHintIndex.hintIndex == 0) { + if (isFirstHint) { if (wrongAnswerCount > 1) { // If more than one answer has been submitted and no hint has yet been shown, show a // hint immediately since the learner is probably stuck. From 4dcedf12b0289de90c1eaf64fefb4737a66b82a5 Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 5 Feb 2021 22:08:54 +0530 Subject: [PATCH 04/14] ktlint fix --- .../app/player/state/StatePlayerRecyclerViewAssembler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 7d3dfb1aac8..0db279d5b8c 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -1,7 +1,7 @@ package org.oppia.android.app.player.state -import android.os.Bundle import android.content.Context +import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.animation.AccelerateInterpolator From 96b3c482ed863b5e8d07c5cb3b791530e8166d64 Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 5 Feb 2021 23:17:34 +0530 Subject: [PATCH 05/14] NIT fix --- .../player/state/StateFragmentPresenter.kt | 8 +++---- .../state/StatePlayerRecyclerViewAssembler.kt | 22 +++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt index 3de113aa4d1..a3525fcb374 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt @@ -101,11 +101,6 @@ class StateFragmentPresenter @Inject constructor( this.topicId = topicId this.storyId = storyId this.explorationId = explorationId - - savedInstanceState?.let { - recyclerViewAssembler.restoreState(savedInstanceState) - } - binding = StateFragmentBinding.inflate( inflater, container, @@ -117,6 +112,9 @@ class StateFragmentPresenter @Inject constructor( binding.congratulationsTextConfettiView, binding.fullScreenConfettiView ) + savedInstanceState?.let { + recyclerViewAssembler.restoreState(savedInstanceState) + } val stateRecyclerViewAdapter = recyclerViewAssembler.adapter val rhsStateRecyclerViewAdapter = recyclerViewAssembler.rhsAdapter diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 0db279d5b8c..9984eecf6fc 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -765,9 +765,9 @@ class StatePlayerRecyclerViewAssembler private constructor( yPosition = height / 2, minAngle = 180.0, maxAngle = 270.0, - timeToLiveMs, + timeToLiveMs = timeToLiveMs, delayMs = 0L, - colorsList + colorsList = colorsList ) config.startConfettiBurst( confettiView, @@ -775,9 +775,9 @@ class StatePlayerRecyclerViewAssembler private constructor( yPosition = height / 2, minAngle = 270.0, maxAngle = 370.0, - timeToLiveMs, + timeToLiveMs = timeToLiveMs, delayMs = 0L, - colorsList + colorsList = colorsList ) } @@ -821,9 +821,9 @@ class StatePlayerRecyclerViewAssembler private constructor( yPosition = 0f, minAngle = -90.0, maxAngle = 90.0, - timeToLiveMillis, - delayMillis, - colorsList + timeToLiveMs = timeToLiveMillis, + delayMs = delayMillis, + colorsList = colorsList ) config.startConfettiBurst( confettiView, @@ -831,9 +831,9 @@ class StatePlayerRecyclerViewAssembler private constructor( yPosition = 0f, minAngle = 90.0, maxAngle = 270.0, - timeToLiveMillis, - delayMillis, - colorsList + timeToLiveMs = timeToLiveMillis, + delayMs = delayMillis, + colorsList = colorsList ) } @@ -1269,7 +1269,7 @@ class StatePlayerRecyclerViewAssembler private constructor( */ fun hasConversationView(hasConversationView: Boolean): Builder { this.hasConversationView = hasConversationView - featureSets += PlayerFeatureSet(showCongratulationsOnCorrectAnswer = true) + featureSets += PlayerFeatureSet(showCelebrationOnCorrectAnswer = true) return this } From 76745762108b55ac85bb57d2208fa671796092df Mon Sep 17 00:00:00 2001 From: farees Date: Sat, 6 Feb 2021 15:08:09 +0530 Subject: [PATCH 06/14] Using proto in bundle --- .../app/player/state/StatePlayerRecyclerViewAssembler.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 9984eecf6fc..6c544fe5b06 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -89,6 +89,8 @@ import org.oppia.android.databinding.SubmittedAnswerItemBinding import org.oppia.android.databinding.SubmittedAnswerListItemBinding import org.oppia.android.databinding.SubmittedHtmlAnswerItemBinding import org.oppia.android.databinding.TextInputInteractionItemBinding +import org.oppia.android.util.extensions.getProto +import org.oppia.android.util.extensions.putProto import org.oppia.android.util.parser.HtmlParser import org.oppia.android.util.threading.BackgroundDispatcher import javax.inject.Inject @@ -169,14 +171,17 @@ class StatePlayerRecyclerViewAssembler private constructor( fun saveState(bundle: Bundle) { bundle.putInt(KEY_TRACKED_WRONG_ANSWER_COUNT, hintHandler.trackedWrongAnswerCount) - // bundle.putSerializable(KEY_PREVIOUS_HELP_INDEX, hintHandler.previousHelpIndex as Serializable) + bundle.putProto(KEY_PREVIOUS_HELP_INDEX, hintHandler.previousHelpIndex) bundle.putInt(KEY_HINT_SEQUENCE_NUMBER, hintHandler.hintSequenceNumber) bundle.putBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE, hintHandler.isHintVisibleInLatestState) } fun restoreState(bundle: Bundle) { hintHandler.trackedWrongAnswerCount = bundle.getInt(KEY_TRACKED_WRONG_ANSWER_COUNT) - // hintHandler.previousHelpIndex = bundle.get(KEY_PREVIOUS_HELP_INDEX) as HelpIndex + hintHandler.previousHelpIndex = bundle.getProto( + KEY_PREVIOUS_HELP_INDEX, + HelpIndex.getDefaultInstance() + ) hintHandler.hintSequenceNumber = bundle.getInt(KEY_HINT_SEQUENCE_NUMBER) hintHandler.isHintVisibleInLatestState = bundle.getBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE) } From 0a566c3844d54b23f14ff8fc4f739ebf3311ba45 Mon Sep 17 00:00:00 2001 From: farees Date: Wed, 10 Feb 2021 14:18:41 +0530 Subject: [PATCH 07/14] Added single proto for to handle the hint related configuration changes. --- .../state/StatePlayerRecyclerViewAssembler.kt | 31 ++++++++++--------- model/src/main/proto/exploration.proto | 7 +++++ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 6c544fe5b06..7a4b728171f 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -22,8 +22,11 @@ import nl.dionsegijn.konfetti.KonfettiView import org.oppia.android.app.model.AnswerAndResponse import org.oppia.android.app.model.EphemeralState import org.oppia.android.app.model.EphemeralState.StateTypeCase +import org.oppia.android.app.model.Exploration import org.oppia.android.app.model.HelpIndex import org.oppia.android.app.model.HelpIndex.IndexTypeCase.INDEXTYPE_NOT_SET +import org.oppia.android.app.model.Hint +import org.oppia.android.app.model.HintState import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.PendingState import org.oppia.android.app.model.State @@ -99,10 +102,7 @@ private typealias AudioUiManagerRetriever = () -> AudioUiManager? /** The fragment tag corresponding to the concept card dialog fragment. */ const val CONCEPT_CARD_DIALOG_FRAGMENT_TAG = "CONCEPT_CARD_FRAGMENT" -const val KEY_TRACKED_WRONG_ANSWER_COUNT = "TRACKED_WRONG_ANSWER_COUNT" -const val KEY_PREVIOUS_HELP_INDEX = "PREVIOUS_HELP_INDEX" -const val KEY_HINT_SEQUENCE_NUMBER = "HINT_SEQUENCE_NUMBER" -const val KEY_IS_HINT_VISIBLE_IN_LATEST_STATE = "IS_HINT_VISIBLE_IN_LATEST_STATE" +const val KEY_HINT_STATE = "HINT_STATE" private const val CONGRATULATIONS_TEXT_VIEW_FADE_MILLIS: Long = 600 private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800 @@ -170,20 +170,21 @@ class StatePlayerRecyclerViewAssembler private constructor( val isCorrectAnswer = ObservableField(false) fun saveState(bundle: Bundle) { - bundle.putInt(KEY_TRACKED_WRONG_ANSWER_COUNT, hintHandler.trackedWrongAnswerCount) - bundle.putProto(KEY_PREVIOUS_HELP_INDEX, hintHandler.previousHelpIndex) - bundle.putInt(KEY_HINT_SEQUENCE_NUMBER, hintHandler.hintSequenceNumber) - bundle.putBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE, hintHandler.isHintVisibleInLatestState) + val hintState:HintState = HintState.newBuilder().apply { + wrongAnswerCount = hintHandler.trackedWrongAnswerCount + helpIndex = hintHandler.previousHelpIndex + hintSequenceNumber = hintHandler.hintSequenceNumber + isHintVisibleInLatestState = hintHandler.isHintVisibleInLatestState + }.build() + bundle.putProto(KEY_HINT_STATE, hintState) } fun restoreState(bundle: Bundle) { - hintHandler.trackedWrongAnswerCount = bundle.getInt(KEY_TRACKED_WRONG_ANSWER_COUNT) - hintHandler.previousHelpIndex = bundle.getProto( - KEY_PREVIOUS_HELP_INDEX, - HelpIndex.getDefaultInstance() - ) - hintHandler.hintSequenceNumber = bundle.getInt(KEY_HINT_SEQUENCE_NUMBER) - hintHandler.isHintVisibleInLatestState = bundle.getBoolean(KEY_IS_HINT_VISIBLE_IN_LATEST_STATE) + val hintState = bundle.getProto(KEY_HINT_STATE, HintState.newBuilder().build()) + hintHandler.trackedWrongAnswerCount = hintState.wrongAnswerCount + hintHandler.previousHelpIndex = hintState.helpIndex + hintHandler.hintSequenceNumber = hintState.hintSequenceNumber + hintHandler.isHintVisibleInLatestState = hintState.isHintVisibleInLatestState } private val lifecycleSafeTimerFactory = LifecycleSafeTimerFactory(backgroundCoroutineDispatcher) diff --git a/model/src/main/proto/exploration.proto b/model/src/main/proto/exploration.proto index 0135b2d95c0..bf2094abdf5 100644 --- a/model/src/main/proto/exploration.proto +++ b/model/src/main/proto/exploration.proto @@ -253,6 +253,13 @@ message UserAnswer { string content_description = 5; } +message HintState { + int32 wrongAnswerCount = 1; + HelpIndex helpIndex = 2; + int32 hintSequenceNumber = 3; + bool isHintVisibleInLatestState = 4; +} + message AnswerAndResponse { // A previous answer the learner submitted. UserAnswer user_answer = 1; From e8789f5c4111b6ad431bba96b133df18fb906a6d Mon Sep 17 00:00:00 2001 From: farees Date: Wed, 10 Feb 2021 14:19:54 +0530 Subject: [PATCH 08/14] Ktlint fix --- .../app/player/state/StatePlayerRecyclerViewAssembler.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 7a4b728171f..304e2660598 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -22,10 +22,8 @@ import nl.dionsegijn.konfetti.KonfettiView import org.oppia.android.app.model.AnswerAndResponse import org.oppia.android.app.model.EphemeralState import org.oppia.android.app.model.EphemeralState.StateTypeCase -import org.oppia.android.app.model.Exploration import org.oppia.android.app.model.HelpIndex import org.oppia.android.app.model.HelpIndex.IndexTypeCase.INDEXTYPE_NOT_SET -import org.oppia.android.app.model.Hint import org.oppia.android.app.model.HintState import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.PendingState @@ -170,7 +168,7 @@ class StatePlayerRecyclerViewAssembler private constructor( val isCorrectAnswer = ObservableField(false) fun saveState(bundle: Bundle) { - val hintState:HintState = HintState.newBuilder().apply { + val hintState: HintState = HintState.newBuilder().apply { wrongAnswerCount = hintHandler.trackedWrongAnswerCount helpIndex = hintHandler.previousHelpIndex hintSequenceNumber = hintHandler.hintSequenceNumber From e601ff9972a38b0d2360eb9e2971b2488b7dceac Mon Sep 17 00:00:00 2001 From: farees Date: Wed, 10 Feb 2021 15:32:11 +0530 Subject: [PATCH 09/14] protobuf fix --- model/src/main/proto/exploration.proto | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/src/main/proto/exploration.proto b/model/src/main/proto/exploration.proto index bf2094abdf5..95bb0c26065 100644 --- a/model/src/main/proto/exploration.proto +++ b/model/src/main/proto/exploration.proto @@ -254,10 +254,10 @@ message UserAnswer { } message HintState { - int32 wrongAnswerCount = 1; - HelpIndex helpIndex = 2; - int32 hintSequenceNumber = 3; - bool isHintVisibleInLatestState = 4; + int32 wrong_answer_count = 1; + HelpIndex help_index = 2; + int32 hint_sequence_number = 3; + bool is_hint_visible_in_latest_state = 4; } message AnswerAndResponse { From b63e7ce020d9df5dfa995c376ab59058e54a9be5 Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 12 Feb 2021 03:22:09 +0530 Subject: [PATCH 10/14] Resolving requested changes --- .../state/StatePlayerRecyclerViewAssembler.kt | 24 +++-- .../questionplayer/QuestionPlayerFragment.kt | 7 +- .../QuestionPlayerFragmentPresenter.kt | 15 ++- .../app/player/state/StateFragmentTest.kt | 95 ------------------- model/src/main/proto/exploration.proto | 7 -- model/src/main/proto/ui_state.proto | 20 ++++ 6 files changed, 58 insertions(+), 110 deletions(-) create mode 100644 model/src/main/proto/ui_state.proto diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 304e2660598..d49ab235725 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -24,10 +24,10 @@ import org.oppia.android.app.model.EphemeralState import org.oppia.android.app.model.EphemeralState.StateTypeCase import org.oppia.android.app.model.HelpIndex import org.oppia.android.app.model.HelpIndex.IndexTypeCase.INDEXTYPE_NOT_SET -import org.oppia.android.app.model.HintState import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.PendingState import org.oppia.android.app.model.State +import org.oppia.android.app.model.StatePlayerSavedHintState import org.oppia.android.app.model.StringList import org.oppia.android.app.model.SubtitledHtml import org.oppia.android.app.model.UserAnswer @@ -100,7 +100,7 @@ private typealias AudioUiManagerRetriever = () -> AudioUiManager? /** The fragment tag corresponding to the concept card dialog fragment. */ const val CONCEPT_CARD_DIALOG_FRAGMENT_TAG = "CONCEPT_CARD_FRAGMENT" -const val KEY_HINT_STATE = "HINT_STATE" +private const val KEY_HINT_STATE = "HINT_STATE" private const val CONGRATULATIONS_TEXT_VIEW_FADE_MILLIS: Long = 600 private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800 @@ -167,18 +167,26 @@ class StatePlayerRecyclerViewAssembler private constructor( val isCorrectAnswer = ObservableField(false) + /* + Saves the StatePlayerSavedHintState in the onSavedInstance called + in the onSaveInstanceState of the StateFragment. + */ fun saveState(bundle: Bundle) { - val hintState: HintState = HintState.newBuilder().apply { + val statePlayerSavedHintState = StatePlayerSavedHintState.newBuilder().apply { wrongAnswerCount = hintHandler.trackedWrongAnswerCount helpIndex = hintHandler.previousHelpIndex hintSequenceNumber = hintHandler.hintSequenceNumber isHintVisibleInLatestState = hintHandler.isHintVisibleInLatestState }.build() - bundle.putProto(KEY_HINT_STATE, hintState) + bundle.putProto(KEY_HINT_STATE, statePlayerSavedHintState) } + /* + Restores the StatePlayerSavedHintState in the onCreate called + in the onCreateView of the StateFragment. + */ fun restoreState(bundle: Bundle) { - val hintState = bundle.getProto(KEY_HINT_STATE, HintState.newBuilder().build()) + val hintState = bundle.getProto(KEY_HINT_STATE, StatePlayerSavedHintState.getDefaultInstance()) hintHandler.trackedWrongAnswerCount = hintState.wrongAnswerCount hintHandler.previousHelpIndex = hintState.helpIndex hintHandler.hintSequenceNumber = hintState.hintSequenceNumber @@ -306,13 +314,17 @@ class StatePlayerRecyclerViewAssembler private constructor( } } + if (isTerminalState && playerFeatureSet.showCelebrationAtEndOfSession) { + maybeShowCelebrationForEndOfSession() + } + maybeAddNavigationButtons( conversationPendingItemList, extraInteractionPendingItemList, hasPreviousState, canContinueToNextState, hasGeneralContinueButton, - ephemeralState.stateTypeCase == EphemeralState.StateTypeCase.TERMINAL_STATE + isTerminalState ) return Pair(conversationPendingItemList, extraInteractionPendingItemList) } diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt index 399a5388cbf..61802173df5 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt @@ -45,7 +45,12 @@ class QuestionPlayerFragment : container: ViewGroup?, savedInstanceState: Bundle? ): View? { - return questionPlayerFragmentPresenter.handleCreateView(inflater, container) + return questionPlayerFragmentPresenter.handleCreateView(inflater, container, savedInstanceState) + } + + override fun onSaveInstanceState(outState: Bundle) { + super.onSaveInstanceState(outState) + questionPlayerFragmentPresenter.handleOnSaveInstanceState(outState) } override fun onAnswerReadyForSubmission(answer: UserAnswer) { diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt index f5adf68904c..5445161ff54 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt @@ -1,6 +1,7 @@ package org.oppia.android.app.topic.questionplayer import android.content.Context +import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -70,7 +71,11 @@ class QuestionPlayerFragmentPresenter @Inject constructor( private lateinit var questionId: String private lateinit var currentQuestionState: State - fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { + fun handleCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle? + ): View? { binding = QuestionPlayerFragmentBinding.inflate( inflater, container, @@ -83,6 +88,10 @@ class QuestionPlayerFragmentPresenter @Inject constructor( binding.congratulationsTextConfettiView ) + savedInstanceState?.let { + recyclerViewAssembler.restoreState(savedInstanceState) + } + binding.apply { lifecycleOwner = fragment viewModel = questionViewModel @@ -105,6 +114,10 @@ class QuestionPlayerFragmentPresenter @Inject constructor( return binding.root } + fun handleOnSaveInstanceState(bundle: Bundle) { + recyclerViewAssembler.saveState(bundle) + } + fun revealHint(saveUserChoice: Boolean, hintIndex: Int) { subscribeToHint( questionAssessmentProgressController.submitHintIsRevealed( diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt index dd39a511252..9dec15c4c99 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt @@ -20,7 +20,6 @@ import androidx.test.espresso.action.GeneralLocation import androidx.test.espresso.action.Press import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.action.ViewActions.closeSoftKeyboard -import androidx.test.espresso.action.ViewActions.pressBack import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.contrib.RecyclerViewActions.scrollToHolder @@ -104,7 +103,6 @@ import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.loguploader.LogUploadWorkerModule import org.oppia.android.domain.oppialogger.loguploader.WorkManagerConfigurationModule import org.oppia.android.domain.question.QuestionModule -import org.oppia.android.domain.topic.FRACTIONS_EXPLORATION_ID_0 import org.oppia.android.domain.topic.FRACTIONS_EXPLORATION_ID_1 import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_0 @@ -907,99 +905,6 @@ class StateFragmentTest { } } - @Test - fun testStateFragment_forTwoWrongAnswers_revealHint() { - launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { - startPlayingExploration() - clickContinueInteractionButton() - clickContinueInteractionButton() - clickContinueInteractionButton() - selectMultipleChoiceOption(optionPosition = 2) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - onView(withId(R.id.hints_and_solution_fragment_container)).check( - matches( - not(isDisplayed()) - ) - ) - selectMultipleChoiceOption(optionPosition = 1) - selectMultipleChoiceOption(optionPosition = 1) - onView(withId(R.id.hints_and_solution_fragment_container)).check( - matches( - isDisplayed() - ) - ) - } - } - - @Test - fun testStateFragment_forTwoWrongAnswers_revealHint_dotDisappears() { - launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { - startPlayingExploration() - clickContinueInteractionButton() - clickContinueInteractionButton() - clickContinueInteractionButton() - selectMultipleChoiceOption(optionPosition = 2) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - - selectMultipleChoiceOption(optionPosition = 1) - selectMultipleChoiceOption(optionPosition = 1) - onView(withId(R.id.dot_hint)).check( - matches( - isDisplayed() - ) - ) - onView(withId(R.id.hints_and_solution_fragment_container)).perform(click()) - onView(withId(R.id.reveal_hint_button)).perform(click()) - onView(isRoot()).perform(pressBack()) - onView(withId(R.id.dot_hint)).check( - matches( - not(isDisplayed()) - ) - ) - } - } - - @Test - fun testStateFragment_forTwoWrongAnswers_revealHint_configurationChange_newHintNotRevealed() { - launchForExploration(FRACTIONS_EXPLORATION_ID_0).use { - startPlayingExploration() - clickContinueInteractionButton() - clickContinueInteractionButton() - clickContinueInteractionButton() - selectMultipleChoiceOption(optionPosition = 2) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - selectMultipleChoiceOption(optionPosition = 1) - clickContinueNavigationButton() - - selectMultipleChoiceOption(optionPosition = 1) - selectMultipleChoiceOption(optionPosition = 1) - onView(withId(R.id.dot_hint)).check( - matches( - isDisplayed() - ) - ) - onView(withId(R.id.hints_and_solution_fragment_container)).perform(click()) - onView(withId(R.id.reveal_hint_button)).perform(click()) - onView(isRoot()).perform(pressBack()) - rotateToLandscape() - onView(withId(R.id.dot_hint)).check( - matches( - not(isDisplayed()) - ) - ) - } - } - @Test fun testStateFragment_forMisconception_showsLinkTextForConceptCard() { launchForExploration(FRACTIONS_EXPLORATION_ID_1).use { diff --git a/model/src/main/proto/exploration.proto b/model/src/main/proto/exploration.proto index 95bb0c26065..0135b2d95c0 100644 --- a/model/src/main/proto/exploration.proto +++ b/model/src/main/proto/exploration.proto @@ -253,13 +253,6 @@ message UserAnswer { string content_description = 5; } -message HintState { - int32 wrong_answer_count = 1; - HelpIndex help_index = 2; - int32 hint_sequence_number = 3; - bool is_hint_visible_in_latest_state = 4; -} - message AnswerAndResponse { // A previous answer the learner submitted. UserAnswer user_answer = 1; diff --git a/model/src/main/proto/ui_state.proto b/model/src/main/proto/ui_state.proto new file mode 100644 index 00000000000..0fdcf8bc27f --- /dev/null +++ b/model/src/main/proto/ui_state.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; + +package model; + +option java_package = "org.oppia.android.app.model"; +option java_multiple_files = true; + +import "exploration.proto"; + +// Corresponds to HintHandler in the exploration to make it life cycle aware +message StatePlayerSavedHintState { + // Wrong answer count in the current state + int32 wrong_answer_count = 1; + // HelpIndex of the current state + HelpIndex help_index = 2; + // nth hint of the current state + int32 hint_sequence_number = 3; + // A boolean that which toggles the visibility of the hint bulb + bool is_hint_visible_in_latest_state = 4; +} From 376a9bb5de7e2b0f4edc45af7528d88e3e2a7abe Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 12 Feb 2021 17:07:40 +0530 Subject: [PATCH 11/14] Resolving requested changes --- .../state/StatePlayerRecyclerViewAssembler.kt | 17 +++++++++-------- model/src/main/proto/ui_state.proto | 10 +++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index d49ab235725..04cb403342e 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -100,7 +100,7 @@ private typealias AudioUiManagerRetriever = () -> AudioUiManager? /** The fragment tag corresponding to the concept card dialog fragment. */ const val CONCEPT_CARD_DIALOG_FRAGMENT_TAG = "CONCEPT_CARD_FRAGMENT" -private const val KEY_HINT_STATE = "HINT_STATE" +private const val KEY_HINT_STATE = "KEY_HINT_STATE" private const val CONGRATULATIONS_TEXT_VIEW_FADE_MILLIS: Long = 600 private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800 @@ -167,9 +167,10 @@ class StatePlayerRecyclerViewAssembler private constructor( val isCorrectAnswer = ObservableField(false) - /* - Saves the StatePlayerSavedHintState in the onSavedInstance called - in the onSaveInstanceState of the StateFragment. + /** + * Saves transient state that the assembler depends on. + * This should be used to retain state across configuration changes, + * and state saved through this method should be restored via [restoreState]. */ fun saveState(bundle: Bundle) { val statePlayerSavedHintState = StatePlayerSavedHintState.newBuilder().apply { @@ -181,9 +182,10 @@ class StatePlayerRecyclerViewAssembler private constructor( bundle.putProto(KEY_HINT_STATE, statePlayerSavedHintState) } - /* - Restores the StatePlayerSavedHintState in the onCreate called - in the onCreateView of the StateFragment. + /** + * Restores transient state that the assembler depends on. + * This should be used to retain state across configuration changes, + * and state saved through this method should be saved via [saveState]. */ fun restoreState(bundle: Bundle) { val hintState = bundle.getProto(KEY_HINT_STATE, StatePlayerSavedHintState.getDefaultInstance()) @@ -1285,7 +1287,6 @@ class StatePlayerRecyclerViewAssembler private constructor( */ fun hasConversationView(hasConversationView: Boolean): Builder { this.hasConversationView = hasConversationView - featureSets += PlayerFeatureSet(showCelebrationOnCorrectAnswer = true) return this } diff --git a/model/src/main/proto/ui_state.proto b/model/src/main/proto/ui_state.proto index 0fdcf8bc27f..22cf4b026e9 100644 --- a/model/src/main/proto/ui_state.proto +++ b/model/src/main/proto/ui_state.proto @@ -7,14 +7,14 @@ option java_multiple_files = true; import "exploration.proto"; -// Corresponds to HintHandler in the exploration to make it life cycle aware +// Represents savable state for the hint & solution handler in the state player. message StatePlayerSavedHintState { - // Wrong answer count in the current state + // Wrong answer count in the current state. int32 wrong_answer_count = 1; - // HelpIndex of the current state + // HelpIndex of the current state. HelpIndex help_index = 2; - // nth hint of the current state + // nth hint of the current state. int32 hint_sequence_number = 3; - // A boolean that which toggles the visibility of the hint bulb + // Whether the hint bulb is current visible. bool is_hint_visible_in_latest_state = 4; } From cae556764ee95088b2001e5b1e63203f74328e26 Mon Sep 17 00:00:00 2001 From: farees Date: Fri, 12 Feb 2021 17:49:42 +0530 Subject: [PATCH 12/14] Using defaults for the StatePlayerSavedHintState --- .../state/StatePlayerRecyclerViewAssembler.kt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 04cb403342e..74ec5297e6a 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -105,6 +105,10 @@ private const val KEY_HINT_STATE = "KEY_HINT_STATE" private const val CONGRATULATIONS_TEXT_VIEW_FADE_MILLIS: Long = 600 private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800 +private const val DEFAULT_WRONG_ANSWER_COUNT = 0 +private const val DEFAULT_HINT_SEQUENCE_NUMBER = 0 +private const val DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE = false + /** * An assembler for generating the list of view models to bind to the state player recycler view. * This class also handles some non-recycler view feature management, such as the congratulations @@ -188,7 +192,11 @@ class StatePlayerRecyclerViewAssembler private constructor( * and state saved through this method should be saved via [saveState]. */ fun restoreState(bundle: Bundle) { - val hintState = bundle.getProto(KEY_HINT_STATE, StatePlayerSavedHintState.getDefaultInstance()) + val hintState = bundle.getProto(KEY_HINT_STATE, StatePlayerSavedHintState.newBuilder().apply { + wrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT + hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER + isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE + }.build()) hintHandler.trackedWrongAnswerCount = hintState.wrongAnswerCount hintHandler.previousHelpIndex = hintState.helpIndex hintHandler.hintSequenceNumber = hintState.hintSequenceNumber @@ -1487,10 +1495,10 @@ class StatePlayerRecyclerViewAssembler private constructor( private val delayShowAdditionalHintsMs: Long, private val delayShowAdditionalHintsFromWrongAnswerMs: Long ) { - var trackedWrongAnswerCount = 0 + var trackedWrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT var previousHelpIndex: HelpIndex = HelpIndex.getDefaultInstance() - var hintSequenceNumber = 0 - var isHintVisibleInLatestState = false + var hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER + var isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE /** Resets this handler to prepare it for a new state, cancelling any pending hints. */ fun reset() { From 39814de1dabc512913a14a2aa994f5407ce7b149 Mon Sep 17 00:00:00 2001 From: farees Date: Sat, 13 Feb 2021 11:22:52 +0530 Subject: [PATCH 13/14] Resolving requested changes --- .../state/StatePlayerRecyclerViewAssembler.kt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 74ec5297e6a..dc20b6983d7 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -173,6 +173,7 @@ class StatePlayerRecyclerViewAssembler private constructor( /** * Saves transient state that the assembler depends on. + * * This should be used to retain state across configuration changes, * and state saved through this method should be restored via [restoreState]. */ @@ -188,15 +189,19 @@ class StatePlayerRecyclerViewAssembler private constructor( /** * Restores transient state that the assembler depends on. + * * This should be used to retain state across configuration changes, * and state saved through this method should be saved via [saveState]. */ fun restoreState(bundle: Bundle) { - val hintState = bundle.getProto(KEY_HINT_STATE, StatePlayerSavedHintState.newBuilder().apply { - wrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT - hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER - isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE - }.build()) + val hintState = bundle.getProto( + KEY_HINT_STATE, + StatePlayerSavedHintState.newBuilder().apply { + wrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT + hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER + isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE + }.build() + ) hintHandler.trackedWrongAnswerCount = hintState.wrongAnswerCount hintHandler.previousHelpIndex = hintState.helpIndex hintHandler.hintSequenceNumber = hintState.hintSequenceNumber From b8bc619147a8c9f347d798586c34c3c3bec77ff8 Mon Sep 17 00:00:00 2001 From: farees Date: Wed, 17 Feb 2021 11:17:28 +0530 Subject: [PATCH 14/14] Resolving requested changes --- .../player/state/StatePlayerRecyclerViewAssembler.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index dc20b6983d7..8f3b66f974e 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -107,6 +107,7 @@ private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800 private const val DEFAULT_WRONG_ANSWER_COUNT = 0 private const val DEFAULT_HINT_SEQUENCE_NUMBER = 0 +private val DEFAULT_PREVIOUS_HINT_INDEX = HelpIndex.getDefaultInstance() private const val DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE = false /** @@ -198,6 +199,7 @@ class StatePlayerRecyclerViewAssembler private constructor( KEY_HINT_STATE, StatePlayerSavedHintState.newBuilder().apply { wrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT + helpIndex = DEFAULT_PREVIOUS_HINT_INDEX hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE }.build() @@ -1501,19 +1503,19 @@ class StatePlayerRecyclerViewAssembler private constructor( private val delayShowAdditionalHintsFromWrongAnswerMs: Long ) { var trackedWrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT - var previousHelpIndex: HelpIndex = HelpIndex.getDefaultInstance() + var previousHelpIndex: HelpIndex = DEFAULT_PREVIOUS_HINT_INDEX var hintSequenceNumber = DEFAULT_HINT_SEQUENCE_NUMBER var isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE /** Resets this handler to prepare it for a new state, cancelling any pending hints. */ fun reset() { - trackedWrongAnswerCount = 0 - previousHelpIndex = HelpIndex.getDefaultInstance() + trackedWrongAnswerCount = DEFAULT_WRONG_ANSWER_COUNT + previousHelpIndex = DEFAULT_PREVIOUS_HINT_INDEX // Cancel any potential pending hints by advancing the sequence number. Note that this isn't // reset to 0 to ensure that all previous hint tasks are cancelled, and new tasks can be // scheduled without overlapping with past sequence numbers. hintSequenceNumber++ - isHintVisibleInLatestState = false + isHintVisibleInLatestState = DEFAULT_IS_HINT_VISIBLE_IN_LATEST_STATE } /** Hide hint when moving to any previous state. */