Skip to content

Commit

Permalink
Fix #4848 Content Description Generation for Content and SelectionInt…
Browse files Browse the repository at this point in the history
…eractionContent ViewModels (#5704)

<!-- 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 #4848 
This PR addresses post-PR review issues from #5614 by refining content
description generation. The key updates include:

- The `getContentDescription` function is now utilized in
`ContentViewModel` and `SelectionInteractionContentViewModel`, ensuring
accurate content descriptions with the necessary `customTagHandlers`
1. `ContentViewModel`: Handles `CUSTOM_LIST_LI_TAG`,
`CUSTOM_LIST_OL_TAG`, `CUSTOM_LIST_UL_TAG`, `CUSTOM_IMG_TAG`,
`CUSTOM_CONCEPT_CARD_TAG`, and `CUSTOM_MATH_TAG`
   2. `SelectionInteractionContentViewModel`: Handles `CUSTOM_IMG_TAG`  

- Improved handling of tags where content resides in attributes, such as
anchor tags: `<a href="https://example.com">Click here</a>`, ensuring
proper extraction of meaningful descriptions

- Handled edge cases in `CustomHtmlContentHandler` to improve
`getContentDescription` logic, addressing inconsistencies and included
comments for better understanding.

## 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 17, 2025
1 parent 38419bf commit 2c6f8a8
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 101 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.oppia.android.app.player.state

import android.app.Application
import android.content.Context
import android.view.LayoutInflater
import android.view.View
Expand Down Expand Up @@ -91,7 +92,18 @@ import org.oppia.android.databinding.SubmittedHtmlAnswerItemBinding
import org.oppia.android.databinding.TextInputInteractionItemBinding
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.parser.html.CUSTOM_CONCEPT_CARD_TAG
import org.oppia.android.util.parser.html.CUSTOM_IMG_TAG
import org.oppia.android.util.parser.html.CUSTOM_LIST_LI_TAG
import org.oppia.android.util.parser.html.CUSTOM_LIST_OL_TAG
import org.oppia.android.util.parser.html.CUSTOM_LIST_UL_TAG
import org.oppia.android.util.parser.html.CUSTOM_MATH_TAG
import org.oppia.android.util.parser.html.ConceptCardTagHandler
import org.oppia.android.util.parser.html.HtmlParser
import org.oppia.android.util.parser.html.ImageTagHandler
import org.oppia.android.util.parser.html.LiTagHandler
import org.oppia.android.util.parser.html.MathTagHandler
import org.oppia.android.util.threading.BackgroundDispatcher
import javax.inject.Inject

Expand Down Expand Up @@ -145,7 +157,9 @@ class StatePlayerRecyclerViewAssembler private constructor(
private val hasConversationView: Boolean,
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController,
private var userAnswerState: UserAnswerState
private var userAnswerState: UserAnswerState,
private val consoleLogger: ConsoleLogger,
private val conceptCardTagHandlerFactory: ConceptCardTagHandler.Factory,
) : HtmlParser.CustomOppiaTagActionListener {
/**
* A list of view models corresponding to past view models that are hidden by default. These are
Expand Down Expand Up @@ -180,6 +194,25 @@ class StatePlayerRecyclerViewAssembler private constructor(
}
}

private val displayLocale = resourceHandler.getDisplayLocale()
private val customTagHandlers = mapOf(
CUSTOM_LIST_LI_TAG to LiTagHandler(context, displayLocale),
CUSTOM_LIST_UL_TAG to LiTagHandler(context, displayLocale),
CUSTOM_LIST_OL_TAG to LiTagHandler(context, displayLocale),
CUSTOM_IMG_TAG to ImageTagHandler(consoleLogger),
CUSTOM_CONCEPT_CARD_TAG to ConceptCardTagHandler(
conceptCardTagHandlerFactory.createConceptCardLinkClickListener(),
consoleLogger
),
// Pick an arbitrary line height since rendering doesn't actually happen.
CUSTOM_MATH_TAG to MathTagHandler(
consoleLogger,
context.assets,
10.0f,
false,
context.applicationContext as Application,
)
)
private val isSplitView = ObservableField<Boolean>(false)

override fun onConceptCardLinkClicked(view: View, skillId: String) {
Expand Down Expand Up @@ -350,7 +383,8 @@ class StatePlayerRecyclerViewAssembler private constructor(
gcsEntityId,
hasConversationView,
isSplitView.get()!!,
playerFeatureSet.conceptCardSupport
playerFeatureSet.conceptCardSupport,
customTagHandlers
)
}
}
Expand Down Expand Up @@ -913,7 +947,9 @@ class StatePlayerRecyclerViewAssembler private constructor(
private val translationController: TranslationController,
private val multiTypeBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory,
private val singleTypeBuilderFactory: BindableAdapter.SingleTypeBuilder.Factory,
private val userAnswerState: UserAnswerState
private val userAnswerState: UserAnswerState,
private val consoleLogger: ConsoleLogger,
private val conceptCardTagHandlerFactory: ConceptCardTagHandler.Factory,
) {

private val adapterBuilder: BindableAdapter.MultiTypeBuilder<StateItemViewModel,
Expand Down Expand Up @@ -1400,7 +1436,9 @@ class StatePlayerRecyclerViewAssembler private constructor(
hasConversationView,
resourceHandler,
translationController,
userAnswerState
userAnswerState,
consoleLogger,
conceptCardTagHandlerFactory
)
if (playerFeatureSet.conceptCardSupport) {
customTagListener.proxyListener = assembler
Expand All @@ -1420,7 +1458,9 @@ class StatePlayerRecyclerViewAssembler private constructor(
private val resourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController,
private val multiAdapterBuilderFactory: BindableAdapter.MultiTypeBuilder.Factory,
private val singleAdapterFactory: BindableAdapter.SingleTypeBuilder.Factory
private val singleAdapterFactory: BindableAdapter.SingleTypeBuilder.Factory,
private val consoleLogger: ConsoleLogger,
private val conceptCardTagHandlerFactory: ConceptCardTagHandler.Factory,
) {
/**
* Returns a new [Builder] for the specified GCS resource bucket information for loading
Expand All @@ -1446,7 +1486,9 @@ class StatePlayerRecyclerViewAssembler private constructor(
translationController,
multiAdapterBuilderFactory,
singleAdapterFactory,
userAnswerState
userAnswerState,
consoleLogger,
conceptCardTagHandlerFactory
)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,35 @@
package org.oppia.android.app.player.state.itemviewmodel

import android.text.Spannable
import android.text.SpannableStringBuilder
import org.oppia.android.util.parser.html.CustomHtmlContentHandler

/** [StateItemViewModel] for content-card state. */
class ContentViewModel(
val htmlContent: CharSequence,
val gcsEntityId: String,
val hasConversationView: Boolean,
val isSplitView: Boolean,
val supportsConceptCards: Boolean
val supportsConceptCards: Boolean,
val customTagHandlers: Map<String, CustomHtmlContentHandler.CustomTagHandler>
) : StateItemViewModel(ViewType.CONTENT) {

private val underscoreRegex = Regex("(?<=\\s|[,.;?!])_{3,}(?=\\s|[,.;?!])")
private val replacementText = "Blank"

/** Returns content description by extracting text from [htmlContent]. */
fun getContentDescription(): String {
val contentDescription = CustomHtmlContentHandler.getContentDescription(
htmlContent.toString(),
imageRetriever = null,
customTagHandlers = customTagHandlers
)
return replaceRegexWithBlank(contentDescription)
}

/**
* Replaces "2+ underscores, with space/punctuation on both sides" in the input text with a
* replacement string "blank", returning a Spannable.
* Adjusts offsets to handle text length changes during replacements.
*/
fun replaceRegexWithBlank(inputText: CharSequence): Spannable {
val spannableStringBuilder = SpannableStringBuilder(inputText)
val matches = underscoreRegex.findAll(inputText)
var lengthOffset = 0

for (match in matches) {
val matchStart = match.range.first + lengthOffset
val matchEnd = match.range.last + 1 + lengthOffset
spannableStringBuilder.replace(matchStart, matchEnd, replacementText)

// Adjust offset due to change in length (difference between old and new text length)
lengthOffset += replacementText.length - (matchEnd - matchStart)
}
return spannableStringBuilder
}
private fun replaceRegexWithBlank(inputText: CharSequence): String =
underscoreRegex.replace(inputText, replacementText)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,38 @@ package org.oppia.android.app.player.state.itemviewmodel

import androidx.databinding.ObservableBoolean
import org.oppia.android.app.model.SubtitledHtml
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.viewmodel.ObservableViewModel
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.parser.html.CustomHtmlContentHandler

/** [ObservableViewModel] for MultipleChoiceInput values or ItemSelectionInput values. */
class SelectionInteractionContentViewModel(
val htmlContent: SubtitledHtml,
val hasConversationView: Boolean,
private val itemIndex: Int,
private val selectionInteractionViewModel: SelectionInteractionViewModel,
val isEnabled: ObservableBoolean
val isEnabled: ObservableBoolean,
val customTagHandlers: Map<String, CustomHtmlContentHandler.CustomTagHandler>,
private val writtenTranslationContext: WrittenTranslationContext,
private val translationController: TranslationController,
) : ObservableViewModel() {
var isAnswerSelected = ObservableBoolean()

/** Returns content description by extracting text from [htmlContent]. */
fun getContentDescription(): String {
val contentSubtitledHtml =
translationController.extractString(
htmlContent, writtenTranslationContext
)
return CustomHtmlContentHandler.getContentDescription(
contentSubtitledHtml,
imageRetriever = null,
customTagHandlers = customTagHandlers
)
}

/** Handles item click by updating the selection state based on user interaction. */
fun handleItemClicked() {
val isCurrentlySelected = isAnswerSelected.get()
val shouldNowBeSelected =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiv
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ObservableArrayList
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.parser.html.CUSTOM_IMG_TAG
import org.oppia.android.util.parser.html.CustomHtmlContentHandler
import org.oppia.android.util.parser.html.ImageTagHandler
import javax.inject.Inject

/** Corresponds to the type of input that should be used for an item selection interaction view. */
Expand Down Expand Up @@ -52,7 +56,8 @@ class SelectionInteractionViewModel private constructor(
val writtenTranslationContext: WrittenTranslationContext,
private val translationController: TranslationController,
private val resourceHandler: AppLanguageResourceHandler,
userAnswerState: UserAnswerState
userAnswerState: UserAnswerState,
consoleLogger: ConsoleLogger
) : StateItemViewModel(ViewType.SELECTION_INTERACTION), InteractionAnswerHandler {
private val interactionId: String = interaction.id

Expand Down Expand Up @@ -81,9 +86,20 @@ class SelectionInteractionViewModel private constructor(
ObservableBoolean(true)
}
}
val choiceItems: ObservableList<SelectionInteractionContentViewModel> =
computeChoiceItems(choiceSubtitledHtmls, hasConversationView, this, enabledItemsList)
private val customTagHandlers = mapOf<String, CustomHtmlContentHandler.CustomTagHandler>(
CUSTOM_IMG_TAG to ImageTagHandler(consoleLogger)
)

val choiceItems: ObservableList<SelectionInteractionContentViewModel> =
computeChoiceItems(
choiceSubtitledHtmls,
hasConversationView,
this,
enabledItemsList,
this@SelectionInteractionViewModel.writtenTranslationContext,
translationController,
customTagHandlers
)
private var pendingAnswerError: String? = null
private val isAnswerAvailable = ObservableField(false)
val errorMessage = ObservableField<String>("")
Expand Down Expand Up @@ -287,7 +303,8 @@ class SelectionInteractionViewModel private constructor(
/** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */
class FactoryImpl @Inject constructor(
private val translationController: TranslationController,
private val resourceHandler: AppLanguageResourceHandler
private val resourceHandler: AppLanguageResourceHandler,
private val consoleLogger: ConsoleLogger
) : InteractionItemFactory {
override fun create(
entityId: String,
Expand All @@ -310,7 +327,8 @@ class SelectionInteractionViewModel private constructor(
writtenTranslationContext,
translationController,
resourceHandler,
userAnswerState
userAnswerState,
consoleLogger
)
}
}
Expand All @@ -320,7 +338,10 @@ class SelectionInteractionViewModel private constructor(
choiceSubtitledHtmls: List<SubtitledHtml>,
hasConversationView: Boolean,
selectionInteractionViewModel: SelectionInteractionViewModel,
enabledItemsList: List<ObservableBoolean>
enabledItemsList: List<ObservableBoolean>,
writtenTranslationContext: WrittenTranslationContext,
translationController: TranslationController,
customTagHandlers: Map<String, CustomHtmlContentHandler.CustomTagHandler>
): ObservableArrayList<SelectionInteractionContentViewModel> {
val observableList = ObservableArrayList<SelectionInteractionContentViewModel>()
observableList += choiceSubtitledHtmls.mapIndexed { index, subtitledHtml ->
Expand All @@ -329,7 +350,10 @@ class SelectionInteractionViewModel private constructor(
hasConversationView = hasConversationView,
itemIndex = index,
selectionInteractionViewModel = selectionInteractionViewModel,
isEnabled = enabledItemsList[index]
isEnabled = enabledItemsList[index],
customTagHandlers,
writtenTranslationContext,
translationController
)
}
return observableList
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/layout/content_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
android:minWidth="48dp"
android:minHeight="48dp"
android:text="@{htmlContent}"
android:contentDescription="@{viewModel.replaceRegexWithBlank(htmlContent)}"
android:contentDescription="@{viewModel.getContentDescription()}"
android:textColor="@color/component_color_shared_primary_text_color"
android:textColorLink="@color/component_color_shared_link_text_color"
android:textSize="16sp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
android:layout_toEndOf="@+id/item_selection_checkbox"
android:fontFamily="sans-serif"
android:text="@{htmlContent}"
android:contentDescription="@{viewModel.getContentDescription()}"
android:textColor="@{viewModel.isEnabled ? @color/component_color_shared_item_selection_interaction_enabled_color : @color/component_color_shared_item_selection_interaction_disabled_color}"
android:textSize="16sp" />
</RelativeLayout>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
android:layout_toEndOf="@+id/multiple_choice_radio_button"
android:fontFamily="sans-serif"
android:text="@{htmlContent}"
android:contentDescription="@{viewModel.getContentDescription()}"
android:textColor="@{viewModel.isAnswerSelected() ? @color/component_color_shared_selection_interaction_selected_text_color : @color/component_color_shared_selection_interaction_unselected_text_color}"
android:textSize="16sp" />
</RelativeLayout>
Expand Down
Loading

0 comments on commit 2c6f8a8

Please sign in to comment.