Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 26 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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

/**
Expand All @@ -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
)
)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -114,7 +127,9 @@ class HintsAndSolutionViewModel private constructor(
writtenTranslationContext,
resourceHandler,
translationController,
solutionViewModelFactory
solutionViewModelFactory,
conceptCardTagHandlerFactory,
consoleLogger
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
)
)
Expand Down
16 changes: 0 additions & 16 deletions scripts/assets/test_file_exemptions.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
return object : ConceptCardLinkClickListener {
override fun onConceptCardLinkClicked(view: View, skillId: String) {}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
@Provides
@Singleton
fun provideConceptCardLinkClickListener(
factory: ConceptCardTagHandler.Factory
): ConceptCardTagHandler.ConceptCardLinkClickListener {
return factory.createConceptCardLinkClickListener()
}

@Provides
@ExplorationHtmlParserEntityType
fun provideExplorationHtmlParserEntityType(): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading