From 91696163198cc9f11689159fc56593b0a0df9c82 Mon Sep 17 00:00:00 2001 From: Rajat Talesra Date: Tue, 22 Oct 2019 15:44:49 +0530 Subject: [PATCH] Fix #156 & #157: Continue/End Exploration player buttons- low-fi (#251) * Button test implementation * Added test cases * Nit changes * Added 1 more test case * Optimised test cases * Nit changes and KDocs * Changes to StateFragmentPresenter * Nit changes * Optimised test-cases --- .../oppia/app/player/state/StateAdapter.kt | 92 +++++++ .../player/state/StateFragmentPresenter.kt | 252 +++++++++++++++++- .../oppia/app/player/state/StateViewModel.kt | 9 +- .../itemviewmodel/StateButtonViewModel.kt | 108 ++++++++ app/src/main/res/layout/state_button_item.xml | 48 ++++ app/src/main/res/layout/state_fragment.xml | 50 +++- .../app/player/state/StateFragmentTest.kt | 212 +++++++++++++++ 7 files changed, 755 insertions(+), 16 deletions(-) create mode 100644 app/src/main/java/org/oppia/app/player/state/StateAdapter.kt create mode 100644 app/src/main/java/org/oppia/app/player/state/itemviewmodel/StateButtonViewModel.kt create mode 100644 app/src/main/res/layout/state_button_item.xml diff --git a/app/src/main/java/org/oppia/app/player/state/StateAdapter.kt b/app/src/main/java/org/oppia/app/player/state/StateAdapter.kt new file mode 100644 index 00000000000..411bbb39ab8 --- /dev/null +++ b/app/src/main/java/org/oppia/app/player/state/StateAdapter.kt @@ -0,0 +1,92 @@ +package org.oppia.app.player.state + +import android.view.LayoutInflater +import android.view.ViewGroup +import androidx.databinding.DataBindingUtil +import androidx.databinding.ViewDataBinding +import androidx.recyclerview.widget.RecyclerView +import androidx.databinding.library.baseAdapters.BR +import kotlinx.android.synthetic.main.state_button_item.view.* +import org.oppia.app.R +import org.oppia.app.player.state.itemviewmodel.StateButtonViewModel +import org.oppia.app.player.state.listener.ButtonInteractionListener +import org.oppia.app.databinding.StateButtonItemBinding + +@Suppress("unused") +private const val VIEW_TYPE_CONTENT = 1 +@Suppress("unused") +private const val VIEW_TYPE_INTERACTION_READ_ONLY = 2 +@Suppress("unused") +private const val VIEW_TYPE_NUMERIC_INPUT_INTERACTION = 3 +@Suppress("unused") +private const val VIEW_TYPE_TEXT_INPUT_INTERACTION = 4 +private const val VIEW_TYPE_STATE_BUTTON = 5 + +/** Adapter to inflate different items/views inside [RecyclerView]. The itemList consists of various ViewModels. */ +class StateAdapter( + private val itemList: MutableList, + private val buttonInteractionListener: ButtonInteractionListener +) : + RecyclerView.Adapter() { + + lateinit var stateButtonViewModel: StateButtonViewModel + + override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder { + return when (viewType) { + // TODO(#249): Generalize this binding to make adding future interactions easier. + VIEW_TYPE_STATE_BUTTON -> { + val inflater = LayoutInflater.from(parent.context) + val binding = + DataBindingUtil.inflate( + inflater, + R.layout.state_button_item, + parent, + /* attachToParent= */false + ) + StateButtonViewHolder(binding, buttonInteractionListener) + } + else -> throw IllegalArgumentException("Invalid view type") as Throwable + } + } + + override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) { + when (holder.itemViewType) { + VIEW_TYPE_STATE_BUTTON -> { + (holder as StateButtonViewHolder).bind(itemList[position] as StateButtonViewModel) + } + } + } + + override fun getItemViewType(position: Int): Int { + return when (itemList[position]) { + is StateButtonViewModel -> { + stateButtonViewModel = itemList[position] as StateButtonViewModel + VIEW_TYPE_STATE_BUTTON + } + else -> throw IllegalArgumentException("Invalid type of data $position") + } + } + + override fun getItemCount(): Int { + return itemList.size + } + + private class StateButtonViewHolder( + val binding: ViewDataBinding, + private val buttonInteractionListener: ButtonInteractionListener + ) : RecyclerView.ViewHolder(binding.root) { + internal fun bind(stateButtonViewModel: StateButtonViewModel) { + binding.setVariable(BR.buttonViewModel, stateButtonViewModel) + binding.root.interaction_button.setOnClickListener { + buttonInteractionListener.onInteractionButtonClicked() + } + binding.root.next_state_image_view.setOnClickListener { + buttonInteractionListener.onNextButtonClicked() + } + binding.root.previous_state_image_view.setOnClickListener { + buttonInteractionListener.onPreviousButtonClicked() + } + binding.executePendingBindings() + } + } +} diff --git a/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt index a1c0ea3501e..3d37856d8fd 100755 --- a/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt @@ -1,8 +1,12 @@ package org.oppia.app.player.state +import android.content.Context import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.view.inputmethod.InputMethodManager +import androidx.appcompat.app.AppCompatActivity +import androidx.databinding.ObservableField import androidx.fragment.app.Fragment import androidx.lifecycle.LiveData import androidx.lifecycle.Observer @@ -10,13 +14,19 @@ import androidx.lifecycle.Transformations import org.oppia.app.R import org.oppia.app.databinding.StateFragmentBinding import org.oppia.app.fragment.FragmentScope +import org.oppia.app.model.AnswerOutcome import org.oppia.app.model.CellularDataPreference import org.oppia.app.model.EphemeralState +import org.oppia.app.model.InteractionObject import org.oppia.app.player.audio.AudioFragment import org.oppia.app.player.audio.CellularDataDialogFragment import org.oppia.app.player.exploration.EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY +import org.oppia.app.player.exploration.ExplorationActivity +import org.oppia.app.player.state.itemviewmodel.StateButtonViewModel +import org.oppia.app.player.state.listener.ButtonInteractionListener import org.oppia.app.viewmodel.ViewModelProvider import org.oppia.domain.audio.CellularDialogController +import org.oppia.domain.exploration.ExplorationDataController import org.oppia.domain.exploration.ExplorationProgressController import org.oppia.util.data.AsyncResult import org.oppia.util.logging.Logger @@ -25,20 +35,53 @@ import javax.inject.Inject private const val TAG_CELLULAR_DATA_DIALOG = "CELLULAR_DATA_DIALOG" private const val TAG_AUDIO_FRAGMENT = "AUDIO_FRAGMENT" +private const val CONTINUE = "Continue" +private const val END_EXPLORATION = "EndExploration" +@Suppress("unused") +private const val LEARN_AGAIN = "LearnAgain" +private const val MULTIPLE_CHOICE_INPUT = "MultipleChoiceInput" +private const val ITEM_SELECT_INPUT = "ItemSelectionInput" +private const val TEXT_INPUT = "TextInput" +private const val FRACTION_INPUT = "FractionInput" +private const val NUMERIC_INPUT = "NumericInput" +private const val NUMERIC_WITH_UNITS = "NumberWithUnits" + +// For context: +// https://github.com/oppia/oppia/blob/37285a/extensions/interactions/Continue/directives/oppia-interactive-continue.directive.ts +private const val DEFAULT_CONTINUE_INTERACTION_TEXT_ANSWER = "Please continue." + /** The presenter for [StateFragment]. */ @FragmentScope class StateFragmentPresenter @Inject constructor( + private val activity: AppCompatActivity, private val fragment: Fragment, private val cellularDialogController: CellularDialogController, + private val stateButtonViewModelProvider: ViewModelProvider, private val viewModelProvider: ViewModelProvider, + private val explorationDataController: ExplorationDataController, private val explorationProgressController: ExplorationProgressController, private val logger: Logger -) { +) : ButtonInteractionListener { private var showCellularDataDialog = true private var useCellularData = false private var explorationId: String? = null + // TODO(#257): Remove this once domain layer is capable to provide this information. + private val oldStateNameList: ArrayList = ArrayList() + + private lateinit var currentEphemeralState: EphemeralState + private var currentAnswerOutcome: AnswerOutcome? = null + + private val itemList: MutableList = ArrayList() + + // TODO(#257): Remove this once domain layer is capable to provide this information. + private var hasGeneralContinueButton: Boolean = false + + private lateinit var stateAdapter: StateAdapter + + private lateinit var binding: StateFragmentBinding + fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { cellularDialogController.getCellularDataPreference() .observe(fragment, Observer> { @@ -49,7 +92,12 @@ class StateFragmentPresenter @Inject constructor( } }) - val binding = StateFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false) + stateAdapter = StateAdapter(itemList, this as ButtonInteractionListener) + + binding = StateFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false) + binding.stateRecyclerView.apply { + adapter = stateAdapter + } binding.let { it.stateFragment = fragment as StateFragment it.viewModel = getStateViewModel() @@ -121,7 +169,36 @@ class StateFragmentPresenter @Inject constructor( private fun subscribeToCurrentState() { ephemeralStateLiveData.observe(fragment, Observer { result -> - logger.d("StateFragment", "getCurrentState: ${result.state.name}") + itemList.clear() + currentEphemeralState = result + + updateDummyStateName() + + val interactionId = result.state.interaction.id + val hasPreviousState = result.hasPreviousState + var canContinueToNextState = false + hasGeneralContinueButton = false + + if (result.stateTypeCase != EphemeralState.StateTypeCase.TERMINAL_STATE) { + if (result.stateTypeCase == EphemeralState.StateTypeCase.COMPLETED_STATE + && !oldStateNameList.contains(result.state.name) + ) { + hasGeneralContinueButton = true + canContinueToNextState = false + } else if (result.completedState.answerList.size > 0 + && oldStateNameList.contains(result.state.name) + ) { + canContinueToNextState = true + hasGeneralContinueButton = false + } + } + + updateNavigationButtonVisibility( + interactionId, + hasPreviousState, + canContinueToNextState, + hasGeneralContinueButton + ) }) } @@ -139,4 +216,173 @@ class StateFragmentPresenter @Inject constructor( } return ephemeralStateResult.getOrDefault(EphemeralState.getDefaultInstance()) } + + /** + * This function listens to the result of submitAnswer. + * Whenever an answer is submitted using ExplorationProgressController.submitAnswer function, + * this function will wait for the response from that function and based on which we can move to next state. + */ + private fun subscribeToAnswerOutcome(answerOutcomeResultLiveData: LiveData>) { + val answerOutcomeLiveData = getAnswerOutcome(answerOutcomeResultLiveData) + answerOutcomeLiveData.observe(fragment, Observer { + currentAnswerOutcome = it + + // 'CONTINUE' button has two different types of functionality in different scenarios. + // If the interaction-id is 'Continue', then learner can click the 'CONTINUE' button which will submit an answer + // and move to next state. In other cases, learner submits an answer and if the answer is correct than the `SUBMIT` + // button changes to 'CONTINUE' and in that case click on 'CONTINUE' button does not submit any answer and + // directly moves to next state. + // Here, after submitting an answer it checks whether the interaction-id was 'Continue', if it is continue then move + // to next state. + if (currentEphemeralState.state.interaction.id == CONTINUE) { + moveToNextState() + } + }) + } + + /** Helper for subscribeToAnswerOutcome. */ + private fun getAnswerOutcome(answerOutcome: LiveData>): LiveData { + return Transformations.map(answerOutcome, ::processAnswerOutcome) + } + + /** Helper for subscribeToAnswerOutcome. */ + private fun processAnswerOutcome(ephemeralStateResult: AsyncResult): AnswerOutcome { + if (ephemeralStateResult.isFailure()) { + logger.e("StateFragment", "Failed to retrieve answer outcome", ephemeralStateResult.getErrorOrNull()!!) + } + return ephemeralStateResult.getOrDefault(AnswerOutcome.getDefaultInstance()) + } + + private fun endExploration() { + explorationDataController.stopPlayingExploration() + (activity as ExplorationActivity).finish() + } + + override fun onInteractionButtonClicked() { + hideKeyboard() + // TODO(#163): Remove these dummy answers and fetch answers from different interaction views. + // NB: This sample data will work only with TEST_EXPLORATION_ID_5 + // 0 -> What Language + // 2 -> Welcome! + // XX -> What Language + val stateWelcomeAnswer = 0 + // finnish -> Numeric input + // suomi -> Numeric input + // XX -> What Language + val stateWhatLanguageAnswer: String = "finnish" + // 121 -> Things You can do + // < 121 -> Estimate 100 + // > 121 -> Numeric Input + // XX -> Numeric Input + val stateNumericInputAnswer = 121 + + if (!hasGeneralContinueButton) { + val interactionObject: InteractionObject = getDummyInteractionObject() + when (currentEphemeralState.state.interaction.id) { + END_EXPLORATION -> endExploration() + CONTINUE -> subscribeToAnswerOutcome(explorationProgressController.submitAnswer(createContinueButtonAnswer())) + MULTIPLE_CHOICE_INPUT -> subscribeToAnswerOutcome( + explorationProgressController.submitAnswer( + InteractionObject.newBuilder().setNonNegativeInt( + stateWelcomeAnswer + ).build() + ) + ) + FRACTION_INPUT, + ITEM_SELECT_INPUT, + NUMERIC_INPUT, + NUMERIC_WITH_UNITS, + TEXT_INPUT -> subscribeToAnswerOutcome( + explorationProgressController.submitAnswer(interactionObject) + ) + } + } else { + moveToNextState() + } + } + + override fun onPreviousButtonClicked() { + explorationProgressController.moveToPreviousState() + } + + override fun onNextButtonClicked() { + moveToNextState() + } + + private fun moveToNextState() { + checkAndUpdateOldStateNameList() + itemList.clear() + currentAnswerOutcome = null + explorationProgressController.moveToNextState() + } + + private fun createContinueButtonAnswer(): InteractionObject { + return InteractionObject.newBuilder().setNormalizedString(DEFAULT_CONTINUE_INTERACTION_TEXT_ANSWER).build() + } + + private fun checkAndUpdateOldStateNameList() { + if (currentAnswerOutcome != null + && !currentAnswerOutcome!!.sameState + && !oldStateNameList.contains(currentEphemeralState.state.name) + ) { + oldStateNameList.add(currentEphemeralState.state.name) + } + } + + private fun updateNavigationButtonVisibility( + interactionId: String, + hasPreviousState: Boolean, + canContinueToNextState: Boolean, + hasGeneralContinueButton: Boolean + ) { + getStateButtonViewModel().setPreviousButtonVisible(hasPreviousState) + + when { + hasGeneralContinueButton -> { + getStateButtonViewModel().clearObservableInteractionId() + getStateButtonViewModel().setObservableInteractionId(CONTINUE) + } + canContinueToNextState -> { + getStateButtonViewModel().clearObservableInteractionId() + getStateButtonViewModel().setNextButtonVisible(canContinueToNextState) + } + else -> { + getStateButtonViewModel().setObservableInteractionId(interactionId) + // TODO(#163): This function controls whether the "Submit" button should be displayed or not. + // Remove this function in final implementation and control this whenever user selects some option in + // MultipleChoiceInput or InputSelectionInput. For now this is `true` because we do not have a mechanism to work + // with MultipleChoiceInput or InputSelectionInput, which will eventually be responsible for controlling this. + getStateButtonViewModel().optionSelected(true) + } + } + itemList.add(getStateButtonViewModel()) + stateAdapter.notifyDataSetChanged() + } + + private fun getStateButtonViewModel(): StateButtonViewModel { + return stateButtonViewModelProvider.getForFragment(fragment, StateButtonViewModel::class.java) + } + + private fun hideKeyboard() { + val inputManager: InputMethodManager = activity.getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager + inputManager.hideSoftInputFromWindow(fragment.view!!.windowToken, InputMethodManager.SHOW_FORCED) + } + + // TODO(#163): Remove this function, this is just for dummy testing purposes. + private fun updateDummyStateName() { + getStateViewModel().setStateName(currentEphemeralState.state.name) + } + + // TODO(#163): Remove this function and fetch this InteractionObject from [StateAdapter]. + private fun getDummyInteractionObject(): InteractionObject { + val interactionObjectBuilder: InteractionObject.Builder = InteractionObject.newBuilder() + when (currentEphemeralState.state.name) { + "Welcome!" -> interactionObjectBuilder.nonNegativeInt = 0 + "What language" -> interactionObjectBuilder.normalizedString = "finnish" + "Things you can do" -> createContinueButtonAnswer() + "Numeric input" -> interactionObjectBuilder.real = 121.0 + else -> InteractionObject.getDefaultInstance() + } + return interactionObjectBuilder.build() + } } diff --git a/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt b/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt index 9d0f34d87d6..042e2ddefb5 100644 --- a/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt +++ b/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt @@ -3,8 +3,15 @@ package org.oppia.app.player.state import androidx.databinding.ObservableField import androidx.lifecycle.ViewModel import org.oppia.app.fragment.FragmentScope +import org.oppia.app.viewmodel.ObservableViewModel import javax.inject.Inject /** [ViewModel] for state-fragment. */ @FragmentScope -class StateViewModel @Inject constructor() : ViewModel() {} +class StateViewModel @Inject constructor() : ObservableViewModel() { + var stateName = ObservableField() + + fun setStateName(state: String) { + stateName.set(state) + } +} diff --git a/app/src/main/java/org/oppia/app/player/state/itemviewmodel/StateButtonViewModel.kt b/app/src/main/java/org/oppia/app/player/state/itemviewmodel/StateButtonViewModel.kt new file mode 100644 index 00000000000..eccdeea22de --- /dev/null +++ b/app/src/main/java/org/oppia/app/player/state/itemviewmodel/StateButtonViewModel.kt @@ -0,0 +1,108 @@ +package org.oppia.app.player.state.itemviewmodel + +import android.content.Context +import android.widget.Button +import androidx.databinding.BindingAdapter +import androidx.databinding.ObservableField +import androidx.lifecycle.ViewModel +import org.oppia.app.fragment.FragmentScope +import javax.inject.Inject +import org.oppia.app.R +import org.oppia.app.viewmodel.ObservableViewModel + +private const val CONTINUE = "Continue" +private const val END_EXPLORATION = "EndExploration" +private const val LEARN_AGAIN = "LearnAgain" +private const val MULTIPLE_CHOICE_INPUT = "MultipleChoiceInput" +private const val ITEM_SELECT_INPUT = "ItemSelectionInput" +private const val TEXT_INPUT = "TextInput" +private const val FRACTION_INPUT = "FractionInput" +private const val NUMERIC_INPUT = "NumericInput" +private const val NUMERIC_WITH_UNITS = "NumberWithUnits" + +/** [ViewModel] for state-fragment. */ +@FragmentScope +class StateButtonViewModel @Inject constructor(val context: Context) : ObservableViewModel() { + companion object { + @JvmStatic + @BindingAdapter("android:button") + fun setBackgroundResource(button: Button, resource: Int) { + button.setBackgroundResource(resource) + } + } + + var isAudioFragmentVisible = ObservableField(false) + + var isNextButtonVisible = ObservableField(false) + var isPreviousButtonVisible = ObservableField(false) + + var observableInteractionId = ObservableField() + var isInteractionButtonActive = ObservableField(false) + var isInteractionButtonVisible = ObservableField(false) + var drawableResourceValue = ObservableField(R.drawable.state_button_primary_background) + + var name = ObservableField() + + fun setObservableInteractionId(interactionId: String) { + setNextButtonVisible(false) + observableInteractionId.set(interactionId) + // TODO(#249): Generalize this binding to make adding future interactions easier. + when (interactionId) { + CONTINUE -> { + isInteractionButtonActive.set(true) + isInteractionButtonVisible.set(true) + name.set(context.getString(R.string.state_continue_button)) + drawableResourceValue.set(R.drawable.state_button_primary_background) + } + END_EXPLORATION -> { + isInteractionButtonActive.set(true) + isInteractionButtonVisible.set(true) + name.set(context.getString(R.string.state_end_exploration_button)) + drawableResourceValue.set(R.drawable.state_button_primary_background) + } + LEARN_AGAIN -> { + isInteractionButtonActive.set(true) + isInteractionButtonVisible.set(true) + name.set(context.getString(R.string.state_learn_again_button)) + drawableResourceValue.set(R.drawable.state_button_blue_background) + } + ITEM_SELECT_INPUT, MULTIPLE_CHOICE_INPUT -> { + isInteractionButtonActive.set(true) + isInteractionButtonVisible.set(false) + name.set(context.getString(R.string.state_submit_button)) + drawableResourceValue.set(R.drawable.state_button_primary_background) + } + FRACTION_INPUT, NUMERIC_INPUT, NUMERIC_WITH_UNITS, TEXT_INPUT -> { + // TODO(#163): The value of isInteractionButtonVisible should be false in this case and it should be updated. + // We are keeping this true for now so that the submit button can work even without any interaction. + isInteractionButtonActive.set(true) + isInteractionButtonVisible.set(true) + name.set(context.getString(R.string.state_submit_button)) + // TODO(#163): The value of drawable should be R.drawable.state_button_transparent_background as per above explanation. + drawableResourceValue.set(R.drawable.state_button_primary_background) + } + } + } + + fun clearObservableInteractionId() { + observableInteractionId.set("") + isInteractionButtonVisible.set(false) + isInteractionButtonActive.set(false) + } + + fun setAudioFragmentVisible(isVisible: Boolean) { + isAudioFragmentVisible.set(isVisible) + } + + fun setNextButtonVisible(isVisible: Boolean) { + isNextButtonVisible.set(isVisible) + } + + fun setPreviousButtonVisible(isVisible: Boolean) { + isPreviousButtonVisible.set(isVisible) + } + + fun optionSelected(isOptionSelected: Boolean) { + isInteractionButtonVisible.set(isOptionSelected) + } +} diff --git a/app/src/main/res/layout/state_button_item.xml b/app/src/main/res/layout/state_button_item.xml new file mode 100644 index 00000000000..9f8be2da665 --- /dev/null +++ b/app/src/main/res/layout/state_button_item.xml @@ -0,0 +1,48 @@ + + + + + + + + + +