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

[BUG]: The story/topic completion text does not update after the app language is changed. #5024

Open
seanlip opened this issue Jun 7, 2023 · 19 comments
Assignees
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@seanlip
Copy link
Member

seanlip commented Jun 7, 2023

Describe the bug
After selecting a language and navigating back to the options menu, the sidenav still shows the stories/topic completion in the old language.

To Reproduce
Steps to reproduce the behavior:

  1. Go to the Options menu, e.g. from the sidenav.
  2. Click on "App Language".
  3. Click on the Arabic option.
  4. Return to the Options menu.
  5. Click on the hamburger dropdown to open the sidenav. Notice that everything is in Arabic except for the "0 Stories Completed | 3 Topics in Progress" text.
  6. Return to the homepage. The "0 Stories Completed | 3 Topics in Progress" text is fixed and now appears in Arabic.
  7. Click back into Options > "App Language" and select English.
  8. Return to the Options menu.
  9. Click on the hamburger dropdown to open the sidenav. Notice that the "0 Stories Completed | 3 Topics in Progress" text now appears in Arabic, whereas everything else appears in English.

Expected behavior

  • In Step 5, the "0 Stories Completed | 3 Topics in Progress" text should also be translated to Arabic.
  • In Step 9, the "0 Stories Completed | 3 Topics in Progress" text should remain in English.

Demonstration
Screenshot_20230607-205707
Screenshot_20230607-205645

Environment

  • Device/emulator being used: Moto device
  • Android or SDK version (e.g. Android 5 or SDK 21): Android 8
  • App version (you can get this through system app settings or via the admin controls menu in-app): 0.11-beta-8c81c98d8b
@seanlip seanlip added the bug End user-perceivable behaviors which are not desirable. label Jun 7, 2023
@seanlip seanlip added Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Jun 7, 2023
@BenHenning
Copy link
Member

I think this also affects dialogs from what I've seen back when implementing the internationalization work. I think the issue splits into two different problem areas:

  • Dialogs
  • The navigation drawer (which I believe is not a dialog)

Dialogs have a separate context from the activity that they're hosted in, so I'm not quite sure how we ensure that they receive the correctly initialized context upon a recreation. Perhaps we need some custom work in an InjectableDialogFragment base class?

The navigation drawer one is a bit more confusing to me because it should just be using the same context as the host activity (which would be recreated upon a language change) when selecting language. Resource selection is quite complicated in Android, however, so it's possible a different resources object is being used for these strings in particular (maybe the application context's resources?).

@adhiamboperes adhiamboperes added the good first issue This item is good for new contributors to make their pull request. label Oct 28, 2023
@aryanmishra29
Copy link

Hi @adhiamboperes, I am unable to reproduce this BUG because the App Language Option Leads to an empty activity.
Video Description
Oppia_AppLangBUG.webm

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes, I am unable to reproduce this BUG because the App Language Option Leads to an empty activity. Video Description Oppia_AppLangBUG.webm

This is due to #5106. If you're unable to run bazel, could you try to repro a different issue?

@aryanmishra29
Copy link

Sure @adhiamboperes.

@adhiamboperes
Copy link
Collaborator

@whyash8, could you PTAL at this issue?

@whyash8
Copy link
Contributor

whyash8 commented Dec 3, 2024

Ok I will take a look.

@whyash8
Copy link
Contributor

whyash8 commented Dec 3, 2024

please assign me this issue

@adhiamboperes
Copy link
Collaborator

ERROR: /home/yash-sharma/.cache/bazel/_bazel_yash-sharma/be3345fe8849b7df3b0505581d7f3bab/external/maven/BUILD:3492:11: AarResourcesExtractor external/maven/_aar/unzipped/resources/com_google_android_material_material failed: (Exit 127): aar_resources_extractor failed: error executing command bazel-out/host/bin/external/bazel_tools/tools/android/aar_resources_extractor --input_aar ... (remaining 9 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox aar_resources_extractor failed: error executing command bazel-out/host/bin/external/bazel_tools/tools/android/aar_resources_extractor --input_aar ... (remaining 9 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox /usr/bin/env: 'python': No such file or directory /*getting this error while building bazel for this application version 0.11-beta-8c81c98d8b// please help to solve @adhiamboperes

@whyash8, please move this to discussions as it is not related to this issue.

@whyash8
Copy link
Contributor

whyash8 commented Dec 20, 2024

Hi @adhiamboperes,

Can you please provide guidance or suggestions on how to start solving this issue? Specifically, any pointers on which files or parts of the codebase I should focus on would be greatly appreciated. I have already setup the appVersion needed for this issue .

Thank you!

@adhiamboperes
Copy link
Collaborator

@whyash8, NavigationDrawerHeaderViewModel is the class responsible for populating this text. profileTopicProgressText and profileStoryProgressText are the affected variables. Some possible causes for the language not updating for the specified strings when language changes are:

  • Wrong initialization order: profileTopicProgressText and profileStoryProgressText depend on ongoingTopicCount and completedStoryCount, which are initialized after the ObservableFields. This may cause incorrect initial values for the ObservableFields.
  • Lifecycle timing and binding-related synchronization issues between the ViewModel and the UI.
  • The headerBinding is bound to the headerViewModel, but executePendingBindings is not explicitly called for headerBinding. If the binding doesn't receive updates immediately, the ObservableFields in the headerViewModel might update, but the UI won't reflect these changes until the next layout pass.
  • Adding the header view (addHeaderView) is done before the LiveData has fully emitted its results. If the LiveData observers fire late, the header view won't automatically refresh.

These are options you can eliminate and can guide you to understand why the update is failing.

@whyash8
Copy link
Contributor

whyash8 commented Dec 23, 2024

Hi @adhiamboperes ,

I am currently working on issue #5024 and noticed a difference between the release-0.11 and develop branches in the NavigationDrawerHeaderViewModel.kt file. Specifically, the release-0.11 branch has a single variable profileProgressText, while the develop branch has separated this into two variables: profileTopicProgressText and profileStoryProgressText.

I believe that aligning the release-0.11 implementation with the develop branch by introducing these two separate variables could potentially resolve the issue and improve the clarity and accuracy of the progress texts.

Before proceeding, I wanted to get your input on whether this would be the best approach or if there are any other considerations I should be aware of.

Thanks for your guidance!

@adhiamboperes
Copy link
Collaborator

@whyash8, could you please confirm that you are suggesting reverting to a single variable? Was this bug not present in 0.11?

@whyash8
Copy link
Contributor

whyash8 commented Dec 24, 2024

@adhiamboperes
The bug is not present in the develop branch and it exists in app version : 8c81c98 and i am suggesting to align the implementation of navigationdrawerheaderviewmodel of app version 8c81c98 with develop branch to resolve the issue in app version 8c81c98.

@adhiamboperes
Copy link
Collaborator

@whyash8, if the bug is not present in the current version of develop, but it was present in an earlier version, it means that it was fixed.

In such situations, we prefer to identify the commit that fixed the issue, and then we can verify that the bug was present before, but not after that commit. In order to do this, we use git bisect. Since you have identified a “bad” commit(8c81c98) where the bug existed, and taking the latest commit on develop as good commit, you can use git bisect until you find the first “good” commit where the bug was fixed.

Once you are able to locate it, we should be able to close this issue.

Here’s a link to git bisect documentation: https://git-scm.com/docs/git-bisect

@swarnalicoder
Copy link

Hi @whyash8,
I’m a beginner in Android development and would love to help with resolving this issue. I’ve been learning about navigation drawers and language handling in Android, and I’m excited to contribute. Can you guide me on which files or sections of the code I should focus on?

@whyash8
Copy link
Contributor

whyash8 commented Dec 25, 2024

@swarnalicoder please look at a different issue . This is already assigned

@whyash8
Copy link
Contributor

whyash8 commented Dec 26, 2024

Hi @adhiamboperes ,

After performing several iterations of git bisect as advised, I have identified that commit 3a369ed (Fixes part of #4120, part of #1051: Fix a lot of build-time warnings (#5402)) resolves the issue #5024.

Thank you for the guidance.

Best regards,
@whyash8

@whyash8
Copy link
Contributor

whyash8 commented Dec 29, 2024

Screenshot_20241229-130735_Oppia.jpg

Commit 3c7713d

@whyash8
Copy link
Contributor

whyash8 commented Dec 29, 2024

Screenshot_20241229-130319_Oppia.jpg

Commit 3a369ed which resolved the issue.
PTAL @adhiamboperes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Development

No branches or pull requests

6 participants