From 993877939502fffee377a211142055ff38f97346 Mon Sep 17 00:00:00 2001 From: Vishwajith Shettigar <76042077+Vishwajith-Shettigar@users.noreply.github.com> Date: Tue, 27 Feb 2024 07:02:16 +0530 Subject: [PATCH] Fix part of #5070: Display empty answer message in image click interaction (#5316) ## Explanation Fix part of #5070, In Image interaction UI, leave submit button enabled when answer is empty. Show an error on submitting an empty answer. [image_blank_input.webm](https://github.com/oppia/oppia-android/assets/76042077/51d5f2d6-03c1-468b-aed0-0fc75560ac48) ([Grammar error](https://github.com/oppia/oppia-android/pull/5316/commits/b944f9909d6d553bb5607b41b7093272d53cbaf7#:~:text=Select%20an%20image%20to%20continue) fixed ) ## Essential Checklist - [ ] 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: ...".) - [ ] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [ ] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [ ] 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)). - [ ] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [ ] 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 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 --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- ...mageRegionSelectionInteractionViewModel.kt | 92 +++++++++++++- ...ageRegionSelectionTestFragmentPresenter.kt | 72 +++++++++-- ...mage_region_selection_interaction_item.xml | 117 +++++++++++------- .../image_region_selection_test_fragment.xml | 40 ++++++ app/src/main/res/values/strings.xml | 1 + ...ImageRegionSelectionInteractionViewTest.kt | 66 ++++++++-- 6 files changed, 325 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/ImageRegionSelectionInteractionViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/ImageRegionSelectionInteractionViewModel.kt index 0c4029e940e..4ba33e25422 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/ImageRegionSelectionInteractionViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/ImageRegionSelectionInteractionViewModel.kt @@ -1,5 +1,6 @@ package org.oppia.android.app.player.state.itemviewmodel +import androidx.annotation.StringRes import androidx.databinding.Observable import androidx.databinding.ObservableField import org.oppia.android.R @@ -9,6 +10,7 @@ import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.InteractionObject import org.oppia.android.app.model.UserAnswer import org.oppia.android.app.model.WrittenTranslationContext +import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver @@ -31,6 +33,9 @@ class ImageRegionSelectionInteractionViewModel private constructor( ) : StateItemViewModel(ViewType.IMAGE_REGION_SELECTION_INTERACTION), InteractionAnswerHandler, OnClickableAreaClickedListener { + private var pendingAnswerError: String? = null + var errorMessage = ObservableField("") + private var isDefaultRegionClicked = false var answerText: CharSequence = "" val selectableRegions: List by lazy { val schemaObject = interaction.customizationArgsMap["imageAndRegions"] @@ -49,25 +54,58 @@ class ImageRegionSelectionInteractionViewModel private constructor( object : Observable.OnPropertyChangedCallback() { override fun onPropertyChanged(sender: Observable, propertyId: Int) { errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( - pendingAnswerError = null, - inputAnswerAvailable = answerText.isNotEmpty() + pendingAnswerError = pendingAnswerError, + inputAnswerAvailable = true // Allow blank answer submission. ) } } isAnswerAvailable.addOnPropertyChangedCallback(callback) + errorMessage.addOnPropertyChangedCallback(callback) + + // Initializing with default values so that submit button is enabled by default. + errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( + pendingAnswerError = null, + inputAnswerAvailable = true + ) } override fun onClickableAreaTouched(region: RegionClickedEvent) { + when (region) { is DefaultRegionClickedEvent -> { answerText = "" isAnswerAvailable.set(false) + isDefaultRegionClicked = true } is NamedRegionClickedEvent -> { answerText = region.regionLabel isAnswerAvailable.set(true) } } + checkPendingAnswerError(AnswerErrorCategory.REAL_TIME) + } + + /** 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? { + when (category) { + AnswerErrorCategory.REAL_TIME -> { + pendingAnswerError = null + } + + AnswerErrorCategory.SUBMIT_TIME -> { + if (answerText.isNotEmpty() || isDefaultRegionClicked) { + pendingAnswerError = null + } else { + pendingAnswerError = + ImageRegionParsingUiError.createFromParsingError( + getSubmitTimeError(answerText.toString()) + ).getErrorMessageFromStringRes(resourceHandler) + } + } + } + + errorMessage.set(pendingAnswerError) + return pendingAnswerError } override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply { @@ -91,10 +129,60 @@ class ImageRegionSelectionInteractionViewModel private constructor( .build() } + /** + * Returns [ImageRegionParsingError.EMPTY_INPUT] if input is blank, or + * [TextParsingError.VALID] if input is not empty. + */ + fun getSubmitTimeError(text: String): ImageRegionParsingError { + if (text.isNullOrBlank()) { + return ImageRegionParsingError.EMPTY_INPUT + } + return ImageRegionParsingError.VALID + } + + /** Represents errors that can occur when parsing region name. */ + enum class ImageRegionParsingError { + + /** Indicates that the considered string is a valid. */ + VALID, + + /** Indicates that the input text was empty. */ + EMPTY_INPUT + } + + enum class ImageRegionParsingUiError(@StringRes private var error: Int?) { + /** Corresponds to [ImageRegionParsingError.VALID]. */ + VALID(error = null), + + /** Corresponds to [ImageRegionParsingError.EMPTY_INPUT]. */ + EMPTY_INPUT(error = R.string.image_error_empty_input); + + /** + * Returns the string corresponding to this error's string resources, or null if there is none. + */ + fun getErrorMessageFromStringRes(resourceHandler: AppLanguageResourceHandler): String? = + error?.let(resourceHandler::getStringInLocale) + + companion object { + /** + * Returns the [ImageRegionParsingUiError] corresponding to the specified [ImageRegionParsingError]. + */ + fun createFromParsingError(parsingError: ImageRegionParsingError): ImageRegionParsingUiError { + return when (parsingError) { + + ImageRegionParsingError.VALID -> VALID + + ImageRegionParsingError.EMPTY_INPUT -> EMPTY_INPUT + } + } + } + } + /** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */ class FactoryImpl @Inject constructor( private val resourceHandler: AppLanguageResourceHandler ) : InteractionItemFactory { + override fun create( entityId: String, hasConversationView: Boolean, diff --git a/app/src/main/java/org/oppia/android/app/testing/ImageRegionSelectionTestFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/testing/ImageRegionSelectionTestFragmentPresenter.kt index a7cc208ec1b..cdd29071e40 100644 --- a/app/src/main/java/org/oppia/android/app/testing/ImageRegionSelectionTestFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/testing/ImageRegionSelectionTestFragmentPresenter.kt @@ -3,11 +3,20 @@ package org.oppia.android.app.testing import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.widget.Button import androidx.fragment.app.Fragment import org.oppia.android.R import org.oppia.android.app.model.ImageWithRegions.LabeledRegion +import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.Point2d +import org.oppia.android.app.model.UserAnswer +import org.oppia.android.app.model.WrittenTranslationContext import org.oppia.android.app.player.state.ImageRegionSelectionInteractionView +import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory +import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver +import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver +import org.oppia.android.app.player.state.itemviewmodel.ImageRegionSelectionInteractionViewModel +import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel import org.oppia.android.app.utility.OnClickableAreaClickedListener import org.oppia.android.databinding.ImageRegionSelectionTestFragmentBinding import javax.inject.Inject @@ -15,18 +24,44 @@ import javax.inject.Inject /** The presenter for [ImageRegionSelectionTestActivity]. */ class ImageRegionSelectionTestFragmentPresenter @Inject constructor( private val fragment: Fragment -) { +) : InteractionAnswerErrorOrAvailabilityCheckReceiver, + InteractionAnswerReceiver { + + @Inject + lateinit var imageRegionSelectionInteractionViewModelFactory: + ImageRegionSelectionInteractionViewModel.FactoryImpl + + /** Gives access to the [ImageRegionSelectionInteractionViewModel]. */ + + private val imageRegionSelectionInteractionViewModel by lazy { + imageRegionSelectionInteractionViewModelFactory + .create() + } + + /** Gives access to the translation context. */ + private lateinit var writtenTranslationContext: WrittenTranslationContext + fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { - val view = + val binding = ImageRegionSelectionTestFragmentBinding.inflate( - inflater, container, /* attachToRoot= */ false - ).root + inflater, container, false + ) + writtenTranslationContext = WrittenTranslationContext.getDefaultInstance() + binding.viewModel = imageRegionSelectionInteractionViewModel + val view = binding.root with(view) { val clickableAreas: List = getClickableAreas() - view.findViewById(R.id.clickable_image_view).apply { - setClickableAreas(clickableAreas) - setOnRegionClicked(fragment as OnClickableAreaClickedListener) - } + view.findViewById(R.id.clickable_image_view) + .apply { + setClickableAreas(clickableAreas) + setOnRegionClicked(fragment as OnClickableAreaClickedListener) + } + } + + val submit_button = view.findViewById