From 893c7a25f613102682495f216c8d0254a8c87375 Mon Sep 17 00:00:00 2001 From: Ayush Yadav <143514610+theayushyadav11@users.noreply.github.com> Date: Thu, 13 Feb 2025 16:59:01 +0530 Subject: [PATCH 1/6] Fix #4121: Added tests for TokenSubject (#5669) Fixes #4121 This pull request introduces a new test class `TokenSubjectTest` to the `oppia_android_test` suite, focusing on validating various aspects of the `Token` class from the `MathTokenizer` utility. The primary changes include the addition of the test class itself and the corresponding Bazel build rule. ### Key Changes: #### Addition of Test Class: * [`testing/src/test/java/org/oppia/android/testing/math/TokenSubjectTest.kt`](diffhunk://#diff-aeecd58bdeac384c551e2a73a6debca08e2bdfb0492eb84007773ed78af057d0R1-R180): Added a new test class `TokenSubjectTest` with multiple test methods to verify the correctness of different `Token` types and their properties. #### Bazel Build Rule: * [`testing/src/test/java/org/oppia/android/testing/math/BUILD.bazel`](diffhunk://#diff-8a86bceb0e654069d42dcb4e76a52f65aeea3bc7d76a9cdff53f5e4bdde93f08R70-R85): Added a new `oppia_android_test` target for `TokenSubjectTest`, specifying its dependencies and configuration. #### Screenshot of the passing tests. ![image](https://github.com/user-attachments/assets/49628814-1616-45be-ae64-2f95aa5ce298) ## 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: theayushyadav11 --- scripts/assets/test_file_exemptions.textproto | 4 - .../android/testing/math/TokenSubject.kt | 2 - .../oppia/android/testing/math/BUILD.bazel | 14 ++ .../android/testing/math/TokenSubjectTest.kt | 217 ++++++++++++++++++ 4 files changed, 231 insertions(+), 6 deletions(-) create mode 100644 testing/src/test/java/org/oppia/android/testing/math/TokenSubjectTest.kt diff --git a/scripts/assets/test_file_exemptions.textproto b/scripts/assets/test_file_exemptions.textproto index 94ebe621a53..02d22431745 100644 --- a/scripts/assets/test_file_exemptions.textproto +++ b/scripts/assets/test_file_exemptions.textproto @@ -4106,10 +4106,6 @@ test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathParsingErrorSubject.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 -} test_file_exemption { exempted_file_path: "testing/src/main/java/org/oppia/android/testing/mockito/MockitoKotlinHelper.kt" test_file_not_required: true diff --git a/testing/src/main/java/org/oppia/android/testing/math/TokenSubject.kt b/testing/src/main/java/org/oppia/android/testing/math/TokenSubject.kt index 737de61a7b9..49cce0f2030 100644 --- a/testing/src/main/java/org/oppia/android/testing/math/TokenSubject.kt +++ b/testing/src/main/java/org/oppia/android/testing/math/TokenSubject.kt @@ -26,8 +26,6 @@ import org.oppia.android.util.math.MathTokenizer.Companion.Token.RightParenthesi import org.oppia.android.util.math.MathTokenizer.Companion.Token.SquareRootSymbol import org.oppia.android.util.math.MathTokenizer.Companion.Token.VariableName -// TODO(#4121): Add tests for this class. - /** * Truth subject for verifying properties of [Token]s. * diff --git a/testing/src/test/java/org/oppia/android/testing/math/BUILD.bazel b/testing/src/test/java/org/oppia/android/testing/math/BUILD.bazel index 52d56ecfcfd..63cb143e73e 100644 --- a/testing/src/test/java/org/oppia/android/testing/math/BUILD.bazel +++ b/testing/src/test/java/org/oppia/android/testing/math/BUILD.bazel @@ -82,3 +82,17 @@ oppia_android_test( "//third_party:robolectric_android-all", ], ) + +oppia_android_test( + name = "TokenSubjectTest", + srcs = ["TokenSubjectTest.kt"], + custom_package = "org.oppia.android.testing.math", + test_class = "org.oppia.android.testing.math.TokenSubjectTest", + test_manifest = "//testing:test_manifest", + deps = [ + "//testing/src/main/java/org/oppia/android/testing/math:token_subject", + "//third_party:com_google_truth_truth", + "//third_party:junit_junit", + "//third_party:robolectric_android-all", + ], +) diff --git a/testing/src/test/java/org/oppia/android/testing/math/TokenSubjectTest.kt b/testing/src/test/java/org/oppia/android/testing/math/TokenSubjectTest.kt new file mode 100644 index 00000000000..a50967aa57d --- /dev/null +++ b/testing/src/test/java/org/oppia/android/testing/math/TokenSubjectTest.kt @@ -0,0 +1,217 @@ +package org.oppia.android.testing.math + +import org.junit.Assert.assertThrows +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.oppia.android.util.math.MathTokenizer.Companion.Token + +/** Tests for [TokenSubject]. */ +@RunWith(JUnit4::class) +class TokenSubjectTest { + + @Test + fun testTokenSubject_passesWithCorrectStartIndex() { + val token = Token.PositiveInteger(42, 10, 15) + TokenSubject.assertThat(token).hasStartIndexThat().isEqualTo(10) + } + + @Test + fun testTokenSubject_failsWithIncorrectStartIndex() { + val token = Token.PositiveInteger(42, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).hasStartIndexThat().isEqualTo(11) + } + } + + @Test + fun testTokenSubject_passesWithCorrectEndIndex() { + val token = Token.PositiveInteger(42, 10, 15) + TokenSubject.assertThat(token).hasEndIndexThat().isEqualTo(15) + } + + @Test + fun testTokenSubject_failsWithIncorrectEndIndex() { + val token = Token.PositiveInteger(42, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).hasEndIndexThat().isEqualTo(14) + } + } + + @Test + fun testTokenSubject_passesWithCorrectPositiveIntegerValue() { + val token = Token.PositiveInteger(42, 10, 15) + TokenSubject.assertThat(token).isPositiveIntegerWhoseValue().isEqualTo(42) + } + + @Test + fun testTokenSubject_failsWithIncorrectPositiveIntegerValue() { + val token = Token.PositiveInteger(42, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).isPositiveIntegerWhoseValue().isEqualTo(45) + } + } + + @Test + fun testTokenSubject_passesWithCoreectPositiveRealNumberValue() { + val token = Token.PositiveRealNumber(3.14, 10, 15) + TokenSubject.assertThat(token).isPositiveRealNumberWhoseValue().isEqualTo(3.14) + } + + fun testTokenSubject_failsWithIncorrectPositiveRealNumberValue() { + val token = Token.PositiveRealNumber(3.14, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).isPositiveRealNumberWhoseValue().isEqualTo(25) + } + } + + @Test + fun testTokenSubject_passesWithCorrectVariableName() { + val token = Token.VariableName("x", 10, 15) + TokenSubject.assertThat(token).isVariableWhoseName().isEqualTo("x") + } + + @Test + fun testTokenSubject_isVariableWhoseName_incorrectName_fails() { + val token = Token.VariableName("x", 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).isVariableWhoseName().isEqualTo("y") + } + } + + @Test + fun testTokenSubject_passesWithCorrectFunctionNameAndAllowedStatus() { + val token = Token.FunctionName("sqrt", true, 10, 15) + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasNameThat().isEqualTo("sqrt") + } + + @Test + fun testTokenSubject_failsWithIncorrectFunctionNameAndAllowedStatus() { + val token = Token.FunctionName("sqrt", true, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasNameThat().isEqualTo("sine") + } + } + + @Test + fun testTokenSubject_symbolIsMinusSymbol() { + val token = Token.MinusSymbol(10, 11) + TokenSubject.assertThat(token).isMinusSymbol() + } + + @Test + fun testTokenSubject_symbolIsSquareRootSymbol() { + val token = Token.SquareRootSymbol(10, 11) + TokenSubject.assertThat(token).isSquareRootSymbol() + } + + @Test + fun testTokenSubject_symbolIsPlusSymbol() { + val token = Token.PlusSymbol(10, 11) + TokenSubject.assertThat(token).isPlusSymbol() + } + + @Test + fun testTokenSubject_symbolIsMultiplySymbol() { + val token = Token.MultiplySymbol(10, 11) + TokenSubject.assertThat(token).isMultiplySymbol() + } + + @Test + fun testTokenSubject_symbolIsDivideSymbol() { + val token = Token.DivideSymbol(10, 11) + TokenSubject.assertThat(token).isDivideSymbol() + } + + @Test + fun testTokenSubject_symbolIsExponentiationSymbol() { + val token = Token.ExponentiationSymbol(10, 11) + TokenSubject.assertThat(token).isExponentiationSymbol() + } + + @Test + fun testTokenSubject_symbolIsEqualsSymbol() { + val token = Token.EqualsSymbol(10, 11) + TokenSubject.assertThat(token).isEqualsSymbol() + } + + @Test + fun testTokenSubject_symbolIsLeftParenthesisSymbol() { + val token = Token.LeftParenthesisSymbol(10, 11) + TokenSubject.assertThat(token).isLeftParenthesisSymbol() + } + + @Test + fun testTokenSubject_symbolIsRightParenthesisSymbol() { + val token = Token.RightParenthesisSymbol(10, 11) + TokenSubject.assertThat(token).isRightParenthesisSymbol() + } + + @Test + fun testTokenSubject_failsWithIncorrectSymbol() { + val token = Token.RightParenthesisSymbol(10, 11) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).isMinusSymbol() + } + } + + @Test + fun testTokenSubject_checkIsInvalidToken_passes() { + val token = Token.InvalidToken(10, 11) + TokenSubject.assertThat(token).isInvalidToken() + } + + @Test + fun testTokenSubject_checkIsInvalidToken_fails() { + val token = Token.PositiveInteger(10, 11, 42) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token).isInvalidToken() + } + } + + @Test + fun testTokenSubject_checkIsIncompleteFunctionName() { + val token = Token.IncompleteFunctionName(10, 11) + TokenSubject.assertThat(token).isIncompleteFunctionName() + } + + @Test + fun testTokenSubject_functionNameSubject_nameCheck_passes() { + val token = Token.FunctionName("sqrt", true, 10, 15) + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasNameThat().isEqualTo("sqrt") + } + + @Test + fun testTokenSubject_functionNameSubject_nameCheck_fails() { + val token = Token.FunctionName("sqrt", true, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasNameThat().isEqualTo("sin") + } + } + + @Test + fun testTokenSubject_functionNameSubject_allowedPropertyCheck_passes() { + val token = Token.FunctionName("sqrt", true, 10, 15) + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasIsAllowedPropertyThat().isTrue() + } + + @Test + fun testTokenSubject_functionNameSubject_allowedPropertyCheck_fails() { + val token = Token.FunctionName("sqrt", false, 10, 15) + assertThrows(AssertionError::class.java) { + TokenSubject.assertThat(token) + .isFunctionNameThat() + .hasIsAllowedPropertyThat().isTrue() + } + } +} From 3eb4fb58187b464b9d5b81c8ec1fa7cb09857b38 Mon Sep 17 00:00:00 2001 From: "translatewiki.net" Date: Thu, 13 Feb 2025 19:15:27 +0530 Subject: [PATCH 2/6] Localisation updates from https://translatewiki.net. (#5701) Translation updates --- app/src/main/res/values-ar/strings.xml | 6 +++--- app/src/main/res/values-pcm-rNG/strings.xml | 4 +--- app/src/main/res/values-pt-rBR/strings.xml | 4 +--- app/src/main/res/values-sw/strings.xml | 4 +--- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/src/main/res/values-ar/strings.xml b/app/src/main/res/values-ar/strings.xml index fe5f53dd75e..5f928cf9c0f 100644 --- a/app/src/main/res/values-ar/strings.xml +++ b/app/src/main/res/values-ar/strings.xml @@ -39,7 +39,7 @@ بطاقة المفهوم بطاقة المفاهيم 1 بطاقة المفاهيم 2 - بطاقة المراجعة + دليل الدراسة هل تريد المغادرة إلى صفحة الموضوع؟ لن يتم حفظ تقدمك. مغادرة @@ -90,9 +90,9 @@ مقدمة الأسئلة الأكثر تكرارا معلومات - الدروس + تعلم ممارسة - مراجعة + يذاكر أدوات تحكم المشرف صفحة الموضوع الموضوع: %s diff --git a/app/src/main/res/values-pcm-rNG/strings.xml b/app/src/main/res/values-pcm-rNG/strings.xml index 0774dae0c11..2271b7f790f 100644 --- a/app/src/main/res/values-pcm-rNG/strings.xml +++ b/app/src/main/res/values-pcm-rNG/strings.xml @@ -36,7 +36,7 @@ Concept Card Concept Card 1 Concept Card 2 - Revision Card + Revision Card Comot go the topic page? Wetin you don do before no go save Comot @@ -86,9 +86,7 @@ Introduction Frequently Asked Questions (FAQs) Info - Lessons Practice - Revision Administrator Controls Topic page Topic: %s diff --git a/app/src/main/res/values-pt-rBR/strings.xml b/app/src/main/res/values-pt-rBR/strings.xml index 63042bc8413..98e6088fff3 100644 --- a/app/src/main/res/values-pt-rBR/strings.xml +++ b/app/src/main/res/values-pt-rBR/strings.xml @@ -45,7 +45,7 @@ Cartão de Conceito Cartão Conceitual 1 Cartão Conceitual 2 - Cartão de Revisão + Cartão de Revisão Pretende ir para a página do tópico? Seu progresso não será salvo. Sair @@ -96,9 +96,7 @@ Introdução Perguntas Frequentes (FAQs) Info - Lições Prática - Revisão Controles do administrador Página do tópico Tópico: %s diff --git a/app/src/main/res/values-sw/strings.xml b/app/src/main/res/values-sw/strings.xml index f827cae27ef..4ec916e993c 100644 --- a/app/src/main/res/values-sw/strings.xml +++ b/app/src/main/res/values-sw/strings.xml @@ -31,7 +31,7 @@ Kutiririsha sauti kunaweza kutumia data nyingi ya mtandao wa simu. Usionyeshe ujumbe huu tena Kadi ya Dhana - Kadi ya Marudio + Kadi ya Marudio Je, ungependa kwenda kwenye Ukurasa wa Mada? Maendeleo yako hayatahifadhiwa. Toka @@ -69,9 +69,7 @@ Utangulizi maswali yanayoulizwa mara kwa mara habari - Masomo Mazoezi - Mazoezi Vidhibiti vya Msimamizi Ukurasa wa mada mada: %s From 4a19f239ce63268d813601a912eb6105027ce84e Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 13 Feb 2025 13:31:55 -0800 Subject: [PATCH 3/6] Fix #5699: Update wiki deployment permissions (#5700) ## Explanation Fixes #5699 This attempts to fix #5699 by updating the job permissions for wiki deployment to include access to content writing (which is now off by default at the Oppia organization level--see #5699). It's not clear whether this will fix the issue since GitHub's documentation is unclear as to which permissions are required in order to be able to push to the wiki, but it seems likely that 'contents' is the correct permission per https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/controlling-permissions-for-github_token#overview. Unfortunately, the fix can't be verified until after the PR is merged as the deployment job only runs manually or upon a PR merging. Note that the permission is intentionally being set job-specific to avoid over-subscribing permissions at the workflow level (since the other job and potentially future jobs in this workflow do not need content write access). ## 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)). ## For UI-specific PRs only N/A -- this is an infrastructure-only change that also only affects wiki deployment and verification workflows. --- .github/workflows/wiki.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/wiki.yml b/.github/workflows/wiki.yml index 5816c9a7e9a..67da9814fe8 100644 --- a/.github/workflows/wiki.yml +++ b/.github/workflows/wiki.yml @@ -36,6 +36,8 @@ jobs: bazel run //scripts:wiki_table_of_contents_check -- ${GITHUB_WORKSPACE} wiki-deploy: + permissions: + contents: write runs-on: ${{ matrix.os }} strategy: matrix: From 38419bf486fa487be19f08e668c58911413ec6cb Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Sun, 16 Feb 2025 22:10:46 -0800 Subject: [PATCH 4/6] Fix part of #5699: Enable workflow dispatching for wiki jobs (#5703) ## Explanation Fixes part of #5699 (even though it's already expected to be fixed). This was missed in #5700. We can't verify the fixes from that PR without the ability to re-run the workflow outside of a PR (or until another PR that modifies the wiki is merged). It seems ideal to have the ability to force-refresh the wiki in this way, anyway. Similar to #5700, this can't be easily verified until it's merged. ## 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)). ## For UI-specific PRs only N/A -- This only affects GitHub actions wiki workflows. --- .github/workflows/wiki.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/wiki.yml b/.github/workflows/wiki.yml index 67da9814fe8..c5e92d52156 100644 --- a/.github/workflows/wiki.yml +++ b/.github/workflows/wiki.yml @@ -1,6 +1,7 @@ name: Deploy to Wiki on: + workflow_dispatch: pull_request: paths: - 'wiki/**' From 2c6f8a87735ea15fc4fc702c547bcf4ed21d1a21 Mon Sep 17 00:00:00 2001 From: Manas <119405883+manas-yu@users.noreply.github.com> Date: Mon, 17 Feb 2025 14:45:00 +0530 Subject: [PATCH 5/6] Fix #4848 Content Description Generation for Content and SelectionInteractionContent ViewModels (#5704) ## Explanation 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: `Click here`, 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 - [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)). --- .../state/StatePlayerRecyclerViewAssembler.kt | 54 +++++++++++-- .../state/itemviewmodel/ContentViewModel.kt | 33 ++++---- .../SelectionInteractionContentViewModel.kt | 22 +++++- .../SelectionInteractionViewModel.kt | 38 +++++++-- app/src/main/res/layout/content_item.xml | 2 +- .../item_selection_interaction_items.xml | 1 + .../multiple_choice_interaction_items.xml | 1 + .../app/player/state/StateFragmentTest.kt | 78 ++++++++++++++++++- .../parser/html/CustomHtmlContentHandler.kt | 34 ++++++-- .../android/util/parser/html/LiTagHandler.kt | 7 +- .../html/CustomHtmlContentHandlerTest.kt | 18 +++++ .../util/parser/html/LiTagHandlerTest.kt | 51 ------------ 12 files changed, 238 insertions(+), 101 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt index 8f1e627d77a..5126bd59028 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt @@ -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 @@ -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 @@ -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 @@ -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(false) override fun onConceptCardLinkClicked(view: View, skillId: String) { @@ -350,7 +383,8 @@ class StatePlayerRecyclerViewAssembler private constructor( gcsEntityId, hasConversationView, isSplitView.get()!!, - playerFeatureSet.conceptCardSupport + playerFeatureSet.conceptCardSupport, + customTagHandlers ) } } @@ -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(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) } diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionContentViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionContentViewModel.kt index 04fea533655..71eabdf43ea 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionContentViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionContentViewModel.kt @@ -2,7 +2,10 @@ 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( @@ -10,10 +13,27 @@ class SelectionInteractionContentViewModel( val hasConversationView: Boolean, private val itemIndex: Int, private val selectionInteractionViewModel: SelectionInteractionViewModel, - val isEnabled: ObservableBoolean + val isEnabled: ObservableBoolean, + val customTagHandlers: Map, + 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 = diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt index d5f4b7016c7..a0604b8d326 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt @@ -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. */ @@ -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 @@ -81,9 +86,20 @@ class SelectionInteractionViewModel private constructor( ObservableBoolean(true) } } - val choiceItems: ObservableList = - computeChoiceItems(choiceSubtitledHtmls, hasConversationView, this, enabledItemsList) + private val customTagHandlers = mapOf( + CUSTOM_IMG_TAG to ImageTagHandler(consoleLogger) + ) + val choiceItems: ObservableList = + computeChoiceItems( + choiceSubtitledHtmls, + hasConversationView, + this, + enabledItemsList, + this@SelectionInteractionViewModel.writtenTranslationContext, + translationController, + customTagHandlers + ) private var pendingAnswerError: String? = null private val isAnswerAvailable = ObservableField(false) val errorMessage = ObservableField("") @@ -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, @@ -310,7 +327,8 @@ class SelectionInteractionViewModel private constructor( writtenTranslationContext, translationController, resourceHandler, - userAnswerState + userAnswerState, + consoleLogger ) } } @@ -320,7 +338,10 @@ class SelectionInteractionViewModel private constructor( choiceSubtitledHtmls: List, hasConversationView: Boolean, selectionInteractionViewModel: SelectionInteractionViewModel, - enabledItemsList: List + enabledItemsList: List, + writtenTranslationContext: WrittenTranslationContext, + translationController: TranslationController, + customTagHandlers: Map ): ObservableArrayList { val observableList = ObservableArrayList() observableList += choiceSubtitledHtmls.mapIndexed { index, subtitledHtml -> @@ -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 diff --git a/app/src/main/res/layout/content_item.xml b/app/src/main/res/layout/content_item.xml index a29a44814fe..192d0a42642 100644 --- a/app/src/main/res/layout/content_item.xml +++ b/app/src/main/res/layout/content_item.xml @@ -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" diff --git a/app/src/main/res/layout/item_selection_interaction_items.xml b/app/src/main/res/layout/item_selection_interaction_items.xml index 7fe2e6a5ba5..c7537f23e0e 100755 --- a/app/src/main/res/layout/item_selection_interaction_items.xml +++ b/app/src/main/res/layout/item_selection_interaction_items.xml @@ -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" /> diff --git a/app/src/main/res/layout/multiple_choice_interaction_items.xml b/app/src/main/res/layout/multiple_choice_interaction_items.xml index dcbf7239e1f..997f88601e3 100755 --- a/app/src/main/res/layout/multiple_choice_interaction_items.xml +++ b/app/src/main/res/layout/multiple_choice_interaction_items.xml @@ -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" /> diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt index 5f436a48dc7..50003cdc606 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt @@ -5230,16 +5230,80 @@ class StateFragmentTest { playThroughRatioExplorationState13() playThroughRatioExplorationState14() - val expectedDescription = "James turned the page, and saw a recipe for banana smoothie." + - " Yummy!\n\n2 cups of milk and 1 cup of banana puree \n\n“I can make this,” he said." + - " “We’ll need to mix milk and banana puree in the ratio Blank.”\n\nCan you complete" + - " James’s sentence? What is the ratio of milk to banana puree?”" + val expectedDescription = "James turned the page, and saw a recipe for banana smoothie. " + + "Yummy!\nImage illustrating 2 cups of milk and 1 cup of banana puree\n“I can make this," + + "” he said. “We’ll need to mix milk and banana puree in the ratio Blank.”\nCan you " + + "complete James’s sentence? What is the ratio of milk to banana puree?”" onView(withId(R.id.content_text_view)) .check(matches(withContentDescription(expectedDescription))) } } + @Test + fun testStateFragment_contentDescription_itemSelectionInteraction() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_2, shouldSavePartialProgress = false).use { + startPlayingExploration() + playThroughPrototypeState1() + playThroughPrototypeState2() + playThroughPrototypeState3() + playThroughPrototypeState4() + verifyViewTypeIsPresent(SELECTION_INTERACTION) + + onView( + atPositionOnView( + recyclerViewId = R.id.selection_interaction_recyclerview, + position = 2, + targetViewId = R.id.item_selection_contents_text_view + ) + ).check(matches(withContentDescription("Green"))) + + onView( + atPositionOnView( + recyclerViewId = R.id.selection_interaction_recyclerview, + position = 3, + targetViewId = R.id.item_selection_contents_text_view + ) + ).check(matches(withContentDescription("Blue"))) + } + } + + @Test + fun testStateFragment_contentDescription_multipleChoiceInteraction() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(RATIOS_EXPLORATION_ID_0, shouldSavePartialProgress = false).use { + startPlayingExploration() + + playThroughRatioExplorationState1() + playThroughRatioExplorationState2() + playThroughRatioExplorationState3() + playThroughRatioExplorationState4() + playThroughRatioExplorationState5() + playThroughRatioExplorationState6() + playThroughRatioExplorationState7() + playThroughRatioExplorationState8() + playThroughRatioExplorationState9() + playThroughRatioExplorationState10() + playThroughRatioExplorationState11() + playThroughRatioExplorationState12() + playThroughRatioExplorationState13() + playThroughRatioExplorationState14() + playThroughRatioExplorationState15() + + val expectedDescription = "Image illustrating On the left, there is 1-unit wide glass " + + "filled with apple puree. On the right, there is 2-unit wide glass filled with milk." + + onView( + atPositionOnView( + recyclerViewId = R.id.selection_interaction_recyclerview, + position = 2, + targetViewId = R.id.multiple_choice_content_text_view + ) + ).check(matches(withContentDescription(expectedDescription))) + } + } + @Test fun testFragment_argumentsAreCorrect() { setUpTestWithLanguageSwitchingFeatureOff() @@ -5345,6 +5409,12 @@ class StateFragmentTest { clickContinueNavigationButton() } + private fun playThroughRatioExplorationState15() { + typeTextInput("2:1") + clickSubmitAnswerButton() + clickContinueNavigationButton() + } + private fun addShadowMediaPlayerException(dataSource: Any, exception: Exception) { val classLoader = StateFragmentTest::class.java.classLoader!! val shadowMediaPlayerClass = classLoader.loadClass("org.robolectric.shadows.ShadowMediaPlayer") diff --git a/utility/src/main/java/org/oppia/android/util/parser/html/CustomHtmlContentHandler.kt b/utility/src/main/java/org/oppia/android/util/parser/html/CustomHtmlContentHandler.kt index 25cc7bf29d3..0e9988be8b4 100644 --- a/utility/src/main/java/org/oppia/android/util/parser/html/CustomHtmlContentHandler.kt +++ b/utility/src/main/java/org/oppia/android/util/parser/html/CustomHtmlContentHandler.kt @@ -28,12 +28,14 @@ class CustomHtmlContentHandler private constructor( private val contentDescriptionBuilder = StringBuilder() private val tagContentDescriptions = mutableMapOf() private var isInListItem = false - private var pendingNewline = false private val blockTags = setOf("p", "ol", "ul", "li", "oppia-ul", "oppia-ol", "oppia-li", "div") + // Indicates if a newline should be added before the next text content for block elements. + private var pendingNewline = false override fun endElement(uri: String?, localName: String?, qName: String?) { originalContentHandler?.endElement(uri, localName, qName) - if (localName in blockTags) { + val tagName = qName ?: localName + if (tagName in blockTags) { isInListItem = false } currentTrackedTag = null @@ -57,7 +59,7 @@ class CustomHtmlContentHandler private constructor( contentDescriptionBuilder.append('\n') pendingNewline = false } - ch?.let { contentDescriptionBuilder.append(String(it, start, length)) } + ch?.let { contentDescriptionBuilder.appendRange(it, start, start + length) } } override fun endDocument() { @@ -69,10 +71,17 @@ class CustomHtmlContentHandler private constructor( // Defer custom tag management to the tag handler so that Android's element parsing takes // precedence. currentTrackedTag = TrackedTag(checkNotNull(localName), checkNotNull(atts)) - if (localName in blockTags) { + val tagName = qName ?: localName + if (tagName in blockTags) { pendingNewline = true isInListItem = true } + if (tagName == "a") { + val href = atts.getValue("href") + if (href != null) { + tagContentDescriptions[contentDescriptionBuilder.length] = "$href " + } + } originalContentHandler?.startElement(uri, localName, qName, atts) } @@ -148,6 +157,7 @@ class CustomHtmlContentHandler private constructor( val attributes: Attributes, val openTagIndex: Int ) + /** * Returns the complete content description for the processed HTML, including descriptions * from all custom tags. @@ -156,11 +166,16 @@ class CustomHtmlContentHandler private constructor( val rawDesc = buildString { var lastIndex = 0 tagContentDescriptions.entries.sortedBy { it.key }.forEach { (index, description) -> - if (index > lastIndex) { - append(contentDescriptionBuilder.substring(lastIndex, index)) + if (index > lastIndex && index <= contentDescriptionBuilder.length) { + append( + contentDescriptionBuilder.substring( + lastIndex, + minOf(index, contentDescriptionBuilder.length) + ) + ) } append(description) - lastIndex = index + lastIndex = minOf(index, contentDescriptionBuilder.length) } if (lastIndex < contentDescriptionBuilder.length) { append(contentDescriptionBuilder.substring(lastIndex)) @@ -168,6 +183,7 @@ class CustomHtmlContentHandler private constructor( } return rawDesc.replace(Regex("\n+"), "\n").trim() } + /** Handler interface for a custom tag and its attributes. */ interface CustomTagHandler { /** @@ -259,6 +275,9 @@ class CustomHtmlContentHandler private constructor( customTagHandlers: Map ): String where T : Html.ImageGetter, T : ImageRetriever { val handler = CustomHtmlContentHandler(customTagHandlers, imageRetriever) + + // Triggers the HTML parsing process, allowing CustomHtmlContentHandler to + // intercept and populate the contentDescriptionBuilder. HtmlCompat.fromHtml( "$html", HtmlCompat.FROM_HTML_MODE_LEGACY, @@ -267,6 +286,7 @@ class CustomHtmlContentHandler private constructor( ) return handler.getContentDescription() } + /** * Returns a new [Spannable] with HTML parsed from [html] using the specified [imageRetriever] * for handling image retrieval, and map of tags to [CustomTagHandler]s for handling custom diff --git a/utility/src/main/java/org/oppia/android/util/parser/html/LiTagHandler.kt b/utility/src/main/java/org/oppia/android/util/parser/html/LiTagHandler.kt index 28321115984..239979e9878 100644 --- a/utility/src/main/java/org/oppia/android/util/parser/html/LiTagHandler.kt +++ b/utility/src/main/java/org/oppia/android/util/parser/html/LiTagHandler.kt @@ -9,7 +9,6 @@ import android.text.Spanned import android.text.style.ImageSpan import androidx.core.view.ViewCompat import org.oppia.android.util.locale.OppiaLocale -import org.xml.sax.Attributes import java.util.Stack /** The custom
  • tag corresponding to [LiTagHandler]. */ @@ -28,7 +27,7 @@ const val CUSTOM_LIST_OL_TAG = "oppia-ol" class LiTagHandler( private val context: Context, private val displayLocale: OppiaLocale.DisplayLocale -) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider { +) : CustomHtmlContentHandler.CustomTagHandler { private val pendingLists = Stack>() private val latestPendingList: ListTag<*, *>? get() = pendingLists.lastOrNull() @@ -366,8 +365,4 @@ class LiTagHandler( private fun > Spannable.addMark(mark: T) = setSpan(mark, length, length, Spanned.SPAN_MARK_MARK) } - - override fun getContentDescription(attributes: Attributes): String { - return "" - } } diff --git a/utility/src/test/java/org/oppia/android/util/parser/html/CustomHtmlContentHandlerTest.kt b/utility/src/test/java/org/oppia/android/util/parser/html/CustomHtmlContentHandlerTest.kt index 8b11dcba7f0..ab491cf396c 100644 --- a/utility/src/test/java/org/oppia/android/util/parser/html/CustomHtmlContentHandlerTest.kt +++ b/utility/src/test/java/org/oppia/android/util/parser/html/CustomHtmlContentHandlerTest.kt @@ -402,6 +402,24 @@ class CustomHtmlContentHandlerTest { ) } + @Test + fun testGetContentDescription_anchorTag_handlesCorrectly() { + val contentDescription = CustomHtmlContentHandler.getContentDescription( + html = + """ +

    Visit Example for more info.

    + Another Link + """.trimIndent(), + imageRetriever = mockImageRetriever, + customTagHandlers = mapOf() + ) + + assertThat(contentDescription).isEqualTo( + "Visit https://example.com Example for more info.\n" + + "https://another.com Another Link" + ) + } + private fun Spannable.getSpansFromWholeString(spanClass: KClass): Array = getSpans(/* start= */ 0, /* end= */ length, spanClass.javaObjectType) diff --git a/utility/src/test/java/org/oppia/android/util/parser/html/LiTagHandlerTest.kt b/utility/src/test/java/org/oppia/android/util/parser/html/LiTagHandlerTest.kt index 13246b7d2d8..3a2f2085326 100644 --- a/utility/src/test/java/org/oppia/android/util/parser/html/LiTagHandlerTest.kt +++ b/utility/src/test/java/org/oppia/android/util/parser/html/LiTagHandlerTest.kt @@ -165,57 +165,6 @@ class LiTagHandlerTest { .hasLength(4) } - @Test - fun testGetContentDescription_handlesNestedOrderedList() { - val displayLocale = createDisplayLocaleImpl(US_ENGLISH_CONTEXT) - val htmlString = "

    You should know the following before going on:

    " + - "The counting numbers (1, 2, 3, 4, 5 ….)" + - "How to tell whether one counting number is bigger or " + - "smaller than another Item 1 Item 2" + - "" - val liTaghandler = LiTagHandler(context, displayLocale) - val contentDescription = - CustomHtmlContentHandler.getContentDescription( - html = htmlString, - imageRetriever = mockImageRetriever, - customTagHandlers = mapOf( - CUSTOM_LIST_LI_TAG to liTaghandler, - CUSTOM_LIST_OL_TAG to liTaghandler - ) - ) - assertThat(contentDescription).isEqualTo( - "You should know the following before going on:\n" + - "The counting numbers (1, 2, 3, 4, 5 ….)\n" + - "How to tell whether one counting number is bigger or smaller than another \n" + - "Item 1 \n" + - "Item 2" - ) - } - - @Test - fun testGetContentDescription_handlesSimpleUnorderedList() { - val displayLocale = createDisplayLocaleImpl(US_ENGLISH_CONTEXT) - val htmlString = "

    You should know the following before going on:
    " + - "The counting numbers (1, 2, 3, 4, 5 ….)" + - "How to tell whether one counting number is bigger or " + - "smaller than another

    " - val liTaghandler = LiTagHandler(context, displayLocale) - val contentDescription = - CustomHtmlContentHandler.getContentDescription( - html = htmlString, - imageRetriever = mockImageRetriever, - customTagHandlers = mapOf( - CUSTOM_LIST_LI_TAG to liTaghandler, - CUSTOM_LIST_OL_TAG to liTaghandler - ) - ) - assertThat(contentDescription).isEqualTo( - "You should know the following before going on:\n" + - "The counting numbers (1, 2, 3, 4, 5 ….)\n" + - "How to tell whether one counting number is bigger or smaller than another" - ) - } - private fun createDisplayLocaleImpl(context: OppiaLocaleContext): DisplayLocaleImpl { val formattingLocale = androidLocaleFactory.createOneOffAndroidLocale(context) return DisplayLocaleImpl(context, formattingLocale, machineLocale, formatterFactory) From d7eaad8ef8ddff8f7aa7c88a7c33212bca927c3d Mon Sep 17 00:00:00 2001 From: Manas <119405883+manas-yu@users.noreply.github.com> Date: Tue, 18 Feb 2025 07:06:45 +0530 Subject: [PATCH 6/6] Fix #3708: Content Description Generation For ImageRegionSelectionInteraction (#5691) ## Explanation 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 - [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)). --- .../ImageRegionSelectionInteractionView.kt | 12 ++-- .../app/utility/ClickableAreasImage.kt | 64 ++++++++++++++++--- app/src/main/res/values/strings.xml | 2 + ...ImageRegionSelectionInteractionViewTest.kt | 27 ++++++-- 4 files changed, 85 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/ImageRegionSelectionInteractionView.kt b/app/src/main/java/org/oppia/android/app/player/state/ImageRegionSelectionInteractionView.kt index e7f2dbe3e2e..a113976602f 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/ImageRegionSelectionInteractionView.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/ImageRegionSelectionInteractionView.kt @@ -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 @@ -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 @@ -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) { @@ -118,7 +120,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor( ::entityId.isInitialized && ::imageUrl.isInitialized && ::onRegionClicked.isInitialized && - ::overlayView.isInitialized + ::overlayView.isInitialized && + ::resourceHandler.isInitialized ) { loadImage() @@ -129,7 +132,8 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor( bindingInterface, isAccessibilityEnabled = accessibilityService.isScreenReaderEnabled(), clickableAreas, - userAnswerState + userAnswerState, + resourceHandler ) areasImage.addRegionViews() performAttachment(areasImage) diff --git a/app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt b/app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt index fa77fa271de..dd57c27f29a 100644 --- a/app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt +++ b/app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt @@ -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. */ @@ -23,7 +24,8 @@ class ClickableAreasImage( bindingInterface: ViewBindingShim, private val isAccessibilityEnabled: Boolean, private val clickableAreas: List, - userAnswerState: UserAnswerState + userAnswerState: UserAnswerState, + private val resourceHandler: AppLanguageResourceHandler ) { private var imageLabel: String? = null private val defaultRegionView by lazy { bindingInterface.getDefaultRegion(parentView) } @@ -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 + ) + } + } } } } @@ -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) { @@ -123,7 +140,7 @@ class ClickableAreasImage( showOrHideRegion(newView, clickableArea) } } - newView.contentDescription = clickableArea.contentDescription + parentView.addView(newView) } @@ -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() { @@ -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) + } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4d6299de34d..4eb884b7ab1 100755 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -191,6 +191,8 @@ Correct! Topic: %s %1$s %2$s! + Image showing %1$s. + Select %1$s image. 1 Chapter %s Chapters diff --git a/app/src/sharedTest/java/org/oppia/android/app/testing/ImageRegionSelectionInteractionViewTest.kt b/app/src/sharedTest/java/org/oppia/android/app/testing/ImageRegionSelectionInteractionViewTest.kt index 6b558212760..9690ec629d4 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/testing/ImageRegionSelectionInteractionViewTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/testing/ImageRegionSelectionInteractionViewTest.kt @@ -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 @@ -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." ) ) } @@ -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) @@ -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." ) ) } @@ -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." ) ) } @@ -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." ) ) } @@ -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." ) ) }