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 #379: Collapse past wrong answers #412

Merged
merged 15 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -7,6 +7,7 @@ import android.view.ViewGroup
import android.view.inputmethod.InputMethodManager
import androidx.appcompat.app.AppCompatActivity
import androidx.databinding.DataBindingUtil
import androidx.databinding.ObservableBoolean
import androidx.fragment.app.Fragment
import androidx.lifecycle.LiveData
import androidx.lifecycle.Observer
Expand All @@ -19,6 +20,7 @@ import org.oppia.app.databinding.ContinueInteractionItemBinding
import org.oppia.app.databinding.FeedbackItemBinding
import org.oppia.app.databinding.FractionInteractionItemBinding
import org.oppia.app.databinding.NumericInputInteractionItemBinding
import org.oppia.app.databinding.PreviousResponsesHeaderItemBinding
import org.oppia.app.databinding.SelectionInteractionItemBinding
import org.oppia.app.databinding.StateButtonItemBinding
import org.oppia.app.databinding.StateFragmentBinding
Expand All @@ -41,12 +43,14 @@ import org.oppia.app.player.state.itemviewmodel.FeedbackViewModel
import org.oppia.app.player.state.itemviewmodel.FractionInteractionViewModel
import org.oppia.app.player.state.itemviewmodel.InteractionViewModelFactory
import org.oppia.app.player.state.itemviewmodel.NumericInputViewModel
import org.oppia.app.player.state.itemviewmodel.PreviousResponsesHeaderViewModel
import org.oppia.app.player.state.itemviewmodel.SelectionInteractionViewModel
import org.oppia.app.player.state.itemviewmodel.StateItemViewModel
import org.oppia.app.player.state.itemviewmodel.StateNavigationButtonViewModel
import org.oppia.app.player.state.itemviewmodel.StateNavigationButtonViewModel.ContinuationNavigationButtonType
import org.oppia.app.player.state.itemviewmodel.SubmittedAnswerViewModel
import org.oppia.app.player.state.itemviewmodel.TextInputViewModel
import org.oppia.app.player.state.listener.PreviousResponsesHeaderClickListener
import org.oppia.app.player.state.listener.StateNavigationButtonListener
import org.oppia.app.recyclerview.BindableAdapter
import org.oppia.app.viewmodel.ViewModelProvider
Expand Down Expand Up @@ -77,7 +81,7 @@ class StateFragmentPresenter @Inject constructor(
private val htmlParserFactory: HtmlParser.Factory,
private val context: Context,
private val interactionViewModelFactoryMap: Map<String, @JvmSuppressWildcards InteractionViewModelFactory>
) : StateNavigationButtonListener {
) : StateNavigationButtonListener, PreviousResponsesHeaderClickListener {

private var showCellularDataDialog = true
private var useCellularData = false
Expand All @@ -89,6 +93,18 @@ class StateFragmentPresenter @Inject constructor(
private val ephemeralStateLiveData: LiveData<AsyncResult<EphemeralState>> by lazy {
explorationProgressController.getCurrentState()
}
/**
* A list of view models corresponding to past view models that are hidden by default. These are intentionally not
* retained upon configuration changes since the user can just re-expand the list. Note that the first element of this
* list (when initialized), will always be the previous answers header to help locate the items in the recycler view
* (when present).
*/
private val previousAnswerViewModels: MutableList<StateItemViewModel> = mutableListOf()
/**
* Whether the previously submitted wrong answers should be expanded. This value is intentionally not retained upon
* configuration changes since the user can just re-expand the list.
*/
private var hasPreviousResponsesExpanded: Boolean = false

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
cellularDialogController.getCellularDataPreference()
Expand Down Expand Up @@ -204,6 +220,12 @@ class StateFragmentPresenter @Inject constructor(
}
}
)
.registerViewDataBinder(
viewType = StateItemViewModel.ViewType.PREVIOUS_RESPONSES_HEADER,
inflateDataBinding = PreviousResponsesHeaderItemBinding::inflate,
setViewModel = PreviousResponsesHeaderItemBinding::setViewModel,
transformViewModel = { it as PreviousResponsesHeaderViewModel }
)
.build()
}

Expand Down Expand Up @@ -297,6 +319,7 @@ class StateFragmentPresenter @Inject constructor(
val scrollToTop = ::currentStateName.isInitialized && currentStateName != ephemeralState.state.name

currentStateName = ephemeralState.state.name
previousAnswerViewModels.clear() // But retain whether the list is currently open.
val pendingItemList = mutableListOf<StateItemViewModel>()
addContentItem(pendingItemList, ephemeralState)
val interaction = ephemeralState.state.interaction
Expand Down Expand Up @@ -391,8 +414,14 @@ class StateFragmentPresenter @Inject constructor(

override fun onNextButtonClicked() = moveToNextState()

override fun onResponsesHeaderClicked() {
togglePreviousAnswers()
}

private fun moveToNextState() {
explorationProgressController.moveToNextState()
explorationProgressController.moveToNextState().observe(fragment, Observer<AsyncResult<Any?>> {
hasPreviousResponsesExpanded = false
})
}

private fun addInteractionForPendingState(
Expand All @@ -412,21 +441,67 @@ class StateFragmentPresenter @Inject constructor(
private fun addPreviousAnswers(
pendingItemList: MutableList<StateItemViewModel>, answersAndResponses: List<AnswerAndResponse>
) {
for (answerAndResponse in answersAndResponses) {
addSubmittedAnswer(pendingItemList, answerAndResponse.userAnswer)
addFeedbackItem(pendingItemList, answerAndResponse.feedback)
if (answersAndResponses.size > 1) {
PreviousResponsesHeaderViewModel(
answersAndResponses.size - 1, ObservableBoolean(hasPreviousResponsesExpanded), this
).let { viewModel ->
pendingItemList += viewModel
previousAnswerViewModels += viewModel
}
// Only add previous answers if current responses are expanded.
for (answerAndResponse in answersAndResponses.take(answersAndResponses.size - 1)) {
createSubmittedAnswer(answerAndResponse.userAnswer).let { viewModel ->
if (hasPreviousResponsesExpanded) {
pendingItemList += viewModel
}
previousAnswerViewModels += viewModel
}
createFeedbackItem(answerAndResponse.feedback)?.let { viewModel ->
if (hasPreviousResponsesExpanded) {
pendingItemList += viewModel
}
previousAnswerViewModels += viewModel
}
}
}
answersAndResponses.lastOrNull()?.let { answerAndResponse ->
pendingItemList += createSubmittedAnswer(answerAndResponse.userAnswer)
createFeedbackItem(answerAndResponse.feedback)?.let(pendingItemList::add)
}
}

/**
* Toggles whether the previous answers should be shown based on the current state stored in
* [PreviousResponsesHeaderViewModel].
*/
private fun togglePreviousAnswers() {
val headerModel = previousAnswerViewModels.first() as PreviousResponsesHeaderViewModel
val expandPreviousAnswers = !headerModel.isExpanded.get()
val headerIndex = viewModel.itemList.indexOf(headerModel)
val previousAnswersAndFeedbacks = previousAnswerViewModels.takeLast(previousAnswerViewModels.size - 1)
if (expandPreviousAnswers) {
// Add the pending view models to the recycler view to expand them.
viewModel.itemList.addAll(headerIndex + 1, previousAnswersAndFeedbacks)
} else {
// Remove the pending view models to collapse the list.
viewModel.itemList.removeAll(previousAnswersAndFeedbacks)
}
// Ensure the header matches the updated state.
headerModel.isExpanded.set(expandPreviousAnswers)
hasPreviousResponsesExpanded = expandPreviousAnswers
recyclerViewAdapter.notifyDataSetChanged()
}

private fun addSubmittedAnswer(pendingItemList: MutableList<StateItemViewModel>, userAnswer: UserAnswer) {
pendingItemList += SubmittedAnswerViewModel(userAnswer)
private fun createSubmittedAnswer(userAnswer: UserAnswer): SubmittedAnswerViewModel {
return SubmittedAnswerViewModel(userAnswer)
}

private fun addFeedbackItem(pendingItemList: MutableList<StateItemViewModel>, feedback: SubtitledHtml) {
private fun createFeedbackItem(feedback: SubtitledHtml): FeedbackViewModel? {
// Only show feedback if there's some to show.
if (feedback.html.isNotEmpty()) {
pendingItemList += FeedbackViewModel(feedback.html)
return FeedbackViewModel(feedback.html)
}
return null
}

private fun updateNavigationButtonVisibility(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package org.oppia.app.player.state

import androidx.databinding.ObservableArrayList
import androidx.databinding.ObservableField
import androidx.databinding.ObservableList
import androidx.lifecycle.ViewModel
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.UserAnswer
import org.oppia.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.app.player.state.itemviewmodel.StateItemViewModel
import org.oppia.app.viewmodel.ObservableArrayList
import org.oppia.app.viewmodel.ObservableViewModel
import javax.inject.Inject

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.oppia.app.player.state.itemviewmodel

import androidx.databinding.ObservableBoolean
import org.oppia.app.player.state.listener.PreviousResponsesHeaderClickListener

/** [ViewModel] for the header of the section of previously submitted answers. */
class PreviousResponsesHeaderViewModel(
val previousAnswerCount: Int, var isExpanded: ObservableBoolean,
private val previousResponsesHeaderClickListener: PreviousResponsesHeaderClickListener
) : StateItemViewModel(ViewType.PREVIOUS_RESPONSES_HEADER) {
/** Called when the user clicks on the previous response header. */
fun onResponsesHeaderClicked() = previousResponsesHeaderClickListener.onResponsesHeaderClicked()
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.oppia.app.player.state.itemviewmodel

import androidx.databinding.ObservableArrayList
import androidx.databinding.ObservableList
import org.oppia.app.model.Interaction
import org.oppia.app.model.InteractionObject
Expand All @@ -9,6 +8,7 @@ import org.oppia.app.model.UserAnswer
import org.oppia.app.player.state.SelectionItemInputType
import org.oppia.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.app.player.state.answerhandling.InteractionAnswerReceiver
import org.oppia.app.viewmodel.ObservableArrayList

/** ViewModel for multiple or item-selection input choice list. */
class SelectionInteractionViewModel(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ abstract class StateItemViewModel(val viewType: ViewType) : ObservableViewModel(
NUMERIC_INPUT_INTERACTION,
TEXT_INPUT_INTERACTION,
SUBMITTED_ANSWER,
PREVIOUS_RESPONSES_HEADER
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.oppia.app.player.state.listener

/** Listener for when the previous response header is clicked. */
interface PreviousResponsesHeaderClickListener {
/** Called when the user clicks on the response header. */
fun onResponsesHeaderClicked()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package org.oppia.app.viewmodel

import androidx.databinding.ListChangeRegistry
import androidx.databinding.ObservableList

/** A version of Android's ObservableArrayList, except with correct Kotlin overrides. */
class ObservableArrayList<T> : ArrayList<T>(), ObservableList<T> {
private val listeners: ListChangeRegistry by lazy(::ListChangeRegistry)

override fun add(element: T): Boolean {
super.add(element)
notifyAdd(size - 1, 1)
return true
}

override fun add(index: Int, element: T) {
super.add(index, element)
notifyAdd(index, 1)
}

override fun addAll(elements: Collection<T>): Boolean {
val oldSize = size
val added = super.addAll(elements)
if (added) {
notifyAdd(oldSize, size - oldSize)
}
return added
}

override fun addAll(index: Int, elements: Collection<T>): Boolean {
val added = super.addAll(index, elements)
if (added) {
notifyAdd(index, elements.size)
}
return added
}

override fun removeAll(elements: Collection<T>): Boolean {
val firstIndex = elements.firstOrNull()?.let { indexOf(it) } ?: -1
val removed = super.removeAll(elements)
if (removed && firstIndex > -1) {
notifyRemove(firstIndex, elements.size)
}
return removed
}

override fun clear() {
val oldSize = size
super.clear()
if (oldSize != 0) {
notifyRemove(0, oldSize)
}
}

override fun set(index: Int, element: T): T {
val `val` = super.set(index, element)
listeners.notifyChanged(this, index, 1)
return `val`
}

override fun removeRange(fromIndex: Int, toIndex: Int) {
super.removeRange(fromIndex, toIndex)
notifyRemove(fromIndex, toIndex - fromIndex)
}

override fun addOnListChangedCallback(callback: ObservableList.OnListChangedCallback<out ObservableList<T>>?) {
listeners.add(callback)
}

override fun removeOnListChangedCallback(callback: ObservableList.OnListChangedCallback<out ObservableList<T>>?) {
listeners.remove(callback)
}

private fun notifyAdd(start: Int, count: Int) {
listeners.notifyInserted(this, start, count)
}

private fun notifyRemove(start: Int, count: Int) {
listeners.notifyRemoved(this, start, count)
}
}
9 changes: 9 additions & 0 deletions app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:fillColor="#FF333333"
android:pathData="M7,10l5,5 5,-5z" />
</vector>
9 changes: 9 additions & 0 deletions app/src/main/res/drawable/ic_arrow_right_grey_24dp.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:fillColor="#FF333333"
android:pathData="M10,17l5,-5 -5,-5v10z"/>
</vector>
59 changes: 59 additions & 0 deletions app/src/main/res/layout/previous_responses_header_item.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">

<data>

<variable
name="viewModel"
type="org.oppia.app.player.state.itemviewmodel.PreviousResponsesHeaderViewModel" />
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="16dp"
android:clickable="true"
android:focusable="true"
android:minHeight="48dp"
android:paddingTop="16dp"
android:paddingBottom="16dp"
android:paddingStart="20dp"
android:paddingEnd="16dp"
android:onClick="@{(v) -> viewModel.onResponsesHeaderClicked()}">

<FrameLayout
android:layout_width="0dp"
android:layout_height="1dp"
android:background="@color/mid_grey"
app:layout_constraintBottom_toBottomOf="@+id/previous_responses_header_text"
app:layout_constraintEnd_toStartOf="@+id/previous_responses_header_text"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="@+id/previous_responses_header_text" />

<TextView
android:id="@+id/previous_responses_header_text"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:drawableEnd="@{viewModel.isExpanded ? @drawable/ic_arrow_down_grey_24dp : @drawable/ic_arrow_right_grey_24dp}"
android:fontFamily="sans-serif-medium"
android:padding="8dp"
android:text="@{@string/previous_responses_header(viewModel.previousAnswerCount)}"
android:textAllCaps="true"
android:textColor="@color/mid_grey"
android:textSize="14sp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<FrameLayout
android:layout_width="0dp"
android:layout_height="1dp"
android:background="@color/mid_grey"
app:layout_constraintBottom_toBottomOf="@+id/previous_responses_header_text"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@+id/previous_responses_header_text"
app:layout_constraintTop_toTopOf="@+id/previous_responses_header_text" />
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
1 change: 1 addition & 0 deletions app/src/main/res/values/colors.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<color name="grey">#CCCCCC</color>
<!-- INPUT LAYOUT EDITTEXT -->
<color name="edit_text_hint">#80333333</color>
<color name="mid_grey">#ff333333</color>
<color name="blue_shade">#395FD0</color>
<color name="blue_shade_80">#80395FD0</color>
<color name="blue_shade_60">#60395FD0</color>
Expand Down
Loading