Skip to content

Commit

Permalink
Fix #3708: Content Description Generation For ImageRegionSelectionInt…
Browse files Browse the repository at this point in the history
…eraction (#5691)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- 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.
  -->
Fix #3708

This PR adds logic for dynamic content descriptions in
**ImageRegionSelectionInteraction**, i.e. generating distinct
descriptions for selected and not selected regions. For example:
- **Selected:** "Image showing Region 3." 
- **Not Selected:** "Select Region 3 image."

## Before

![image](https://github.com/user-attachments/assets/ddce71d7-f092-431a-af12-2d8e7ec727f3)

![image](https://github.com/user-attachments/assets/93764cf6-a1b8-4fa0-81b0-62b36a6f09d9)

## After

![image](https://github.com/user-attachments/assets/b23193f2-de50-4e30-a970-f45296f29483)

![image](https://github.com/user-attachments/assets/98e7f591-85df-4cb4-bb66-d38c1fb4e808)


## 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)).
  • Loading branch information
manas-yu authored Feb 18, 2025
1 parent 2c6f8a8 commit d7eaad8
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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.translation.AppLanguageResourceHandler
import org.oppia.android.app.utility.ClickableAreasImage
import org.oppia.android.app.utility.OnClickableAreaClickedListener
import org.oppia.android.app.view.ViewComponentFactory
Expand Down Expand Up @@ -46,6 +47,7 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
@Inject lateinit var machineLocale: OppiaLocale.MachineLocale
@Inject lateinit var accessibilityService: AccessibilityService
@Inject lateinit var imageLoader: ImageLoader
@Inject lateinit var resourceHandler: AppLanguageResourceHandler

private lateinit var entityId: String
private lateinit var overlayView: FrameLayout
Expand All @@ -64,8 +66,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
maybeInitializeClickableAreas()
}

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

fun setEntityId(entityId: String) {
Expand Down Expand Up @@ -118,7 +120,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
::entityId.isInitialized &&
::imageUrl.isInitialized &&
::onRegionClicked.isInitialized &&
::overlayView.isInitialized
::overlayView.isInitialized &&
::resourceHandler.isInitialized
) {
loadImage()

Expand All @@ -129,7 +132,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
bindingInterface,
isAccessibilityEnabled = accessibilityService.isScreenReaderEnabled(),
clickableAreas,
userAnswerState
userAnswerState,
resourceHandler
)
areasImage.addRegionViews()
performAttachment(areasImage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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 org.oppia.android.app.translation.AppLanguageResourceHandler
import kotlin.math.roundToInt

/** Helper class to handle clicks on an image along with highlighting the selected region. */
Expand All @@ -23,7 +24,8 @@ class ClickableAreasImage(
bindingInterface: ViewBindingShim,
private val isAccessibilityEnabled: Boolean,
private val clickableAreas: List<LabeledRegion>,
userAnswerState: UserAnswerState
userAnswerState: UserAnswerState,
private val resourceHandler: AppLanguageResourceHandler
) {
private var imageLabel: String? = null
private val defaultRegionView by lazy { bindingInterface.getDefaultRegion(parentView) }
Expand Down Expand Up @@ -60,6 +62,16 @@ class ClickableAreasImage(
// Remove any previously selected region excluding 0th index(image view)
if (index > 0) {
childView.setBackgroundResource(0)
if (childView.tag != null) {
val regionLabel = childView.tag as String
clickableAreas.find { it.label == regionLabel }?.let { clickableArea ->
updateRegionContentDescription(
childView,
clickableArea,
regionLabel == imageLabel
)
}
}
}
}
}
Expand Down Expand Up @@ -112,8 +124,13 @@ class ClickableAreasImage(
newView.isFocusable = true
newView.isFocusableInTouchMode = true
newView.tag = clickableArea.label

val isInitiallySelected = clickableArea.label.equals(imageLabel)
updateRegionContentDescription(newView, clickableArea, isInitiallySelected)

newView.initializeToggleRegionTouchListener(clickableArea)
if (clickableArea.label.equals(imageLabel)) {

if (isInitiallySelected) {
showOrHideRegion(newView = newView, clickableArea = clickableArea)
}
if (isAccessibilityEnabled) {
Expand All @@ -123,7 +140,7 @@ class ClickableAreasImage(
showOrHideRegion(newView, clickableArea)
}
}
newView.contentDescription = clickableArea.contentDescription

parentView.addView(newView)
}

Expand All @@ -143,14 +160,18 @@ class ClickableAreasImage(
}

private fun showOrHideRegion(newView: View, clickableArea: LabeledRegion) {
resetRegionSelectionViews()
listener.onClickableAreaTouched(
NamedRegionClickedEvent(
clickableArea.label,
clickableArea.contentDescription
if (clickableArea.label != imageLabel) {
imageLabel = clickableArea.label
resetRegionSelectionViews()
newView.setBackgroundResource(R.drawable.selected_region_background)

listener.onClickableAreaTouched(
NamedRegionClickedEvent(
clickableArea.label,
generateContentDescription(clickableArea, true)
)
)
)
newView.setBackgroundResource(R.drawable.selected_region_background)
}
}

private fun View.initializeShowRegionTouchListener() {
Expand All @@ -172,4 +193,27 @@ class ClickableAreasImage(
return@setOnTouchListener true
}
}

private fun generateContentDescription(
clickableArea: LabeledRegion,
isSelected: Boolean
): String = if (isSelected) {
resourceHandler.getStringInLocaleWithWrapping(
R.string.selected_image_region_selection_content_description,
clickableArea.label
)
} else {
resourceHandler.getStringInLocaleWithWrapping(
R.string.unselected_image_region_selection_content_description,
clickableArea.label
)
}

private fun updateRegionContentDescription(
view: View,
clickableArea: LabeledRegion,
isSelected: Boolean
) {
view.contentDescription = generateContentDescription(clickableArea, isSelected)
}
}
2 changes: 2 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@
<string name="correct">Correct!</string>
<string name="topic_prefix">Topic: %s</string>
<string name="welcome_profile_name">%1$s %2$s!</string>
<string name="selected_image_region_selection_content_description">Image showing %1$s.</string>
<string name="unselected_image_region_selection_content_description">Select %1$s image.</string>
<plurals name="chapter_count">
<item quantity="one">1 Chapter</item>
<item quantity="other">%s Chapters</item>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.isEnabled
import androidx.test.espresso.matcher.ViewMatchers.withContentDescription
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withTagValue
import androidx.test.espresso.matcher.ViewMatchers.withText
Expand Down Expand Up @@ -177,7 +178,7 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 3", contentDescription = "You have selected Region 3"
regionLabel = "Region 3", contentDescription = "Image showing Region 3."
)
)
}
Expand Down Expand Up @@ -218,12 +219,26 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 2", contentDescription = "You have selected Region 2"
regionLabel = "Region 2", contentDescription = "Image showing Region 2."
)
)
}
}

@Test
// TODO(#1611): Fix ImageRegionSelectionInteractionViewTest
@RunOn(TestPlatform.ESPRESSO)
fun testImageRegionSelectionInteractionView_initialContentDescriptionRegion3_isCorrect() {
launch(ImageRegionSelectionTestActivity::class.java).use {
onView(
allOf(
withTagValue(`is`("Region 3")),
withContentDescription("Select Region 3 image.")
)
).check(matches(isDisplayed()))
}
}

@Test
// TODO(#1611): Fix ImageRegionSelectionInteractionViewTest
@RunOn(TestPlatform.ESPRESSO)
Expand Down Expand Up @@ -280,7 +295,7 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 2", contentDescription = "You have selected Region 2"
regionLabel = "Region 2", contentDescription = "Image showing Region 2."
)
)
}
Expand Down Expand Up @@ -308,7 +323,7 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 3", contentDescription = "You have selected Region 3"
regionLabel = "Region 3", contentDescription = "Image showing Region 3."
)
)
}
Expand Down Expand Up @@ -352,7 +367,7 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 3", contentDescription = "You have selected Region 3"
regionLabel = "Region 3", contentDescription = "Image showing Region 3."
)
)
}
Expand Down Expand Up @@ -394,7 +409,7 @@ class ImageRegionSelectionInteractionViewTest {
assertThat(regionClickedEvent.value)
.isEqualTo(
NamedRegionClickedEvent(
regionLabel = "Region 2", contentDescription = "You have selected Region 2"
regionLabel = "Region 2", contentDescription = "Image showing Region 2."
)
)
}
Expand Down

0 comments on commit d7eaad8

Please sign in to comment.