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 8 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 conceptCardListener 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 conceptCardListener: 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()
customTagHandlers = mapOf(
CUSTOM_CONCEPT_CARD_TAG to ConceptCardTagHandler(
conceptCardListener,
consoleLogger
)
)
).toString()
}
}
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 conceptCardListener: ConceptCardTagHandler.ConceptCardLinkClickListener,
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,20 @@ class HintsAndSolutionViewModel private constructor(
resourceHandler.toHumanReadableString(hintIndex + 1)
),
hintSummary = translationController.extractString(
hint.hintContent, writtenTranslationContext
hint.hintContent,
writtenTranslationContext
),
isHintRevealed = isHintRevealed
isHintRevealed = isHintRevealed,
conceptCardListener = conceptCardListener,
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 +106,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 conceptCardListener: ConceptCardTagHandler.ConceptCardLinkClickListener,
private val consoleLogger: ConsoleLogger
) {
/**
* Returns a new [HintsAndSolutionViewModel] that populates a list of item view models (to be
Expand All @@ -114,7 +126,9 @@ class HintsAndSolutionViewModel private constructor(
writtenTranslationContext,
resourceHandler,
translationController,
solutionViewModelFactory
solutionViewModelFactory,
conceptCardListener,
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 lesson concept card test_skill_id_1."
)
)
)
Expand Down Expand Up @@ -2594,7 +2591,7 @@ class ExplorationActivityTest {

onView(withId(R.id.hints_and_solution_summary))
.inRoot(isDialog())
.perform(openClickableSpan("test_skill_id_1 concept card"))
.perform(openClickableSpan("lesson"))

testCoroutineDispatchers.runCurrent()

Expand Down Expand Up @@ -2904,7 +2901,7 @@ class ExplorationActivityTest {
MetricLogSchedulerModule::class, TestingBuildFlavorModule::class,
ActivityRouterModule::class,
CpuPerformanceSnapshotterModule::class, ExplorationProgressModule::class,
TestAuthenticationModule::class
TestAuthenticationModule::class,
]
)
interface TestApplicationComponent : ApplicationComponent {
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
Original file line number Diff line number Diff line change
Expand Up @@ -2184,7 +2184,7 @@ class StateFragmentLocalTest {
// Click on the link for opening the concept card.
onView(withId(R.id.hints_and_solution_summary))
.inRoot(isDialog())
.perform(openClickableSpan("test_skill_id_1 concept card"))
.perform(openClickableSpan("lesson"))
testCoroutineDispatchers.runCurrent()

onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed()))
Expand Down
2 changes: 1 addition & 1 deletion domain/src/main/assets/test_exp_id_2.json
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@
"hints": [{
"hint_content": {
"content_id": "hint_6",
"html": "<p>Remember that two halves, when added together, make one whole.</p><p>Click on this <oppia-noninteractive-skillreview skill_id-with-value=\"&amp;quot;test_skill_id_1&amp;quot;\" text-with-value=\"&amp;quot;test_skill_id_1 concept card&amp;quot;\"></oppia-noninteractive-skillreview>.</p>"
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
"html": "<p>Remember that two halves, when added together, make one whole.</p><p>Click on this <oppia-noninteractive-skillreview skill_id-with-value=\"&amp;quot;test_skill_id_1&amp;quot;\" text-with-value=\"&amp;quot;lesson&amp;quot;\"></oppia-noninteractive-skillreview>.</p>"
}
}],
"solution": {
Expand Down
2 changes: 1 addition & 1 deletion domain/src/main/assets/test_exp_id_2.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ states {
}
hint {
hint_content {
html: "<p>Remember that two halves, when added together, make one whole.</p><p>Click on this <oppia-noninteractive-skillreview skill_id-with-value=\"&amp;quot;test_skill_id_1&amp;quot;\" text-with-value=\"&amp;quot;test_skill_id_1 concept card&amp;quot;\"></oppia-noninteractive-skillreview>.</p>"
html: "<p>Remember that two halves, when added together, make one whole.</p><p>Click on this <oppia-noninteractive-skillreview skill_id-with-value=\"&amp;quot;test_skill_id_1&amp;quot;\" text-with-value=\"&amp;quot;lesson&amp;quot;\"></oppia-noninteractive-skillreview>.</p>"
content_id: "hint_6"
}
}
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
@@ -1,11 +1,21 @@
package org.oppia.android.util.parser.html

import android.view.View
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 provideConceptCardListener(): ConceptCardTagHandler.ConceptCardLinkClickListener {
return object : ConceptCardTagHandler.ConceptCardLinkClickListener {
override fun onConceptCardLinkClicked(view: View, skillId: String) {}
}
}

@Provides
@ExplorationHtmlParserEntityType
fun provideExplorationHtmlParserEntityType(): String {
Expand Down
Loading