Skip to content

Commit

Permalink
Fix part of oppia#4470, Fix oppia#4471, Fix 4474: Handle configuratio…
Browse files Browse the repository at this point in the history
…n change using onSavedInstance. (oppia#5458)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix part of oppia#4470. 
Fixes oppia#4471.
Fixes oppia#4474

This PR enables the retention of input when the device configuration
changes using onSavedInstance.

List of interactions covered in this PR:
1. FractionInputInteraction
2. NumericInputInteraction
3. TextInputInteraction
4. MathExpressionInteraction
5. RatioExpressionInteraction


List of interactions not covered in this PR:
1. Image Region selection interaction.
2. Drag and Drop interaction.

<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
  • Loading branch information
Vishwajith-Shettigar authored Aug 2, 2024
1 parent 1b72693 commit 7568bd3
Show file tree
Hide file tree
Showing 28 changed files with 631 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.oppia.android.app.fragment.InjectableFragment
import org.oppia.android.app.model.HelpIndex
import org.oppia.android.app.model.StateFragmentArguments
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand Down Expand Up @@ -42,6 +43,9 @@ class StateFragment :
/** Arguments key for StateFragment. */
const val STATE_FRAGMENT_ARGUMENTS_KEY = "StateFragment.arguments"

/** Arguments key for StateFragment saved state. */
const val STATE_FRAGMENT_STATE_KEY = "StateFragment.state"

/**
* Creates a new instance of a StateFragment.
* @param internalProfileId used by StateFragment to mark progress.
Expand Down Expand Up @@ -86,6 +90,12 @@ class StateFragment :
): View? {
val args =
arguments?.getProto(STATE_FRAGMENT_ARGUMENTS_KEY, StateFragmentArguments.getDefaultInstance())

val userAnswerState = savedInstanceState?.getProto(
STATE_FRAGMENT_STATE_KEY,
UserAnswerState.getDefaultInstance()
) ?: UserAnswerState.getDefaultInstance()

val internalProfileId = args?.internalProfileId ?: -1
val topicId = args?.topicId!!
val storyId = args.storyId!!
Expand All @@ -97,7 +107,8 @@ class StateFragment :
internalProfileId,
topicId,
storyId,
explorationId
explorationId,
userAnswerState
)
}

Expand Down Expand Up @@ -154,4 +165,12 @@ class StateFragment :
fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard()

fun getExplorationCheckpointState() = stateFragmentPresenter.getExplorationCheckpointState()

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
outState.putProto(
STATE_FRAGMENT_STATE_KEY,
stateFragmentPresenter.getUserAnswerState()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.State
import org.oppia.android.app.model.SurveyQuestionName
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.player.audio.AudioButtonListener
import org.oppia.android.app.player.audio.AudioFragment
import org.oppia.android.app.player.audio.AudioUiManager
Expand Down Expand Up @@ -111,7 +112,8 @@ class StateFragmentPresenter @Inject constructor(
internalProfileId: Int,
topicId: String,
storyId: String,
explorationId: String
explorationId: String,
userAnswerState: UserAnswerState
): View? {
profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
this.topicId = topicId
Expand All @@ -125,7 +127,7 @@ class StateFragmentPresenter @Inject constructor(
/* attachToRoot= */ false
)
recyclerViewAssembler = createRecyclerViewAssembler(
assemblerBuilderFactory.create(resourceBucketName, entityType, profileId),
assemblerBuilderFactory.create(resourceBucketName, entityType, profileId, userAnswerState),
binding.congratulationsTextView,
binding.congratulationsTextConfettiView,
binding.fullScreenConfettiView
Expand Down Expand Up @@ -373,6 +375,7 @@ class StateFragmentPresenter @Inject constructor(
private fun subscribeToAnswerOutcome(
answerOutcomeResultLiveData: LiveData<AsyncResult<AnswerOutcome>>
) {
recyclerViewAssembler.resetUserAnswerState()
val answerOutcomeLiveData = getAnswerOutcome(answerOutcomeResultLiveData)
answerOutcomeLiveData.observe(
fragment,
Expand All @@ -393,6 +396,11 @@ class StateFragmentPresenter @Inject constructor(
)
}

/** Returns the [UserAnswerState] representing the user's current pending answer. */
fun getUserAnswerState(): UserAnswerState {
return stateViewModel.getUserAnswerState(recyclerViewAssembler::getPendingAnswerHandler)
}

/** Helper for subscribeToAnswerOutcome. */
private fun getAnswerOutcome(
answerOutcome: LiveData<AsyncResult<AnswerOutcome>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.StringList
import org.oppia.android.app.model.SubtitledHtml
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.player.audio.AudioUiManager
import org.oppia.android.app.player.state.StatePlayerRecyclerViewAssembler.Builder.Factory
Expand Down Expand Up @@ -143,7 +144,8 @@ class StatePlayerRecyclerViewAssembler private constructor(
backgroundCoroutineDispatcher: CoroutineDispatcher,
private val hasConversationView: Boolean,
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController
private val translationController: TranslationController,
private var userAnswerState: UserAnswerState
) : HtmlParser.CustomOppiaTagActionListener {
/**
* A list of view models corresponding to past view models that are hidden by default. These are
Expand Down Expand Up @@ -323,10 +325,16 @@ class StatePlayerRecyclerViewAssembler private constructor(
hasPreviousButton,
isSplitView.get()!!,
writtenTranslationContext,
timeToStartNoticeAnimationMs
timeToStartNoticeAnimationMs,
userAnswerState
)
}

/** Reset userAnswerState once the user submits an answer. */
fun resetUserAnswerState() {
userAnswerState = UserAnswerState.getDefaultInstance()
}

private fun addContentItem(
pendingItemList: MutableList<StateItemViewModel>,
ephemeralState: EphemeralState,
Expand Down Expand Up @@ -904,7 +912,8 @@ class StatePlayerRecyclerViewAssembler private constructor(
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController,
private val multiTypeBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory,
private val singleTypeBuilderFactory: BindableAdapter.SingleTypeBuilder.Factory
private val singleTypeBuilderFactory: BindableAdapter.SingleTypeBuilder.Factory,
private val userAnswerState: UserAnswerState
) {

private val adapterBuilder: BindableAdapter.MultiTypeBuilder<StateItemViewModel,
Expand Down Expand Up @@ -1390,7 +1399,8 @@ class StatePlayerRecyclerViewAssembler private constructor(
backgroundCoroutineDispatcher,
hasConversationView,
resourceHandler,
translationController
translationController,
userAnswerState
)
if (playerFeatureSet.conceptCardSupport) {
customTagListener.proxyListener = assembler
Expand All @@ -1416,7 +1426,12 @@ class StatePlayerRecyclerViewAssembler private constructor(
* Returns a new [Builder] for the specified GCS resource bucket information for loading
* assets, and the current logged in [ProfileId].
*/
fun create(resourceBucketName: String, entityType: String, profileId: ProfileId): Builder {
fun create(
resourceBucketName: String,
entityType: String,
profileId: ProfileId,
userAnswerState: UserAnswerState
): Builder {
return Builder(
accessibilityService,
htmlParserFactory,
Expand All @@ -1430,7 +1445,8 @@ class StatePlayerRecyclerViewAssembler private constructor(
resourceHandler,
translationController,
multiAdapterBuilderFactory,
singleAdapterFactory
singleAdapterFactory,
userAnswerState
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import androidx.lifecycle.Observer
import androidx.lifecycle.Transformations
import androidx.lifecycle.ViewModel
import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.EphemeralState
import org.oppia.android.app.model.OppiaLanguage
import org.oppia.android.app.model.Profile
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.model.WrittenTranslationLanguageSelection
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel
import org.oppia.android.app.viewmodel.ObservableArrayList
Expand Down Expand Up @@ -150,6 +151,13 @@ class StateViewModel @Inject constructor(
)
}

fun getUserAnswerState(
retrieveAnswerHandler: (List<StateItemViewModel>) -> InteractionAnswerHandler?
): UserAnswerState {
return retrieveAnswerHandler(getAnswerItemList())?.getUserAnswerState()
?: UserAnswerState.getDefaultInstance()
}

private fun getPendingAnswerWithoutError(
answerHandler: InteractionAnswerHandler?
): UserAnswer? {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.oppia.android.app.player.state.answerhandling

import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState

/**
* A handler for interaction answers. Handlers can either require an additional user action before the answer can be
Expand All @@ -26,6 +28,11 @@ interface InteractionAnswerHandler {
fun getPendingAnswer(): UserAnswer? {
return null
}

/** Returns the current pending answer. */
fun getUserAnswerState(): UserAnswerState {
return UserAnswerState.getDefaultInstance()
}
}

/**
Expand All @@ -35,11 +42,3 @@ interface InteractionAnswerHandler {
interface InteractionAnswerReceiver {
fun onAnswerReadyForSubmission(answer: UserAnswer)
}

/** Categories of errors that can be inferred from a pending answer. */
enum class AnswerErrorCategory {
/** Corresponds to errors that may be found while the user is trying to input an answer. */
REAL_TIME,
/** Corresponds to errors that may be found only when a user tries to submit an answer. */
SUBMIT_TIME
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import androidx.fragment.app.Fragment
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
Expand Down Expand Up @@ -57,7 +58,8 @@ class ContinueInteractionViewModel private constructor(
hasPreviousButton: Boolean,
isSplitView: Boolean,
writtenTranslationContext: WrittenTranslationContext,
timeToStartNoticeAnimationMs: Long?
timeToStartNoticeAnimationMs: Long?,
userAnswerState: UserAnswerState
): StateItemViewModel {
return ContinueInteractionViewModel(
interactionAnswerReceiver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.databinding.Observable
import androidx.databinding.ObservableField
import androidx.recyclerview.widget.RecyclerView
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.ListOfSetsOfHtmlStrings
Expand All @@ -14,8 +15,8 @@ import org.oppia.android.app.model.StringList
import org.oppia.android.app.model.SubtitledHtml
import org.oppia.android.app.model.TranslatableHtmlContentId
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand Down Expand Up @@ -163,6 +164,7 @@ class DragAndDropSortInteractionViewModel private constructor(
AnswerErrorCategory.REAL_TIME -> null
AnswerErrorCategory.SUBMIT_TIME ->
getSubmitTimeError().getErrorMessageFromStringRes(resourceHandler)
else -> null
}
errorMessage.set(pendingAnswerError)
return pendingAnswerError
Expand Down Expand Up @@ -250,7 +252,8 @@ class DragAndDropSortInteractionViewModel private constructor(
hasPreviousButton: Boolean,
isSplitView: Boolean,
writtenTranslationContext: WrittenTranslationContext,
timeToStartNoticeAnimationMs: Long?
timeToStartNoticeAnimationMs: Long?,
userAnswerState: UserAnswerState
): StateItemViewModel {
return DragAndDropSortInteractionViewModel(
entityId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import android.text.TextWatcher
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.parser.FractionParsingUiError
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand All @@ -27,10 +28,12 @@ class FractionInteractionViewModel private constructor(
private val errorOrAvailabilityCheckReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver,
private val writtenTranslationContext: WrittenTranslationContext,
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController
private val translationController: TranslationController,
userAnswerState: UserAnswerState
) : StateItemViewModel(ViewType.FRACTION_INPUT_INTERACTION), InteractionAnswerHandler {
private var pendingAnswerError: String? = null
var answerText: CharSequence = ""
var answerText: CharSequence = userAnswerState.textInputAnswer
private var answerErrorCetegory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR
var isAnswerAvailable = ObservableField<Boolean>(false)
var errorMessage = ObservableField<String>("")

Expand All @@ -54,6 +57,7 @@ class FractionInteractionViewModel private constructor(
/* pendingAnswerError= */null,
/* inputAnswerAvailable= */true
)
checkPendingAnswerError(userAnswerState.answerErrorCategory)
}

override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply {
Expand All @@ -69,6 +73,7 @@ class FractionInteractionViewModel private constructor(

/** It checks the pending error for the current fraction input, and correspondingly updates the error string based on the specified error category. */
override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
answerErrorCetegory = category
when (category) {
AnswerErrorCategory.REAL_TIME -> {
if (answerText.isNotEmpty()) {
Expand All @@ -86,6 +91,7 @@ class FractionInteractionViewModel private constructor(
fractionParser.getSubmitTimeError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
}
else -> {}
}
errorMessage.set(pendingAnswerError)
return pendingAnswerError
Expand All @@ -110,6 +116,13 @@ class FractionInteractionViewModel private constructor(
}
}

override fun getUserAnswerState(): UserAnswerState {
return UserAnswerState.newBuilder().apply {
this.textInputAnswer = answerText.toString()
this.answerErrorCategory = answerErrorCetegory
}.build()
}

private fun deriveHintText(interaction: Interaction): CharSequence {
// The subtitled unicode can apparently exist in the structure in two different formats.
val placeholderUnicodeOption1 =
Expand Down Expand Up @@ -149,7 +162,8 @@ class FractionInteractionViewModel private constructor(
hasPreviousButton: Boolean,
isSplitView: Boolean,
writtenTranslationContext: WrittenTranslationContext,
timeToStartNoticeAnimationMs: Long?
timeToStartNoticeAnimationMs: Long?,
userAnswerState: UserAnswerState
): StateItemViewModel {
return FractionInteractionViewModel(
interaction,
Expand All @@ -158,7 +172,8 @@ class FractionInteractionViewModel private constructor(
answerErrorReceiver,
writtenTranslationContext,
resourceHandler,
translationController
translationController,
userAnswerState
)
}
}
Expand Down
Loading

0 comments on commit 7568bd3

Please sign in to comment.