Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1737: Singleton for exploration to handle input for configuration changes #2514

Closed
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,18 @@ class StateFragment :
fun revealSolution() = stateFragmentPresenter.revealSolution()

fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard()

override fun onResume() {
super.onResume()
stateFragmentPresenter.handleOnResume()
}

override fun onDestroyView() {
super.onDestroyView()
stateFragmentPresenter.handleDestroyView()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.oppia.android.app.player.state

import android.content.Context
import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
Expand Down Expand Up @@ -32,6 +33,7 @@ import org.oppia.android.app.utility.SplitScreenManager
import org.oppia.android.app.viewmodel.ViewModelProvider
import org.oppia.android.databinding.StateFragmentBinding
import org.oppia.android.domain.exploration.ExplorationProgressController
import org.oppia.android.domain.state.RetriveUserAnswer
import org.oppia.android.domain.topic.StoryProgressController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
Expand Down Expand Up @@ -154,6 +156,7 @@ class StateFragmentPresenter @Inject constructor(
viewModel.setHintBulbVisibility(false)
hideKeyboard()
moveToNextState()
RetriveUserAnswer.clearUserAnswer()
}

fun onNextButtonClicked() = moveToNextState()
Expand Down Expand Up @@ -521,4 +524,18 @@ class StateFragmentPresenter @Inject constructor(
Date().time
)
}

fun handleOnResume() {
RetriveUserAnswer.getUserAnswer()?.let {
viewModel.setPendingAnswer(it, recyclerViewAssembler::getPendingAnswerHandler)
Log.d("testSingleton", "userAnswer -> ${it.answer}")
Log.d("testSingleton", "userAnswer -> $it")
}
}

fun handleDestroyView() {
RetriveUserAnswer.setUserAnswer(
viewModel.getPendingAnswer(recyclerViewAssembler::getPendingAnswerHandler)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ class StateViewModel @Inject constructor() : ObservableViewModel() {
) ?: UserAnswer.getDefaultInstance()
}

fun setPendingAnswer(
userAnswer: UserAnswer,
retrieveAnswerHandler: (List<StateItemViewModel>) -> InteractionAnswerHandler?
) {
retrieveAnswerHandler(getAnswerItemList())?.setPendingAnswer(userAnswer)
}

private fun getPendingAnswerWithoutError(
answerHandler: InteractionAnswerHandler?
): UserAnswer? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ interface InteractionAnswerHandler {
fun getPendingAnswer(): UserAnswer? {
return null
}

fun setPendingAnswer(userAnswer: UserAnswer)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class ContinueInteractionViewModel(
.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

fun handleButtonClicked() {
interactionAnswerReceiver.onAnswerReadyForSubmission(getPendingAnswer())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class DragAndDropSortInteractionViewModel(
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

/** Returns an HTML list containing all of the HTML string elements as items in the list. */
private fun convertItemsToAnswer(htmlItems: List<StringList>): ListOfSetsOfHtmlStrings {
return ListOfSetsOfHtmlStrings.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.oppia.android.app.player.state.itemviewmodel
import android.content.Context
import android.text.Editable
import android.text.TextWatcher
import android.util.Log
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
Expand All @@ -23,7 +24,7 @@ class FractionInteractionViewModel(
private val interactionAnswerErrorOrAvailabilityCheckReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver // ktlint-disable max-line-length
) : StateItemViewModel(ViewType.FRACTION_INPUT_INTERACTION), InteractionAnswerHandler {
private var pendingAnswerError: String? = null
var answerText: CharSequence = ""
var answerText = ObservableField<String>("")
var isAnswerAvailable = ObservableField<Boolean>(false)
var errorMessage = ObservableField<String>("")

Expand All @@ -36,7 +37,7 @@ class FractionInteractionViewModel(
override fun onPropertyChanged(sender: Observable, propertyId: Int) {
interactionAnswerErrorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck(
pendingAnswerError,
answerText.isNotEmpty()
answerText.get()!!.isNotEmpty()
)
}
}
Expand All @@ -46,8 +47,8 @@ class FractionInteractionViewModel(

override fun getPendingAnswer(): UserAnswer {
val userAnswerBuilder = UserAnswer.newBuilder()
if (answerText.isNotEmpty()) {
val answerTextString = answerText.toString()
if (answerText.get()!!.isNotEmpty()) {
val answerTextString = answerText.get().toString()
userAnswerBuilder.answer = InteractionObject.newBuilder()
.setFraction(stringToFractionParser.parseFractionFromString(answerTextString))
.build()
Expand All @@ -56,19 +57,24 @@ class FractionInteractionViewModel(
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
answerText.set(userAnswer.plainAnswer)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenHenning Here I'm unable to update the View using this method is there any other way to do this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be specific on what isn’t working? It’s sometimes hard to tell from just the code. The more specific you can be the better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm able to send the userAnswer (pending answer after configuration change) to the FractionInteractionViewModel but i'm unable to update it using the ObservableField for answerText (updated it string -> ObservableField to update the Input through viewmodel). Here I'm able to get the value but I'm unable to update the FractionInteractionView.

Is the implementation correct
Is there any other way to update the FractionInteractionView using the ViewModel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Just to check: did you confirm:

  • setPendingAnswer is being called here
  • setPendingAnswer is being passed new data
  • The current value of answerText when setPendingAnswer is called isn't what you expect (e.g. empty)

If all are true, I'm not exactly sure what's wrong. observable fields should work out of the box, but you could try calling notifyChange on the view model (per https://github.com/oppia/oppia-android/blob/develop/app/src/main/java/org/oppia/android/app/viewmodel/ObservableViewModel.kt#L23). FWIW I think you can actually avoid observable field if you use notifyChange, but maybe start with both to reduce the number of things that could be wrong here.

Copy link
Contributor

@rt4914 rt4914 Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this:
answerText is already set to the value which is coming from setPendingAnswer but the FractionInteractionView is not displaying it.
Also, notifyChange() did not work.
I think the issues is something else because answerText is showing the correct value its just not visible in screen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I'm wondering if this is behaving this way because we have a text watcher set. It might be the case that we aren't actually copying over the value to answerText directly from databinding (& instead rely on the text watcher). I won't have time to dig into this right now, but @FareesHussain I suggest searching for whether a text watcher can interfere with databinding.

Log.d("testSingleton", "i've set the value finally it is ${userAnswer.plainAnswer}")
}

/** 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? {
if (answerText.isNotEmpty()) {
if (answerText.get()!!.isNotEmpty()) {
when (category) {
AnswerErrorCategory.REAL_TIME ->
pendingAnswerError =
stringToFractionParser.getRealTimeAnswerError(answerText.toString())
stringToFractionParser.getRealTimeAnswerError(answerText.get().toString())
.getErrorMessageFromStringRes(
context
)
AnswerErrorCategory.SUBMIT_TIME ->
pendingAnswerError =
stringToFractionParser.getSubmitTimeError(answerText.toString())
stringToFractionParser.getSubmitTimeError(answerText.get().toString())
.getErrorMessageFromStringRes(
context
)
Expand All @@ -84,12 +90,14 @@ class FractionInteractionViewModel(
}

override fun onTextChanged(answer: CharSequence, start: Int, before: Int, count: Int) {
answerText = answer.toString().trim()
val isAnswerTextAvailable = answerText.isNotEmpty()
if (isAnswerTextAvailable != isAnswerAvailable.get()) {
isAnswerAvailable.set(isAnswerTextAvailable)
if (answer.isNotEmpty()) {
answerText.set(answer.toString().trim())
val isAnswerTextAvailable = answerText.get()!!.isNotEmpty()
if (isAnswerTextAvailable != isAnswerAvailable.get()) {
isAnswerAvailable.set(isAnswerTextAvailable)
}
checkPendingAnswerError(AnswerErrorCategory.REAL_TIME)
}
checkPendingAnswerError(AnswerErrorCategory.REAL_TIME)
}

override fun afterTextChanged(s: Editable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ class ImageRegionSelectionInteractionViewModel(
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

private fun parseClickOnImage(answerTextString: String): ClickOnImage {
val region = selectableRegions.find { it.label == answerTextString }
return ClickOnImage.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,8 @@ class NumericInputViewModel(
}
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class RatioExpressionInputInteractionViewModel(
private val numberOfTerms =
interaction.customizationArgsMap["numberOfTerms"]?.signedInt ?: 0

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

init {
val callback: Observable.OnPropertyChangedCallback =
object : Observable.OnPropertyChangedCallback() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class SelectionInteractionViewModel(
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

/** Returns an HTML list containing all of the HTML string elements as items in the list. */
private fun convertSelectedItemsToHtmlString(htmlItems: Collection<String>): String {
return when (htmlItems.size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ class TextInputViewModel(
return userAnswerBuilder.build()
}

override fun setPendingAnswer(userAnswer: UserAnswer) {
// TODO("Not yet implemented")
}

private fun deriveHintText(interaction: Interaction): CharSequence {
// The default placeholder for text input is empty.
return interaction.customizationArgsMap["placeholder"]?.subtitledUnicode?.unicodeStr ?: ""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.oppia.android.domain.state

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

object RetriveUserAnswer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a Dagger singleton (see other @Singleton marked classes in the codebase).

// text and error message for fraction [FractionInteractionView.kt]
// list of check boxes as a string and back to check box [SelectionInteractionView.kt]
// text and error msg for [NumericInputInteractionView.kt]
// text and error message for [RatioInputInteractionView.kt]
// text for [TextInputInteractionView.kt]
// list for drag & drop [DragDropSortInteractionView.kt]
// list for drag & drop with merging [DragDropSortInteractionView.kt]
// selected img in the [ImageRegionSelectionInteractionView.kt]

private var userAnswer: UserAnswer? = null

fun setUserAnswer(solution: UserAnswer) {
this.userAnswer = solution
}
fun getUserAnswer(): UserAnswer? = userAnswer

fun clearUserAnswer() {
userAnswer = null
}
}