Skip to content

Commit

Permalink
Fix #4470, #4472, #4473 : Handle configuration change using onSavedIn…
Browse files Browse the repository at this point in the history
…stance. (#5478)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes #4470, Fixes #4472, Fixes #4473.

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

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


| Drag and Drop |
-------------------


https://github.com/user-attachments/assets/1919f99d-d56d-4bbe-b0bc-6092c488165e

| Image Region  |
-------------------


https://github.com/user-attachments/assets/ac1ebcd8-92cd-47ca-82af-1208751a7093

Rest are covered in this PR #5458

<!--
- 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 26, 2024
1 parent a85cc50 commit 3b7ddb9
Show file tree
Hide file tree
Showing 8 changed files with 243 additions and 27 deletions.
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

0 comments on commit 3b7ddb9

Please sign in to comment.