From 6bb3deecabb4a78a27c7cd8037f43063ffae35a0 Mon Sep 17 00:00:00 2001 From: Vishwajith-Shettigar Date: Sat, 20 Jan 2024 11:03:52 +0530 Subject: [PATCH 01/12] allow blank input --- ...mageRegionSelectionInteractionViewModel.kt | 99 ++++++++++++++++++- ...mage_region_selection_interaction_item.xml | 1 + app/src/main/res/values/strings.xml | 1 + 3 files changed, 99 insertions(+), 2 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..b2b154964c5 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 @@ -17,6 +19,7 @@ import org.oppia.android.app.utility.DefaultRegionClickedEvent import org.oppia.android.app.utility.NamedRegionClickedEvent import org.oppia.android.app.utility.OnClickableAreaClickedListener import org.oppia.android.app.utility.RegionClickedEvent +import org.oppia.android.util.math.FractionParser import javax.inject.Inject /** [StateItemViewModel] for image region selection. */ @@ -31,12 +34,16 @@ 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"] schemaObject?.customSchemaValue?.imageWithRegions?.labelRegionsList ?: listOf() } + private val fractionParser = FractionParser() val imagePath: String by lazy { val schemaObject = interaction.customizationArgsMap["imageAndRegions"] schemaObject?.customSchemaValue?.imageWithRegions?.imagePath ?: "" @@ -49,25 +56,68 @@ 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 ) } } isAnswerAvailable.addOnPropertyChangedCallback(callback) + errorMessage.addOnPropertyChangedCallback(callback) + + 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 fraction input, and correspondingly updates the error string based on the specified error category. */ + override fun checkPendingAnswerError(category: AnswerErrorCategory): String? { + when (category) { + AnswerErrorCategory.REAL_TIME -> { + + if (answerText.isNotEmpty()) { + pendingAnswerError = + ImageRegionParsingUiError.createFromParsingError( + getSubmitTimeError(answerText.toString()) + ).getErrorMessageFromStringRes(resourceHandler) + } else { + pendingAnswerError = + ImageRegionParsingUiError.createFromParsingError( + ImageRegionParsingError.VALID + ).getErrorMessageFromStringRes(resourceHandler) + } + } + + 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,6 +141,51 @@ class ImageRegionSelectionInteractionViewModel private constructor( .build() } + 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 diff --git a/app/src/main/res/layout/image_region_selection_interaction_item.xml b/app/src/main/res/layout/image_region_selection_interaction_item.xml index 375835975b7..ca193c62f22 100644 --- a/app/src/main/res/layout/image_region_selection_interaction_item.xml +++ b/app/src/main/res/layout/image_region_selection_interaction_item.xml @@ -54,6 +54,7 @@ app:onRegionClicked="@{(region) -> viewModel.onClickableAreaTouched(region)}" app:overlayView="@{interactionContainerFrameLayout}" /> + Return To Topic Previous Responses (%s) Clicks on %s + Select a image to continue. Learn Again (%s) See More From 87a35cba30824847fb100cda1c4811b9317358cb Mon Sep 17 00:00:00 2001 From: Vishwajith-Shettigar Date: Sat, 20 Jan 2024 12:35:59 +0530 Subject: [PATCH 02/12] added blank input error view --- ...mage_region_selection_interaction_item.xml | 118 +++++++++++------- 1 file changed, 71 insertions(+), 47 deletions(-) diff --git a/app/src/main/res/layout/image_region_selection_interaction_item.xml b/app/src/main/res/layout/image_region_selection_interaction_item.xml index ca193c62f22..7224cabb202 100644 --- a/app/src/main/res/layout/image_region_selection_interaction_item.xml +++ b/app/src/main/res/layout/image_region_selection_interaction_item.xml @@ -11,53 +11,77 @@ type="org.oppia.android.app.player.state.itemviewmodel.ImageRegionSelectionInteractionViewModel" /> - - - + + - - - - + android:background="@drawable/drag_drop_white_background" + android:descendantFocusability="beforeDescendants" + android:focusableInTouchMode="true" + android:paddingStart="16dp" + android:paddingTop="12dp" + android:paddingEnd="16dp" + android:paddingBottom="12dp" + app:explorationSplitViewMarginApplicable="@{viewModel.hasConversationView && viewModel.splitView}" + app:explorationSplitViewMarginBottom="@{@dimen/space_0dp}" + app:explorationSplitViewMarginEnd="@{@dimen/interaction_item_split_view_margin_end}" + app:explorationSplitViewMarginStart="@{@dimen/interaction_item_split_view_margin_start}" + app:explorationSplitViewMarginTop="@{@dimen/interaction_item_split_view_margin_top}" + app:explorationViewMarginApplicable="@{viewModel.hasConversationView && !viewModel.splitView}" + app:explorationViewMarginBottom="@{@dimen/space_0dp}" + app:explorationViewMarginEnd="@{@dimen/interaction_item_exploration_view_margin_end}" + app:explorationViewMarginStart="@{@dimen/interaction_item_exploration_view_margin_start}" + app:explorationViewMarginTop="@{@dimen/interaction_item_exploration_view_margin_top}" + app:layout_constraintEnd_toEndOf="parent" + app:layout_constraintStart_toStartOf="parent" + app:layout_constraintTop_toTopOf="parent" + app:questionSplitViewMarginApplicable="@{!viewModel.hasConversationView && viewModel.splitView}" + app:questionSplitViewMarginBottom="@{@dimen/space_0dp}" + app:questionSplitViewMarginEnd="@{@dimen/interaction_item_split_view_margin_end}" + app:questionSplitViewMarginStart="@{@dimen/interaction_item_split_view_margin_start}" + app:questionSplitViewMarginTop="@{@dimen/interaction_item_split_view_margin_top}" + app:questionViewMarginApplicable="@{!viewModel.hasConversationView && !viewModel.splitView}" + app:questionViewMarginBottom="@{@dimen/space_0dp}" + app:questionViewMarginEnd="@{@dimen/interaction_item_question_view_margin_end}" + app:questionViewMarginStart="@{@dimen/interaction_item_question_view_margin_start}" + app:questionViewMarginTop="@{@dimen/interaction_item_question_view_margin_top}"> + + + + + + + + + From b944f9909d6d553bb5607b41b7093272d53cbaf7 Mon Sep 17 00:00:00 2001 From: Vishwajith-Shettigar Date: Sat, 20 Jan 2024 12:49:11 +0530 Subject: [PATCH 03/12] minor changes --- app/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 6961d60d7fd..eb14d96753f 100755 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -68,7 +68,7 @@ Return To Topic Previous Responses (%s) Clicks on %s - Select a image to continue. + Select an image to continue. Learn Again (%s) See More From beab8595e0523e443c97cf812596afd1141ed9dd Mon Sep 17 00:00:00 2001 From: Vishwajith-Shettigar Date: Sun, 21 Jan 2024 20:37:56 +0530 Subject: [PATCH 04/12] added test --- .../app/activity/ActivityComponentImpl.kt | 1 + .../app/fragment/FragmentComponentImpl.kt | 1 + ...mageRegionSelectionInteractionViewModel.kt | 3 +- ...ageRegionSelectionTestFragmentPresenter.kt | 71 ++++++++++++++++--- .../image_region_selection_test_fragment.xml | 40 +++++++++++ ...ImageRegionSelectionInteractionViewTest.kt | 60 ++++++++++++++-- 6 files changed, 159 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt b/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt index 53f0f35955f..b5757269b82 100644 --- a/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt +++ b/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt @@ -123,6 +123,7 @@ interface ActivityComponentImpl : appCompatCheckBoxBindingAdaptersTestActivity: AppCompatCheckBoxBindingAdaptersTestActivity ) + fun inject(appLanguageActivity: AppLanguageActivity) fun inject(appVersionActivity: AppVersionActivity) fun inject(audioFragmentTestActivity: AudioFragmentTestActivity) diff --git a/app/src/main/java/org/oppia/android/app/fragment/FragmentComponentImpl.kt b/app/src/main/java/org/oppia/android/app/fragment/FragmentComponentImpl.kt index 029d214a41a..7700c5c1c21 100644 --- a/app/src/main/java/org/oppia/android/app/fragment/FragmentComponentImpl.kt +++ b/app/src/main/java/org/oppia/android/app/fragment/FragmentComponentImpl.kt @@ -120,6 +120,7 @@ interface FragmentComponentImpl : FragmentComponent, ViewComponentBuilderInjecto automaticAppDeprecationNoticeDialogFragment: AutomaticAppDeprecationNoticeDialogFragment ) + fun inject(betaNoticeDialogFragment: BetaNoticeDialogFragment) fun inject(cellularAudioDialogFragment: CellularAudioDialogFragment) fun inject(completedStoryListFragment: CompletedStoryListFragment) 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 b2b154964c5..d71f38e758f 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 @@ -19,7 +19,6 @@ import org.oppia.android.app.utility.DefaultRegionClickedEvent import org.oppia.android.app.utility.NamedRegionClickedEvent import org.oppia.android.app.utility.OnClickableAreaClickedListener import org.oppia.android.app.utility.RegionClickedEvent -import org.oppia.android.util.math.FractionParser import javax.inject.Inject /** [StateItemViewModel] for image region selection. */ @@ -43,7 +42,6 @@ class ImageRegionSelectionInteractionViewModel private constructor( schemaObject?.customSchemaValue?.imageWithRegions?.labelRegionsList ?: listOf() } - private val fractionParser = FractionParser() val imagePath: String by lazy { val schemaObject = interaction.customizationArgsMap["imageAndRegions"] schemaObject?.customSchemaValue?.imageWithRegions?.imagePath ?: "" @@ -190,6 +188,7 @@ class ImageRegionSelectionInteractionViewModel private constructor( 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..94840e03b2a 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 @@ -4,10 +4,19 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment +import kotlinx.android.synthetic.main.image_region_selection_test_fragment.view.* 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,43 @@ 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]. */ + + val imageRegionSelectionInteractionViewModel by lazy { + imageRegionSelectionInteractionViewModelFactory + .create() + } + + /** Gives access to the translation context. */ + 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) + } + } + + view.submit_button.setOnClickListener { + imageRegionSelectionInteractionViewModel + .checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) } return view } @@ -72,7 +106,28 @@ class ImageRegionSelectionTestFragmentPresenter @Inject constructor( .build() } + private inline fun StateItemViewModel + .InteractionItemFactory.create( + interaction: Interaction = Interaction.getDefaultInstance() + ): T { + + return create( + entityId = "fake_entity_id", + hasConversationView = false, + interaction = interaction, + interactionAnswerReceiver = this@ImageRegionSelectionTestFragmentPresenter, + answerErrorReceiver = this@ImageRegionSelectionTestFragmentPresenter, + hasPreviousButton = false, + isSplitView = false, + writtenTranslationContext, + timeToStartNoticeAnimationMs = null + ) as T + } + private fun createPoint2d(x: Float, y: Float): Point2d { return Point2d.newBuilder().setX(x).setY(y).build() } + + override fun onAnswerReadyForSubmission(answer: UserAnswer) { + } } diff --git a/app/src/main/res/layout/image_region_selection_test_fragment.xml b/app/src/main/res/layout/image_region_selection_test_fragment.xml index 01bd0230c13..f92ac5cbd4a 100644 --- a/app/src/main/res/layout/image_region_selection_test_fragment.xml +++ b/app/src/main/res/layout/image_region_selection_test_fragment.xml @@ -3,10 +3,25 @@ xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools"> + + + + + + + + + + +