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 #4470, #4472, #4473 : Handle configuration change using onSavedInstance. #5478

Merged
merged 25 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3274c45
Drap and drop interaction retain input when devices configuration cha…
Vishwajith-Shettigar Aug 7, 2024
f73b5e7
klint
Vishwajith-Shettigar Aug 7, 2024
b42552a
retain input in image selection interaction
Vishwajith-Shettigar Aug 9, 2024
cebc439
klint
Vishwajith-Shettigar Aug 9, 2024
1e99c58
Clean up
Vishwajith-Shettigar Aug 9, 2024
01a426e
Added comments
Vishwajith-Shettigar Aug 9, 2024
07ff0ac
Clean up
Vishwajith-Shettigar Aug 9, 2024
bfaf1f2
remove observer
Vishwajith-Shettigar Aug 10, 2024
a9647ba
remove observer
Vishwajith-Shettigar Aug 10, 2024
7597c1d
Added comment
Vishwajith-Shettigar Aug 10, 2024
fe281dd
Modified comment
Vishwajith-Shettigar Aug 10, 2024
f7b59b6
Reorganize code for clarity
Vishwajith-Shettigar Aug 12, 2024
8420daa
Added test for drag and drop and image interaction inout
Vishwajith-Shettigar Aug 13, 2024
6e7aa3e
handle null using let
Vishwajith-Shettigar Aug 13, 2024
c2b0e66
Fix test
Vishwajith-Shettigar Aug 13, 2024
cb9e639
Merge branch 'develop' into retain-input
Vishwajith-Shettigar Aug 14, 2024
73a4203
modified tests
Vishwajith-Shettigar Aug 14, 2024
f11ae72
modified tests
Vishwajith-Shettigar Aug 14, 2024
50f35db
modified tests
Vishwajith-Shettigar Aug 14, 2024
654414d
Added test for drag and drop error message
Vishwajith-Shettigar Aug 14, 2024
5220eb5
klint
Vishwajith-Shettigar Aug 14, 2024
3f407f2
Removed Default region retention
Vishwajith-Shettigar Aug 17, 2024
c807bdd
Modified comments for better understanding
Vishwajith-Shettigar Aug 21, 2024
ba8d992
Merge branch 'develop' into retain-input
Vishwajith-Shettigar Aug 23, 2024
4755ca3
Merge branch 'develop' into retain-input
Vishwajith-Shettigar Aug 24, 2024
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 @@ -9,6 +9,7 @@ import androidx.core.view.forEachIndexed
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import org.oppia.android.app.model.ImageWithRegions
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.shim.ViewBindingShim
import org.oppia.android.app.utility.ClickableAreasImage
import org.oppia.android.app.utility.OnClickableAreaClickedListener
Expand Down Expand Up @@ -52,6 +53,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
private lateinit var imageUrl: String
private lateinit var clickableAreas: List<ImageWithRegions.LabeledRegion>

private lateinit var userAnswerState: UserAnswerState

/**
* Sets the URL for the image & initiates loading it. This is intended to be called via
* data-binding.
Expand All @@ -61,6 +64,10 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
maybeInitializeClickableAreas()
}

fun setUserAnswerState(userAnswerrState: UserAnswerState) {
this.userAnswerState = userAnswerrState
}

fun setEntityId(entityId: String) {
this.entityId = entityId
maybeInitializeClickableAreas()
Expand Down Expand Up @@ -121,7 +128,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
onRegionClicked,
bindingInterface,
isAccessibilityEnabled = accessibilityService.isScreenReaderEnabled(),
clickableAreas
clickableAreas,
userAnswerState
)
areasImage.addRegionViews()
performAttachment(areasImage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ class DragAndDropSortInteractionViewModel private constructor(
val isSplitView: Boolean,
private val writtenTranslationContext: WrittenTranslationContext,
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController
private val translationController: TranslationController,
userAnswerState: UserAnswerState
) : StateItemViewModel(ViewType.DRAG_DROP_SORT_INTERACTION),
InteractionAnswerHandler,
OnItemDragListener,
Expand All @@ -71,10 +72,18 @@ class DragAndDropSortInteractionViewModel private constructor(
subtitledHtml.contentId to translatedHtml
}

private var answerErrorCetegory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR

private val _originalChoiceItems: MutableList<DragDropInteractionContentViewModel> =
computeChoiceItems(contentIdHtmlMap, choiceSubtitledHtmls, this, resourceHandler)
computeOriginalChoiceItems(contentIdHtmlMap, choiceSubtitledHtmls, this, resourceHandler)

private val _choiceItems = _originalChoiceItems.toMutableList()
private val _choiceItems = computeSelectedChoiceItems(
contentIdHtmlMap,
choiceSubtitledHtmls,
this,
resourceHandler,
userAnswerState
)
val choiceItems: List<DragDropInteractionContentViewModel> = _choiceItems

private var pendingAnswerError: String? = null
Expand All @@ -99,6 +108,7 @@ class DragAndDropSortInteractionViewModel private constructor(
pendingAnswerError = null,
inputAnswerAvailable = true
)
checkPendingAnswerError(userAnswerState.answerErrorCategory)
}

override fun onItemDragged(
Expand Down Expand Up @@ -160,6 +170,7 @@ class DragAndDropSortInteractionViewModel private constructor(
* updates the error string based on the specified error category.
*/
override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
answerErrorCetegory = category
pendingAnswerError = when (category) {
AnswerErrorCategory.REAL_TIME -> null
AnswerErrorCategory.SUBMIT_TIME ->
Expand Down Expand Up @@ -232,9 +243,9 @@ class DragAndDropSortInteractionViewModel private constructor(
}

private fun getSubmitTimeError(): DragAndDropSortInteractionError {
return if (_originalChoiceItems == _choiceItems)
return if (_originalChoiceItems == _choiceItems) {
DragAndDropSortInteractionError.EMPTY_INPUT
else
} else
DragAndDropSortInteractionError.VALID
}

Expand Down Expand Up @@ -263,13 +274,30 @@ class DragAndDropSortInteractionViewModel private constructor(
isSplitView,
writtenTranslationContext,
resourceHandler,
translationController
translationController,
userAnswerState
)
}
}

override fun getUserAnswerState(): UserAnswerState {
if (_choiceItems == _originalChoiceItems) {
return UserAnswerState.newBuilder().apply {
this.answerErrorCategory = answerErrorCetegory
}.build()
}
return UserAnswerState.newBuilder().apply {
val htmlContentIds = _choiceItems.map { it.htmlContent }
listOfSetsOfTranslatableHtmlContentIds =
ListOfSetsOfTranslatableHtmlContentIds.newBuilder().apply {
addAllContentIdLists(htmlContentIds)
}.build()
answerErrorCategory = answerErrorCetegory
}.build()
}

companion object {
private fun computeChoiceItems(
private fun computeOriginalChoiceItems(
contentIdHtmlMap: Map<String, String>,
choiceStrings: List<SubtitledHtml>,
dragAndDropSortInteractionViewModel: DragAndDropSortInteractionViewModel,
Expand All @@ -293,4 +321,28 @@ class DragAndDropSortInteractionViewModel private constructor(
}.toMutableList()
}
}

private fun computeSelectedChoiceItems(
contentIdHtmlMap: Map<String, String>,
choiceStrings: List<SubtitledHtml>,
dragAndDropSortInteractionViewModel: DragAndDropSortInteractionViewModel,
resourceHandler: AppLanguageResourceHandler,
userAnswerState: UserAnswerState
): MutableList<DragDropInteractionContentViewModel> {
return if (userAnswerState.listOfSetsOfTranslatableHtmlContentIds.contentIdListsCount == 0) {
_originalChoiceItems.toMutableList()
} else {
userAnswerState.listOfSetsOfTranslatableHtmlContentIds.contentIdListsList
.mapIndexed { index, contentId ->
DragDropInteractionContentViewModel(
contentIdHtmlMap = contentIdHtmlMap,
htmlContent = contentId,
itemIndex = index,
listSize = choiceStrings.size,
dragAndDropSortInteractionViewModel = dragAndDropSortInteractionViewModel,
resourceHandler = resourceHandler
)
}.toMutableList()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class ImageRegionSelectionInteractionViewModel private constructor(
private val errorOrAvailabilityCheckReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver,
val isSplitView: Boolean,
private val writtenTranslationContext: WrittenTranslationContext,
private val resourceHandler: AppLanguageResourceHandler
private val resourceHandler: AppLanguageResourceHandler,
userAnswerState: UserAnswerState
) : StateItemViewModel(ViewType.IMAGE_REGION_SELECTION_INTERACTION),
InteractionAnswerHandler,
OnClickableAreaClickedListener {
Expand All @@ -43,6 +44,12 @@ class ImageRegionSelectionInteractionViewModel private constructor(
schemaObject?.customSchemaValue?.imageWithRegions?.labelRegionsList ?: listOf()
}

val observableUserAnswrerState by lazy {
ObservableField(userAnswerState)
}

private var answerErrorCetegory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR

val imagePath: String by lazy {
val schemaObject = interaction.customizationArgsMap["imageAndRegions"]
schemaObject?.customSchemaValue?.imageWithRegions?.imagePath ?: ""
Expand All @@ -68,10 +75,10 @@ class ImageRegionSelectionInteractionViewModel private constructor(
pendingAnswerError = null,
inputAnswerAvailable = true
)
checkPendingAnswerError(userAnswerState.answerErrorCategory)
}

override fun onClickableAreaTouched(region: RegionClickedEvent) {

when (region) {
is DefaultRegionClickedEvent -> {
answerText = ""
Expand All @@ -88,6 +95,7 @@ class ImageRegionSelectionInteractionViewModel private constructor(

/** It checks the pending error for the current image region 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 -> {
pendingAnswerError = null
Expand All @@ -110,18 +118,35 @@ class ImageRegionSelectionInteractionViewModel private constructor(
return pendingAnswerError
}

override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply {
val answerTextString = answerText.toString()
answer = InteractionObject.newBuilder().apply {
clickOnImage = parseClickOnImage(answerTextString)
override fun getUserAnswerState(): UserAnswerState {
return UserAnswerState.newBuilder().apply {
if (answerText.isNotEmpty()) {
this.imageLabel = answerText.toString()
}
this.answerErrorCategory = answerErrorCetegory
}.build()
plainAnswer = resourceHandler.getStringInLocaleWithWrapping(
R.string.image_interaction_answer_text,
answerTextString
)
this.writtenTranslationContext =
this@ImageRegionSelectionInteractionViewModel.writtenTranslationContext
}.build()
}

override fun getPendingAnswer(): UserAnswer {
// Resetting Observable UserAnswerState to its default instance to ensure that
// the ImageRegionSelectionInteractionView reflects no image region selection.
// This is necessary because ImageRegionSelectionInteractionView is not recreated every time
// the user submits an answer, causing it to retain the old UserAnswerState.
observableUserAnswrerState.set(UserAnswerState.getDefaultInstance())

return UserAnswer.newBuilder().apply {
val answerTextString = answerText.toString()
answer = InteractionObject.newBuilder().apply {
clickOnImage = parseClickOnImage(answerTextString)
}.build()
plainAnswer = resourceHandler.getStringInLocaleWithWrapping(
R.string.image_interaction_answer_text,
answerTextString
)
this.writtenTranslationContext =
this@ImageRegionSelectionInteractionViewModel.writtenTranslationContext
}.build()
}

private fun parseClickOnImage(answerTextString: String): ClickOnImage {
val region = selectableRegions.find { it.label == answerTextString }
Expand Down Expand Up @@ -204,7 +229,8 @@ class ImageRegionSelectionInteractionViewModel private constructor(
answerErrorReceiver,
isSplitView,
writtenTranslationContext,
resourceHandler
resourceHandler,
userAnswerState
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.core.view.forEachIndexed
import androidx.core.view.isVisible
import org.oppia.android.R
import org.oppia.android.app.model.ImageWithRegions.LabeledRegion
import org.oppia.android.app.model.UserAnswerState
import org.oppia.android.app.player.state.ImageRegionSelectionInteractionView
import org.oppia.android.app.shim.ViewBindingShim
import kotlin.math.roundToInt
Expand All @@ -21,11 +22,19 @@ class ClickableAreasImage(
private val listener: OnClickableAreaClickedListener,
bindingInterface: ViewBindingShim,
private val isAccessibilityEnabled: Boolean,
private val clickableAreas: List<LabeledRegion>
private val clickableAreas: List<LabeledRegion>,
userAnswerState: UserAnswerState
) {
private var imageLabel: String? = null
private val defaultRegionView by lazy { bindingInterface.getDefaultRegion(parentView) }

init { imageView.initializeShowRegionTouchListener() }
init {
imageView.initializeShowRegionTouchListener()

if (userAnswerState.imageLabel.isNotBlank()) {
imageLabel = userAnswerState.imageLabel
}
}

/**
* Called when an image is clicked.
Expand All @@ -41,7 +50,7 @@ class ClickableAreasImage(
defaultRegionView.setBackgroundResource(R.drawable.selected_region_background)
defaultRegionView.x = x
defaultRegionView.y = y
listener.onClickableAreaTouched(DefaultRegionClickedEvent())
listener.onClickableAreaTouched(DefaultRegionClickedEvent(x, y))
}
}

Expand Down Expand Up @@ -104,6 +113,9 @@ class ClickableAreasImage(
newView.isFocusableInTouchMode = true
newView.tag = clickableArea.label
newView.initializeToggleRegionTouchListener(clickableArea)
if (clickableArea.label.equals(imageLabel)) {
showOrHideRegion(newView = newView, clickableArea = clickableArea)
}
if (isAccessibilityEnabled) {
// Make default region visibility gone when talkback enabled to avoid any accidental touch.
defaultRegionView.isVisible = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ data class NamedRegionClickedEvent(val regionLabel: String, val contentDescripti
* Class to be used in case when [OnClickableAreaClickedListener] is called with an unspecified
* region that is when any other is tapped on which wasn't defined by creator.
*/
class DefaultRegionClickedEvent : RegionClickedEvent()
class DefaultRegionClickedEvent(val x: Float, val y: Float) : RegionClickedEvent()
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
app:clickableAreas="@{viewModel.selectableRegions}"
app:entityId="@{viewModel.entityId}"
app:imageUrl="@{viewModel.imagePath}"
app:userAnswerState="@{viewModel.observableUserAnswrerState}"
app:onRegionClicked="@{(region) -> viewModel.onClickableAreaTouched(region)}"
app:overlayView="@{interactionContainerFrameLayout}" />

Expand Down
Loading
Loading