Fix #4097, Fix #4838, Fix #4170: Resolving TODOs (#5655)

## Explanation Fixes #4097, Fix #4848 and Fix #4170 This PR resolves the TODOs left in #5627, #5614 and #5647. 1. **Integrating `getContentDescription` in `HintViewModel`:** - Replaced `fromHtml` with `getContentDescription` introduced in #4848. - Added `ConceptCardTagHandler` to `customTagHandlers` for handling `CUSTOM_CONCEPT_CARD_TAG`. - Injected `ConceptCardLinkClickListener` into the `HintsAndSolutionViewModel.Factory`. 2. **Resolving TODOs for #4097:** - Removed TODOs and text-file-exemptions(in scripts/assets) for `FractionSubject`, `MathExpressionSubject`, `RealSubject`, and `MathEquationSubject`. 3. **Resolving TODOs for #4170 :** - Removed TODO comment from `MathTagHandler`. ## Essential Checklist  - [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)). --------- Co-authored-by: Adhiambo Peres <[email protected]>
oppia · Jan 28, 2025 · 4f58be9 · 4f58be9
1 parent f6d43d1
commit 4f58be9
Show file tree

Hide file tree

Showing 13 changed files with 70 additions and 46 deletions.
diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintViewModel.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintViewModel.kt
@@ -1,6 +1,9 @@
 package org.oppia.android.app.hintsandsolution
 
 import androidx.databinding.ObservableBoolean
+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.ConceptCardTagHandler
 import org.oppia.android.util.parser.html.CustomHtmlContentHandler
 
 /**
@@ -9,21 +12,30 @@ import org.oppia.android.util.parser.html.CustomHtmlContentHandler
  * @property title the title of this hint, relative to others (this is generated by the app)
  * @property hintSummary the core hint text (which may contain HTML) to show the user
  * @property isHintRevealed whether the hint is currently expanded and viewable
+ * @property conceptCardLinkClickListener listener for handling concept card clicks
+ * @property consoleLogger logger for handling any parsing errors
  */
 class HintViewModel(
   val title: String,
   val hintSummary: String,
-  val isHintRevealed: ObservableBoolean
+  val isHintRevealed: ObservableBoolean,
+  private val conceptCardLinkClickListener: ConceptCardTagHandler.ConceptCardLinkClickListener,
+  private val consoleLogger: ConsoleLogger
 ) : HintsAndSolutionItemViewModel() {
   /**
    * A screenreader-friendly version of [hintSummary] that should be used for readout, in place of
    * the original summary.
    */
   val hintContentDescription: String by lazy {
-    CustomHtmlContentHandler.fromHtml(
+    CustomHtmlContentHandler.getContentDescription(
       hintSummary,
       imageRetriever = null,
-      customTagHandlers = mapOf()
-    ).toString()
+      customTagHandlers = mapOf(
+        CUSTOM_CONCEPT_CARD_TAG to ConceptCardTagHandler(
+          conceptCardLinkClickListener,
+          consoleLogger
+        )
+      )
+    )
   }
 }
diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionViewModel.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionViewModel.kt
@@ -14,6 +14,8 @@ import org.oppia.android.domain.hintsandsolution.isHintRevealed
 import org.oppia.android.domain.hintsandsolution.isSolutionAvailable
 import org.oppia.android.domain.hintsandsolution.isSolutionRevealed
 import org.oppia.android.domain.translation.TranslationController
+import org.oppia.android.util.logging.ConsoleLogger
+import org.oppia.android.util.parser.html.ConceptCardTagHandler
 import javax.inject.Inject
 
 /**
@@ -27,7 +29,9 @@ class HintsAndSolutionViewModel private constructor(
   private val writtenTranslationContext: WrittenTranslationContext,
   private val resourceHandler: AppLanguageResourceHandler,
   private val translationController: TranslationController,
-  private val solutionViewModelFactory: SolutionViewModel.Factory
+  private val solutionViewModelFactory: SolutionViewModel.Factory,
+  private val conceptCardTagHandlerFactory: ConceptCardTagHandler.Factory,
+  private val consoleLogger: ConsoleLogger
 ) : ObservableViewModel() {
   private val hintList by lazy { helpIndex.dropLastUnavailable(state.interaction.hintList) }
   private val solution by lazy {
@@ -57,7 +61,9 @@ class HintsAndSolutionViewModel private constructor(
   private fun createViewModels(): List<HintsAndSolutionItemViewModel> {
     return hintList.mapIndexed { index, hint ->
       createHintViewModel(
-        index, hint, isHintRevealed = ObservableBoolean(helpIndex.isHintRevealed(index, hintList))
+        index,
+        hint,
+        isHintRevealed = ObservableBoolean(helpIndex.isHintRevealed(index, hintList))
       )
     } + listOfNotNull(solution?.let(this::createSolutionViewModel)) + ReturnToLessonViewModel
   }
@@ -73,16 +79,21 @@ class HintsAndSolutionViewModel private constructor(
         resourceHandler.toHumanReadableString(hintIndex + 1)
       ),
       hintSummary = translationController.extractString(
-        hint.hintContent, writtenTranslationContext
+        hint.hintContent,
+        writtenTranslationContext
       ),
-      isHintRevealed = isHintRevealed
+      isHintRevealed = isHintRevealed,
+      conceptCardLinkClickListener =
+        conceptCardTagHandlerFactory.createConceptCardLinkClickListener(),
+      consoleLogger = consoleLogger
     )
   }
 
   private fun createSolutionViewModel(solution: Solution): SolutionViewModel {
     return solutionViewModelFactory.create(
       solutionSummary = translationController.extractString(
-        solution.explanation, writtenTranslationContext
+        solution.explanation,
+        writtenTranslationContext
       ),
       isSolutionRevealed = isSolutionRevealed,
       isSolutionExclusive = solution.answerIsExclusive,
@@ -96,7 +107,9 @@ class HintsAndSolutionViewModel private constructor(
   class Factory @Inject constructor(
     private val resourceHandler: AppLanguageResourceHandler,
     private val translationController: TranslationController,
-    private val solutionViewModelFactory: SolutionViewModel.Factory
+    private val solutionViewModelFactory: SolutionViewModel.Factory,
+    private val conceptCardTagHandlerFactory: ConceptCardTagHandler.Factory,
+    private val consoleLogger: ConsoleLogger
   ) {
     /**
      * Returns a new [HintsAndSolutionViewModel] that populates a list of item view models (to be
@@ -114,7 +127,9 @@ class HintsAndSolutionViewModel private constructor(
         writtenTranslationContext,
         resourceHandler,
         translationController,
-        solutionViewModelFactory
+        solutionViewModelFactory,
+        conceptCardTagHandlerFactory,
+        consoleLogger
       )
     }
   }

diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt
@@ -2288,15 +2288,12 @@ class ExplorationActivityTest {
       openHintsAndSolutionsDialog()
       pressRevealHintButton(hintPosition = 0)
 
-      // TODO(#4848): Fix content description generation & update this test to verify using the
-      //  correct text.
-      // Ensure the hint description is correct and doesn't contain any HTML.
       onView(withId(R.id.hints_and_solution_summary))
         .check(
           matches(
             withContentDescription(
-              "Remember that two halves, when added together," +
-                " make one whole.\n\nClick on this .\n\n"
+              "Remember that two halves, when added together, make one whole." +
+                "\nClick on this test_skill_id_1 concept card."
             )
           )
         )

diff --git a/.../sharedTest/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityTest.kt b/.../sharedTest/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityTest.kt
@@ -457,7 +457,7 @@ class QuestionPlayerActivityTest {
           matches(
             withContentDescription(
               "To write a fraction, you need to know its denominator, which is the total " +
-                "number of pieces in the whole. All of these pieces should be the same size.\n\n"
+                "number of pieces in the whole. All of these pieces should be the same size."
             )
           )
         )

diff --git a/scripts/assets/test_file_exemptions.textproto b/scripts/assets/test_file_exemptions.textproto
@@ -4102,18 +4102,6 @@ test_file_exemption {
   exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/ComparableOperationSubject.kt"
   test_file_not_required: true
 }
-test_file_exemption {
-  exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/FractionSubject.kt"
-  test_file_not_required: true
-}
-test_file_exemption {
-  exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathEquationSubject.kt"
-  test_file_not_required: true
-}
-test_file_exemption {
-  exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathExpressionSubject.kt"
-  test_file_not_required: true
-}
 test_file_exemption {
   exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathParsingErrorSubject.kt"
   test_file_not_required: true
@@ -4122,10 +4110,6 @@ test_file_exemption {
   exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/PolynomialSubject.kt"
   test_file_not_required: true
 }
-test_file_exemption {
-  exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/RealSubject.kt"
-  test_file_not_required: true
-}
 test_file_exemption {
   exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/TokenSubject.kt"
   test_file_not_required: true

diff --git a/testing/src/main/java/org/oppia/android/testing/math/FractionSubject.kt b/testing/src/main/java/org/oppia/android/testing/math/FractionSubject.kt
@@ -10,8 +10,6 @@ import com.google.common.truth.extensions.proto.LiteProtoSubject
 import org.oppia.android.app.model.Fraction
 import org.oppia.android.util.math.toDouble
 
-// TODO(#4097): Add tests for this class.
-
 /**
  * Truth subject for verifying properties of [Fraction]s.
  *

diff --git a/testing/src/main/java/org/oppia/android/testing/math/MathEquationSubject.kt b/testing/src/main/java/org/oppia/android/testing/math/MathEquationSubject.kt
@@ -10,8 +10,6 @@ import org.oppia.android.testing.math.MathEquationSubject.Companion.assertThat
 import org.oppia.android.testing.math.MathExpressionSubject.Companion.assertThat
 import org.oppia.android.util.math.toRawLatex
 
-// TODO(#4097): Add tests for this class.
-
 /**
  * Truth subject for verifying properties of [MathEquation]s.
  *

diff --git a/testing/src/main/java/org/oppia/android/testing/math/MathExpressionSubject.kt b/testing/src/main/java/org/oppia/android/testing/math/MathExpressionSubject.kt
@@ -23,8 +23,6 @@ import org.oppia.android.testing.math.RealSubject.Companion.assertThat
 import org.oppia.android.util.math.evaluateAsNumericExpression
 import org.oppia.android.util.math.toRawLatex
 
-// TODO(#4097): Add tests for this class.
-
 /**
  * Truth subject for verifying properties of [MathExpression]s.
  *

diff --git a/testing/src/main/java/org/oppia/android/testing/math/RealSubject.kt b/testing/src/main/java/org/oppia/android/testing/math/RealSubject.kt
@@ -10,8 +10,6 @@ import com.google.common.truth.extensions.proto.LiteProtoSubject
 import org.oppia.android.app.model.Real
 import org.oppia.android.testing.math.FractionSubject.Companion.assertThat
 
-// TODO(#4097): Add tests for this class.
-
 /**
  * Truth subject for verifying properties of [Real]s.
  *

diff --git a/utility/src/main/java/org/oppia/android/util/parser/html/ConceptCardTagHandler.kt b/utility/src/main/java/org/oppia/android/util/parser/html/ConceptCardTagHandler.kt
@@ -7,6 +7,7 @@ import android.text.style.ClickableSpan
 import android.view.View
 import org.oppia.android.util.logging.ConsoleLogger
 import org.xml.sax.Attributes
+import javax.inject.Inject
 
 /** The custom tag corresponding to [ConceptCardTagHandler]. */
 const val CUSTOM_CONCEPT_CARD_TAG = "oppia-noninteractive-skillreview"
@@ -52,10 +53,25 @@ class ConceptCardTagHandler(
   }
 
   override fun getContentDescription(attributes: Attributes): String? {
-    val skillId = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_SKILL_ID)
     val text = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_TEXT_VALUE)
-    return if (!skillId.isNullOrBlank() && !text.isNullOrBlank()) {
-      "$text concept card $skillId"
+    return if (!text.isNullOrBlank()) {
+      text
     } else ""
   }
+
+  /** Application-injectable factory for creating [ConceptCardTagHandler]s). */
+  class Factory @Inject constructor() {
+    /**
+     * Creates a new [ConceptCardLinkClickListener] for handling concept card link click events.
+     *
+     * This [ConceptCardLinkClickListener] defines behavior for handling user interactions with links
+     * displayed in concept cards. The `onConceptCardLinkClicked` method provides the clicked [View]
+     * and the associated skill ID to enable further action handling.
+     */
+    fun createConceptCardLinkClickListener(): ConceptCardLinkClickListener {
+      return object : ConceptCardLinkClickListener {
+        override fun onConceptCardLinkClicked(view: View, skillId: String) {}
+      }
+    }
+  }
 }
diff --git a/utility/src/main/java/org/oppia/android/util/parser/html/HtmlParserEntityTypeModule.kt b/utility/src/main/java/org/oppia/android/util/parser/html/HtmlParserEntityTypeModule.kt
@@ -2,10 +2,19 @@ package org.oppia.android.util.parser.html
 
 import dagger.Module
 import dagger.Provides
+import javax.inject.Singleton
 
 /** Provides Html parsing entity type dependencies. */
 @Module
 class HtmlParserEntityTypeModule {
+  @Provides
+  @Singleton
+  fun provideConceptCardLinkClickListener(
+    factory: ConceptCardTagHandler.Factory
+  ): ConceptCardTagHandler.ConceptCardLinkClickListener {
+    return factory.createConceptCardLinkClickListener()
+  }
+
   @Provides
   @ExplorationHtmlParserEntityType
   fun provideExplorationHtmlParserEntityType(): String {

diff --git a/utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt b/utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt
@@ -44,7 +44,6 @@ class MathTagHandler(
     val content = MathContent.parseMathContent(
       attributes.getJsonObjectValue(CUSTOM_MATH_MATH_CONTENT_ATTRIBUTE)
     )
-    // TODO(#4170): Fix vertical alignment centering for inline cached LaTeX.
     val useInlineRendering = when (attributes.getValue(CUSTOM_MATH_RENDER_TYPE_ATTRIBUTE)) {
       "inline" -> true
       "block" -> false

diff --git a/utility/src/test/java/org/oppia/android/util/parser/html/ConceptCardTagHandlerTest.kt b/utility/src/test/java/org/oppia/android/util/parser/html/ConceptCardTagHandlerTest.kt
@@ -125,7 +125,7 @@ class ConceptCardTagHandlerTest {
         imageRetriever = mockImageRetriever,
         customTagHandlers = tagHandlersWithConceptCardSupport
       )
-    assertThat(contentDescription).isEqualTo("refresher lesson concept card skill_id_1")
+    assertThat(contentDescription).isEqualTo("refresher lesson")
   }
 
   @Test