Skip to content

Commit

Permalink
Fix part of #5070: Display empty answer message in image click intera…
Browse files Browse the repository at this point in the history
…ction (#5316)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## 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](b944f99#:~:text=Select%20an%20image%20to%20continue)
fixed )

<!--
- 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. -->
- [ ] 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
<!-- 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

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
Vishwajith-Shettigar and adhiamboperes authored Feb 27, 2024
1 parent 005672e commit 9938779
Show file tree
Hide file tree
Showing 6 changed files with 325 additions and 63 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -31,6 +33,9 @@ class ImageRegionSelectionInteractionViewModel private constructor(
) : StateItemViewModel(ViewType.IMAGE_REGION_SELECTION_INTERACTION),
InteractionAnswerHandler,
OnClickableAreaClickedListener {
private var pendingAnswerError: String? = null
var errorMessage = ObservableField<String>("")
private var isDefaultRegionClicked = false
var answerText: CharSequence = ""
val selectableRegions: List<ImageWithRegions.LabeledRegion> by lazy {
val schemaObject = interaction.customizationArgsMap["imageAndRegions"]
Expand All @@ -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 {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,65 @@ 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

/** 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<ImageRegionSelectionInteractionViewModel>()
}

/** 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<LabeledRegion> = getClickableAreas()
view.findViewById<ImageRegionSelectionInteractionView>(R.id.clickable_image_view).apply {
setClickableAreas(clickableAreas)
setOnRegionClicked(fragment as OnClickableAreaClickedListener)
}
view.findViewById<ImageRegionSelectionInteractionView>(R.id.clickable_image_view)
.apply {
setClickableAreas(clickableAreas)
setOnRegionClicked(fragment as OnClickableAreaClickedListener)
}
}

val submit_button = view.findViewById<Button>(R.id.submit_button)
submit_button.setOnClickListener {
imageRegionSelectionInteractionViewModel
.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME)
}
return view
}
Expand Down Expand Up @@ -72,7 +107,28 @@ class ImageRegionSelectionTestFragmentPresenter @Inject constructor(
.build()
}

private inline fun <reified T : StateItemViewModel> 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) {
}
}
117 changes: 71 additions & 46 deletions app/src/main/res/layout/image_region_selection_interaction_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,52 +11,77 @@
type="org.oppia.android.app.player.state.itemviewmodel.ImageRegionSelectionInteractionViewModel" />
</data>

<FrameLayout
android:id="@+id/interaction_container_frame_layout"
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
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 &amp;&amp; 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 &amp;&amp; !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:questionSplitViewMarginApplicable="@{!viewModel.hasConversationView &amp;&amp; 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 &amp;&amp; !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}">

<org.oppia.android.app.player.state.ImageRegionSelectionInteractionView
android:id="@+id/image_click_interaction_image_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content">

<FrameLayout
android:id="@+id/interaction_container_frame_layout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:adjustViewBounds="true"
app:clickableAreas="@{viewModel.selectableRegions}"
app:entityId="@{viewModel.entityId}"
app:imageUrl="@{viewModel.imagePath}"
app:onRegionClicked="@{(region) -> viewModel.onClickableAreaTouched(region)}"
app:overlayView="@{interactionContainerFrameLayout}" />

<View
android:id="@+id/default_selected_region"
android:layout_width="24dp"
android:layout_height="24dp" />
</FrameLayout>
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 &amp;&amp; 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 &amp;&amp; !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 &amp;&amp; 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 &amp;&amp; !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}">

<org.oppia.android.app.player.state.ImageRegionSelectionInteractionView
android:id="@+id/image_click_interaction_image_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:adjustViewBounds="true"
app:clickableAreas="@{viewModel.selectableRegions}"
app:entityId="@{viewModel.entityId}"
app:imageUrl="@{viewModel.imagePath}"
app:onRegionClicked="@{(region) -> viewModel.onClickableAreaTouched(region)}"
app:overlayView="@{interactionContainerFrameLayout}" />

<View
android:id="@+id/default_selected_region"
android:layout_width="24dp"
android:layout_height="24dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
</FrameLayout>

<TextView
android:id="@+id/image_input_error"
style="@style/InputInteractionErrorTextView"
android:layout_marginStart="10dp"
android:layout_marginEnd="40dp"
android:gravity="end"
android:text="@{viewModel.errorMessage}"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
android:textColor="@color/component_color_shared_input_error_color"
android:visibility="@{viewModel.errorMessage.length()>0? View.VISIBLE : View.GONE}"
app:layout_constraintTop_toBottomOf="@+id/interaction_container_frame_layout" />
</androidx.constraintlayout.widget.ConstraintLayout>

</layout>
Loading

0 comments on commit 9938779

Please sign in to comment.