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]: Fix all Android lint failures. #5169

Open
7 of 25 tasks
seanlip opened this issue Oct 1, 2023 · 32 comments
Open
7 of 25 tasks

[BUG]: Fix all Android lint failures. #5169

seanlip opened this issue Oct 1, 2023 · 32 comments
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: Low Solution is clear and broken into good-first-issue-sized chunks.

Comments

@seanlip
Copy link
Member

seanlip commented Oct 1, 2023

Describe the bug

We would like to fix all the remaining lint issues on Oppia Android. There are currently 195 errors and 1009 warnings. Also, 38 checks are currently disabled.

We'd like to get rid of all these errors and then enable all the lint checks fully on CI (see #1742).

Steps To Reproduce

Run the following command from a terminal in a checked-out Oppia Android repo:

./gradlew :app:lint

Expected Behavior

Running the command above should not show any errors.

Screenshots/Videos

This is the first part of the error report:

Screenshot from 2023-10-02 00-44-03

Project Organization

This project can be divided into multiple parts. Please specify which part you want to take up and show a screenshot of a lint report with no errors for the corresponding issues in order to claim a part (and make a PR for it). Also, explain your general approach to fixing the errors for the part you claimed (don't just suppress the errors).

The relevant parts are listed below:

@seanlip seanlip added good first issue This item is good for new contributors to make their pull request. bug End user-perceivable behaviors which are not desirable. labels Oct 1, 2023
@adhiamboperes adhiamboperes added Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks. labels Oct 1, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Oct 2, 2023

  • All warnings in "Accessibility" category

Hi @adhiamboperes, I would like work on this part. Under this category, the warnings are mainly of 3 types.

  1. Warning: Image without contentDescription.
    Fix: Add a contentDescription to each of those ImageView.

  2. Warning: 'onTouch' lambda should call 'View#performClick' when a click is detected
    Fix: Call customView.performClick( ).

  3. Warning: 'clickable' attribute found, please also add 'focusable'.
    Fix: Set android:focusable="false" as it is a button.

@adhiamboperes adhiamboperes removed the Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. label Oct 2, 2023
@adhiamboperes adhiamboperes added this to the 1.0 Global availability milestone Oct 2, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Oct 2, 2023

  • MissingDefaultResource + AppCompatCustomView + FragmentTagUsage

Hi @adhiamboperes, I would like to work on this next.

  1. MissingDefaultResource mainly has a single issue with a string which has different names in english and other translations. It can be easily fixed, by matching the name of that string in english and other translations.
  2. For AppCompatCustomView, it can be fixed by replacing the Normal views with AppCompat views
  3. <fragment/> tags can be replaced with <androidx.fragment.app.FragmentContainerView/>.

Rd4dev added a commit to Rd4dev/oppia-android that referenced this issue Oct 9, 2023
@Rd4dev
Copy link
Collaborator

Rd4dev commented Oct 9, 2023

Hi @adhiamboperes,

I am a new contributor to oppia-android and would like to start my contributions with the project. I've signed the CLA, filled out the survey form, and set up the environment locally. After some initial exploration, I believe I can assist with some lint issues.

I'd like to begin with the task related to All warnings in "Internationalization" While working on it locally, I encountered two types of issues:

  • hardcoded text
  • padding/margins.

Fix:

  • I've resolved the hardcoded text issue by assigning string resource values and
  • Addressed the padding issues by setting padding end values.

I have the code changes in my forked repository [Link] for your review and have attached before-and-after screenshots.

Screenshots:
Lint Issues Internationalization Before After

If this solution aligns with the project's requirements, I'd appreciate it if you could assign the task to me so I can proceed with making a pull request.

Thank you!

Best regards,
RD [Rama Devi]

@sichchurov
Copy link
Collaborator

InvalidPackage + NewApi + FragmentLiveDataObserve + RestrictedApi

Hi @BenHenning, I would like to work in this part. There is a compile-time error on line 231 in class ControlButtonsViewModel.

Call requires API level 26 (current min is 19): java.util.Base64#getEncoder

I can solve this error by:

  1. using if-else operator. If API 26 and higher use java.util.Base64 class else use android.util.Base64 class

     if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
         Base64.getEncoder().encodeToString(compressedMessage)
     } else {
         android.util.Base64.encodeToString(compressedMessage, 0)
     }
    
  2. android.util.Base64 has encodeToString as equivalent of getEncoder().encode(args)
    it both Passing DEFAULT results in output that adheres to RFC 2045. So I can try to return right away:
    return android.util.Base64.encodeToString(compressedMessage, 0)

adhiamboperes added a commit that referenced this issue Oct 14, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fix #5182 [part of #5169]

Fixes All warnings in "Internationalization" caused by
- hardcoded text : Fixed by assigning `string resource` values
- padding/margins : Fixed by setting `padding end` values

## ScreenShot:
![Lint Issues Internationalization Before
After](https://github.com/oppia/oppia-android/assets/122200035/a2f04893-b670-499b-980b-5151097b47c0)

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
Delete these section if this PR does not include UI-related changes.
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
-->
<br>

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
@Rd4dev
Copy link
Collaborator

Rd4dev commented Oct 15, 2023

Hi @adhiamboperes,

I would like to continue to work on - All warnings in "Usability:Typography", "Usability:Icons", and "Usability" categories task.

The issues can be categorized as:

  1. Borderless Buttons
  2. OK text capitalization
  3. Ellipsis/Hyphen unicode replacement
  4. Image in density drawable folder
  5. Text size too small
  6. To incorporate Autofill

Fix:

  1. Borderless Buttons: Added style="?android:attr/buttonBarButtonStyle" to implement borderless buttons as suggested.
  2. OK Text Capitalization: Capitalized the text in string resources, changing "ok" to "OK" for better consistency.
  3. Ellipsis/Hyphen Unicode Replacement: Used Unicode values to replace ... and - instances in Arabic translations, and added tools:ignore="TypographyDashes" where Unicode couldn't be applied.
  4. Image in Density Drawable Folder: Created and reallocated density images to appropriate folders such as
  • drawable-hdpi
  • drawable-mdpi
  • drawable-xhdpi
  • drawable-xxhdpi
  • drawable-xxxhdpi.
  1. Text Size Too Small: Increased text size to 12sp, as recommended, from the previous size of 10sp.
  2. Incorporating Autofill: Added autofill hint text to the Feedback Edit Text.

I have the code changes in my forked repository [Link1][Link 2] for your review and have attached before-and-after screenshots.

Screenshots
Before After Usability Lint Issues
If this solution aligns with the project's requirements, I'd appreciate it if you could assign the task to me so I can proceed with making a pull request.

Thank you!

Best regards,
RD [Rama Devi]

@sichchurov
Copy link
Collaborator

Hi @adhiamboperes,
I've found a solution of this issue: RestrictedApi

class AppCompatCheckBoxBindingAdapters has an error:

AppCompatCheckBox.setSupportButtonTintList can only be called from within the same library group prefix (referenced groupId=androidx.appcompat with prefix androidx from groupId=oppia-android)

The reason is using a private API. To resolve it, we can use:

CompoundButtonCompat.setButtonTintList(@NonNull CompoundButton button, @Nullable ColorStateList tint)

Here is a screen of lint report
Before

restricted-api-before.png

After

restricted-api-after.png

@Rd4dev
Copy link
Collaborator

Rd4dev commented Oct 23, 2023

Hi @adhiamboperes,

I would like to continue along side on the - InconsistentLayout lint warnings

Cause: Some of the view items defined in the 'layout-sw600dp' configuration were not found in the main layout resource files.
Lint warning: The id "id_name" in layout "resource_file" is missing from the following layout configurations:
                        layout (present in layout-sw600dp)

Fix: Included the absent components in the primary resource layout file.
I have the code changes in my forked repository Link for your review and have attached before-and-after screenshots.

Screenshots
Inconsistent layouts oppia-android lint warning before after

If this solution aligns with the project's requirements, I'd appreciate it if you could assign the task to me so I can proceed with making a pull request.

Thank you!

Best regards,
RD [Rama Devi]

adhiamboperes added a commit that referenced this issue Nov 6, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fix #5204 - [Part of #5169]

To address the lint warnings, two potential solutions were considered. 
1. Initially, there was an option to include the missing IDs, ensuring
consistency across different layout configurations
[[**Link**](Rd4dev@f8977e6)].
However, it was observed that implementing this solution led to
functional disruptions within the current working application, as
confirmed through comprehensive testing procedures (refer to the
attached assets for a detailed overview).



https://github.com/oppia/oppia-android/assets/122200035/82172eaf-a555-4e76-b1ae-8f0db7ddb924



2. The second option was to suppress the lint warnings for the specific
views in the layout-sw600dp configuration files
[[**Link**](Rd4dev@a0f5b1e)]
to maintain the seamless functionality of the application . This was
achieved by incorporating the attribute
**`tools:ignore="InconsistentLayout"`** for relevant views, without
impacting the current operational stability of the application.




https://github.com/oppia/oppia-android/assets/122200035/93a65422-3526-4d76-873f-c08d24abc3ee



Taking the necessary action, I proceeded to implement the suppression of
lint warnings by submitting a pull request.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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".
- [ ] 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
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
@deonwaju
Copy link
Collaborator

deonwaju commented Nov 15, 2023

Before
Screenshot 2023-11-17 at 11 06 19
Updated @adhiamboperes

After
Screenshot 2023-11-17 at 11 28 18

Hi @adhiamboperes, I would like work on this tasks

  1. Warning: VectorDrawableCompat
    Fix: A. Increase minimum API level to 21
    or
    open the app's build.gradle file.
    Inside the android block, add or update the vectorDrawables section under defaultConfig:

android { ... defaultConfig { ... vectorDrawables { useSupportLibrary true } } }

Note: This fix also reduced the VectorRaster: from 62 to 18 warnings

  1. Warning: Addressing Typos from [Typos + StringFormatCount + UnusedQuantit + AllowBackup]
    Fix: Change "Ok" to both letters as caps "OK".
    Fix 2: corrected ratio_error_invalid_colons that had a repeated word "dois" in string resource file pt-rBR/strings.xml

  2. Warning: Addressing UnknownIdInLayout warning on the 13th issue on the list [DefaultLocale + OldTargetApi + UnknownIdInLayout + GradleDependency]
    Fix: Delete the reference that doesn't exist on line 88

Screenshot 2023-11-17 at 10 56 30 Updated @adhiamboperes
  1. Warning: Addressing DisableBaselineAlignment warning on the 13th issue on the list [DisableBaselineAlignment + InefficientWeight + Overdraw + UseCompoundDrawables + UselessLeaf + UselessParent]

Fix: (A) Add android:baselineAligned="false" to LinearLayout on line 12 in "
layout-sw600dp-land/topic_info_fragment.xml"
(B) Add android:baselineAligned="false" to LinearLayout on line 91 in "
"layout/topic_lessons_story_summary.xml"
(C) Add android:baselineAligned="false" to LinearLayout on line 93 in "
"layout-sw600dp-land/topic_lessons_story_summary.xml"
Example below:
Before
Screenshot 2023-11-26 at 10 58 37

After

Screenshot 2023-11-26 at 10 58 53
  1. Warning: Addressing InefficientWeight warning on the 13th issue on the list [DisableBaselineAlignment + InefficientWeight + Overdraw + UseCompoundDrawables + UselessLeaf + UselessParent]
    Fix: adding width 0dp to Textview on line 31
Screenshot 2023-11-17 at 11 13 10

Please assign me these tasks if this solution meets the project requirements so I can move on with submitting a pull request.

Kind regards.

@adhiamboperes Please address from no. 2 to 5.
Before

Screenshot 2023-11-26 at 12 04 52

After
Screenshot 2023-11-26 at 12 11 26

@adhiamboperes
Copy link
Collaborator

@deonwaju, you can work on this once your current issue is resolved and merged.

@deonwaju
Copy link
Collaborator

@adhiamboperes Ok, Thanks, I will as soon as it is.

@deonwaju
Copy link
Collaborator

Hi @adhiamboperes, Can I start work on this pending the time my request is resolved and merged?

@deonwaju
Copy link
Collaborator

Hello @adhiamboperes, I recently updated my solutions, Please help go through them again So I can create a PR.

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Nov 17, 2023

Hello @adhiamboperes, I recently updated my solutions, Please help go through them again So I can create a PR.

@deonwaju, I'm confused because a part of the updated comment seems to be addressing somewhat different warnings than the original comment.

For vector drawable and implied quantity sections, I already created separate issues and mentioned you there. Any further discussions for those specific scenarios will best discussed in the respective issue threads. Also reference those issues when creating a PR.

For additional segments of this main issue that you want to work on, please clarify which item from the list it belongs to so I can generate a separate issue for it. For example, the 'Typos' that you mention in your previous comment, isn't an item on the list, so I'm unsure of which item you are referring to.

@deonwaju
Copy link
Collaborator

Hello @adhiamboperes, I recently updated my solutions, Please help go through them again So I can create a PR.

@deonwaju, I'm confused because a part of the updated comment seems to be addressing somewhat different warnings than the original comment.

For vector drawable and implied quantity sections, I already created separate issues and mentioned you there. Any further discussions for those specific scenarios will best discussed in the respective issue threads. Also reference those issues when creating a PR.

For additional segments of this main issue that you want to work on, please clarify which item from the list it belongs to so I can generate a separate issue for it. For example, the 'Typos' that you mention in your previous comment, isn't an item on the list, so I'm unsure of which item you are referring to.

Ok, sorry about the confusion, still getting the hang of this. I will write my fixes on those issues you created, and also for "Typos" I mentioned, I saw it on the list along with " Typos + StringFormatCount + UnusedQuantit + AllowBackup"
the 11th item on the list in the description of this PR.

Regards

@deonwaju
Copy link
Collaborator

deonwaju commented Nov 26, 2023

Hello @adhiamboperes, please help review my solution from 2 to 5 so i can work on them if its fine by standards.

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 3, 2023

Hello @adhiamboperes

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Dec 4, 2023

@deonwaju, might I suggest that you split each of your numbered sections into a seperate comment in the future?
Re:

Warning: Addressing Typos from [Typos + StringFormatCount + UnusedQuantit + AllowBackup]
Fix: Change "Ok" to both letters as caps "OK".
Fix 2: corrected ratio_error_invalid_colons that had a repeated word "dois" in string resource file pt-rBR/strings.xml

There is a fix for ok button capitalization in #5196.

It would be great to provide a solutiion for the entire [Typos + StringFormatCount + UnusedQuantit + AllowBackup] Item so that I can convert it to one issue.

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 4, 2023

@deonwaju, might I suggest that you split each of your numbered sections into a seperate comment in the future? Re:

Sure, thanks, I am learning everyday. I will split them as soon as i am done with the other issue you assigned to me.

adhiamboperes added a commit that referenced this issue Dec 5, 2023
…gUsage (#5191)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes #5186 [part of #5169]

1. `MissingDefaultResource` has been fixed already by
#5172. It had issues with
these plurals:
-
[minutes_ago](https://github.com/oppia/oppia-android/blob/e060e2ff265ce35c92f1b969f6d7ad002d1a62ad/app/src/main/res/values/strings.xml#L456)
-
[hours_ago](https://github.com/oppia/oppia-android/blob/e060e2ff265ce35c92f1b969f6d7ad002d1a62ad/app/src/main/res/values/strings.xml#L461C18-L461C27)
-
[days_ago](https://github.com/oppia/oppia-android/blob/e060e2ff265ce35c92f1b969f6d7ad002d1a62ad/app/src/main/res/values/strings.xml#L465C18-L465C26)

3. For `AppCompatCustomView`, it is fixed by replacing the Normal views
with AppCompat views

4. `<fragment/>` tags are replaced with
`<androidx.fragment.app.FragmentContainerView/>`, with some changes in
the NavigationDrawerFragment to ensure that binding has been initialized
before it is accessed.

## Screen Recording

https://github.com/oppia/oppia-android/assets/84731134/3a340cff-70a4-4e77-a1a3-e5b7a8c71a6e



## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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)).
<br>

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
@deonwaju
Copy link
Collaborator

deonwaju commented Dec 5, 2023

  1. Fix part of [Typos + StringFormatCount + UnusedQuantit + AllowBackup]
    Fix for Typos: Change "Ok" to both letters as caps "OK" in the string resources affected and also correct ratio_error_invalid_colons that had a repeated word "dois" in string resource file pt-rBR/strings.xml

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 5, 2023

  1. Fix part of [DefaultLocale + OldTargetApi + UnknownIdInLayout + GradleDependency]
    Fix for UnknownIdInLayout: Delete the reference that doesn't exist in solution_summary.xml on line 88

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 5, 2023

  1. Fix part of [DisableBaselineAlignment + InefficientWeight + Overdraw + UseCompoundDrawables + UselessLeaf + UselessParent]
    Fix for DisableBaselineAlignment: (A) Add android:baselineAligned="false" to LinearLayout on line 12 in "layout-sw600dp-land/topic_info_fragment.xml"
    (B) Add android:baselineAligned="false" to LinearLayout on line 91 in "
    "layout/topic_lessons_story_summary.xml"
    (C) Add android:baselineAligned="false" to LinearLayout on line 93 in "
    "layout-sw600dp-land/topic_lessons_story_summary.xml"
    Example below:
    Warning: Addressing DisableBaselineAlignment warning on the 13th issue on the list [DisableBaselineAlignment + InefficientWeight + Overdraw + UseCompoundDrawables + UselessLeaf + UselessParent]

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 5, 2023

  1. Fix part of [DisableBaselineAlignment + InefficientWeight + Overdraw + UseCompoundDrawables + UselessLeaf + UselessParent]
    Fix for InefficientWeight: adding width 0dp to Textview on line 31

@deonwaju
Copy link
Collaborator

deonwaju commented Dec 5, 2023

@adhiamboperes

@aryanmishra29
Copy link

Hi @adhiamboperes, I am unable to repro the issue. When I run the specified command in the terminal in the checked-out repo of the project it shows some different errors but none specified in the issue itself. I am still fairly new to oppia-android's codebase, hence kindly help me repro this issue.

@theMr17
Copy link
Collaborator

theMr17 commented Dec 8, 2023

Hi @aryanmishra29, an easy way to repro the issue is to navigate to Code -> Inspect Code.

image

Then check on Whole Project and click on Analyze.

image

@aryanmishra29
Copy link

Hi @adhiamboperes , there are 145 incomplete translations. I want to work on this issue and my methodology will be to add the strings in string.xml of that respective language by using Google Translate to translate from English to the language specified.

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes , there are 145 incomplete translations. I want to work on this issue and my methodology will be to add the strings in string.xml of that respective language by using Google Translate to translate from English to the language specified.

Hi @aryanmishra29, we have a traslation team currently working on the translations, could you please pick another item here to work on?

@pingu-73
Copy link
Contributor

Hi @adhiamboperes, i would like to work on UnusedAttribute
Warning is happening due to min. sdk version specified in the gradel.build is 19. We can change it to 29 and avoid those warnings.
Before:
Screenshot 2023-12-14 at 1 53 24 PM

After:
Screenshot 2023-12-14 at 2 18 25 PM

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Dec 14, 2023

Hi @adhiamboperes, i would like to work on UnusedAttribute Warning is happening due to min. sdk version specified in the gradel.build is 19. We can change it to 29 and avoid those warnings.

Hi @pingu-73, at the moment, we are unable to update the minSdk version. That removes support for older devices as well as will need a lot of work to verify compatibility with other libraries in the repo(we have a lot!). Could you please suggest another solution? For example, can you identify these unused attributes, and see if we can address each on a case by case basis?

@deonwaju
Copy link
Collaborator

  1. Fix part of [Typos + StringFormatCount + UnusedQuantit + AllowBackup]
    Fix for Typos: Change "Ok" to both letters as caps "OK" in the string resources affected and also correct ratio_error_invalid_colons that had a repeated word "dois" in string resource file pt-rBR/strings.xml

@adhiamboperes

@pingu-73
Copy link
Contributor

Hi @adhiamboperes, i would like to work on UnusedAttribute Warning is happening due to min. sdk version specified in the gradel.build is 19. We can change it to 29 and avoid those warnings.

Hi @pingu-73, at the moment, we are unable to update the minSdk version. That removes support for older devices as well as will need a lot of work to verify compatibility with other libraries in the repo(we have a lot!). Could you please suggest another solution? For example, can you identify these unused attributes, and see if we can address each on a case by case basis?

Hi @adhiamboperes
Another Approach: we can add checks using dataBindings for example:
Screenshot 2023-12-14 at 3 58 00 PM

Screenshot 2023-12-14 at 4 04 31 PM Screenshot 2023-12-14 at 3 55 20 PM

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes, i would like to work on UnusedAttribute Warning is happening due to min. sdk version specified in the gradel.build is 19. We can change it to 29 and avoid those warnings.

Hi @pingu-73, at the moment, we are unable to update the minSdk version. That removes support for older devices as well as will need a lot of work to verify compatibility with other libraries in the repo(we have a lot!). Could you please suggest another solution? For example, can you identify these unused attributes, and see if we can address each on a case by case basis?

Hi @adhiamboperes Another Approach: we can add checks using dataBindings for example:

@pingu-73, this new approach sounds good to me. I have created the issue here: #5273

adhiamboperes added a commit that referenced this issue Jan 11, 2024
Refactor firebase auth wrapper

Refactor dagger bindings for firestore dependencies

Ensure firestore logs show in dev event logs view.

Removed redundant bindings.

Localisation updates from https://translatewiki.net. (#5209)

Translation updates

Fixes #4708: Don't submit answer if it's invalid according to the input (#5205)

Fixes #4708: Don't submit answer if it's invalid according to the input
or else after submitting the recycler view will not restore items on
configuration change. See #4708 for more details

Video demo:

[before](https://drive.google.com/file/d/1bLgo-AYro0UbffR6X8nWo8Xv7oUuNJFa/view?usp=sharing)

[after](https://drive.google.com/file/d/1Oek7j6dgjJmgasyyd9FHtQo4E7zTvEBd/view?usp=sharing)

Continuation of PR #5202 since that one's no longer viable due to being
force pushed in a repo sync through GitHub UI.

- [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
- [ ] 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)).

If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Change unrelated to concerns like dark mode, RTL, accessibility. PR
demonstration of tests passing.

---------

Co-authored-by: Long Wei <longwei@google.com>
Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>

Fix part of #5195: using viewLifecycleOwner (#5207)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
It is not a good idea to use a fragment as a lifecycle owner when
subscribing to liveData objects. It would be correct to use
viewLifecycleOwner.

Lint report before:
<img
src="https://github.com/oppia/oppia-android/assets/71126152/ad482fd4-d3b0-46c7-b7de-febe7a48c07b"
width="70%" height="70%" />

Lint report after
<img
src="https://github.com/oppia/oppia-android/assets/71126152/fe7a26cf-1fcd-4cd5-ae3a-3cb7dcf2659c"
width="70%" height="70%" />

<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Fix #5032 Images in Arabic (RTL) lessons are right-aligned rather than center-aligned. (#5212)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fixes #5032, To make the image center-aligned, we are adding styling to
the HTML content image and removing the left margin by setting
drawableLeft bound to 0 in RTL.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [ ] 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)).

LTR Potrait

![beforeLTRpotrait](https://github.com/oppia/oppia-android/assets/76042077/298fc2c0-7512-40dc-8d56-444fe55f4998)

LTR Landscape

![LTRlandscape](https://github.com/oppia/oppia-android/assets/76042077/94e16ccb-052c-4723-b26e-00fa0591270d)

RTL Potrait

![beforeRTLpotrait](https://github.com/oppia/oppia-android/assets/76042077/59efc1c5-c695-442a-9549-8aa353d61aed)

RTL Landscape

![RTLlandscape](https://github.com/oppia/oppia-android/assets/76042077/dae3c7b8-ce4e-4968-8dea-c4bc23eaf75f)

RTL Potrait

![Potrait_after](https://github.com/oppia/oppia-android/assets/76042077/d5115f63-f71c-44fa-bb60-639e8f8abcef)

RTL Landscape

![landscapeafter](https://github.com/oppia/oppia-android/assets/76042077/9a37d6d1-dc80-489e-827c-8b208fa33d67)

LTR Potrait

![LTRpotraitafter](https://github.com/oppia/oppia-android/assets/76042077/4d51cf7c-68eb-4924-aaaa-64cb44391081)

LTR Landscape

![LTRLandscape](https://github.com/oppia/oppia-android/assets/76042077/802dcd37-5adb-4f5d-8a30-cfa110d76885)

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

fix: Fix Translate Wiki Issues with Splash Activity Screen Strings (#5219)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
When merged, this PR will:
- Remove white spaces and new lines that were added to Splash Activity
Screen strings by the [App/OS Deprecation Milestone 3
PR](#5096). This will allow
TranslateWiki to translate them. See related [comment on the
PR](d471d79#r130759117).

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>

Fix #5214, Fix Part of #1723 : Fix onboarding issue faced by new contributors (#5210)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fix #5214 : Wiki link not redirect to correct destination (
good-first-issue link)
Fix Part of #1723 : Fix onboarding issue faced by new contributors.

1. Tell contributors firmly to use "Android Studio Bumblebee | 2021.1.1
Patch 3."
2. In the wiki, make sure to mention that you need to have JDK 11 to use
oppia-android.
3. Concentrate on using robolectric instead of espresso and provide
clear instructions for setting up tests.

<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [ ] 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)).

Fix #5204 - Inconsistent Layout Lint Warning (#5218)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fix #5204 - [Part of #5169]

To address the lint warnings, two potential solutions were considered.
1. Initially, there was an option to include the missing IDs, ensuring
consistency across different layout configurations
[[**Link**](Rd4dev@f8977e6)].
However, it was observed that implementing this solution led to
functional disruptions within the current working application, as
confirmed through comprehensive testing procedures (refer to the
attached assets for a detailed overview).

https://github.com/oppia/oppia-android/assets/122200035/82172eaf-a555-4e76-b1ae-8f0db7ddb924

2. The second option was to suppress the lint warnings for the specific
views in the layout-sw600dp configuration files
[[**Link**](Rd4dev@a0f5b1e)]
to maintain the seamless functionality of the application . This was
achieved by incorporating the attribute
**`tools:ignore="InconsistentLayout"`** for relevant views, without
impacting the current operational stability of the application.

https://github.com/oppia/oppia-android/assets/122200035/93a65422-3526-4d76-873f-c08d24abc3ee

Taking the necessary action, I proceeded to implement the suppression of
lint warnings by submitting a pull request.

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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".
- [ ] 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)).

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>

Fix #5112: Rendering Inline SVGs (#5208)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fixes #5112

We discovered that `mathImg_` dimesions are in **ex**, and not **px**,
and this unit is not natively rendered on android.

This PR introduces a method that will convert ex to px and use the
resulting dimesions to compute the rendering size of a math image to be
rendered inline. The formula is based on the [androidSvg
library](https://github.com/oppia/androidsvg/blob/5bc9c7553e94c3476e8ea32baea3c77567228fcd/androidsvg/src/main/java/com/caverock/androidsvg/androidrendering/SVGAndroidRenderer.java#L5018)
that is also based on CSS3 specs.

To preserve the size computation of block SVGs, we have created a
seperate method that handles the calculation of rendering dimensions for
inline SVGs.

We have only tested these changes through visual observation, and
confirming that the images scale correctly when reading text size is
changed.

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

|Before|After|
|--|--|

|![IMG_0705](https://github.com/oppia/oppia-android/assets/99066793/62e00542-091c-48b1-9e63-aa1ca3c9abff)|![Screenshot_1698283405](https://github.com/oppia/oppia-android/assets/59600948/e1c534ee-b379-461c-bc39-7854fe57dc70)|

|![Screenshot_1698284125](https://github.com/oppia/oppia-android/assets/59600948/3f47014d-f4e3-4430-9e1e-d4b4609014e7)|![Screenshot_1698283411](https://github.com/oppia/oppia-android/assets/59600948/786445ec-0b75-41af-882b-ae93e2da9b4d)|

|![Screenshot_1697714168](https://github.com/oppia/oppia-android/assets/59600948/9fa8daeb-25ca-4b90-ac66-a6228c81f120)|![Screenshot_1698284519](https://github.com/oppia/oppia-android/assets/59600948/0b39d37d-1471-4ecf-8626-13a01ec35c3e)|

|![Screenshot_1697745323](https://github.com/oppia/oppia-android/assets/59600948/8152c286-8845-4f3b-8ecf-93acd03dbc3e)|![Screenshot_1698284666](https://github.com/oppia/oppia-android/assets/59600948/dbff75ba-bb08-4855-b49a-0e4d337c478b)|

|![Screenshot_1697714820](https://github.com/oppia/oppia-android/assets/59600948/65b7eadc-c8b0-4963-8dbf-d4cd19590e98)|![Screenshot_1698284933](https://github.com/oppia/oppia-android/assets/59600948/9b517dc5-fc75-43f8-b6af-a4e24b1cde28)|

Images scaled to Largest text and display size, and dark mode
||||
|--|--|--|

|![image](https://github.com/oppia/oppia-android/assets/59600948/cedd58b2-a16f-4ed7-9856-df9371242d3a)|![image](https://github.com/oppia/oppia-android/assets/59600948/acf79348-72cb-4732-b91a-1f934861d9f0)|![image](https://github.com/oppia/oppia-android/assets/59600948/fa07209d-fb54-4f21-a267-cc9bd7d26177)|

|![image](https://github.com/oppia/oppia-android/assets/59600948/241f8501-21ce-488f-be6e-a6f2b4bde198)|![image](https://github.com/oppia/oppia-android/assets/59600948/82d0a270-e486-4553-96ae-559ad7b784a2)|![image](https://github.com/oppia/oppia-android/assets/59600948/6080aff6-aff0-46ad-b30c-88e086aeeac7)|

---------

Co-authored-by: Ben Henning <ben@oppia.org>

Fix #3596: Adds Audio Loading UI (#5179)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes #3596

This PR adds an indeterminate progress bar when the audio is loading,
before it starts playing.

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

https://github.com/oppia/oppia-android/assets/84731134/5c6fe393-5d89-4bda-a144-57bdc42a9c57

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>

Fix #5226: EnableContinueButtonAnimation Feature Flag  (#5228)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fix #5226

Remove declarations and usages of the enableContinueButtonAnimation
PlatformParameter.

(A) Removed declaration of
TestPlatformParameterModule.forceEnableContinueButtonAnimation() from
all test functions namely:

testContinueInteractionAnim_openPrototypeExp_checkContinueButtonAnimatesAfter45Seconds(),
testConIntAnim_openProtExp_orientLandscapeAfter30Sec_checkAnimHasNotStarted(),
testConIntAnim_openProtExp_orientLandAfter30Sec_checkAnimStartsIn15SecAfterOrientChange(),
testContNavBtnAnim_openMathExp_checkContNavBtnAnimatesAfter45Seconds(),
testContNavBtnAnim_openMathExp_playThroughSecondState_checkContBtnDoesNotAnimateAfter45Sec(),
testConIntAnim_openFractions_expId1_checkButtonDoesNotAnimate()
in StateFragmentLocalTest.kt file

(B) Removed function declaration
TestPlatformParameterModule.forceEnableContinueButtonAnimation(false)
from setup() function
in StateFragmentTest.kt


(C) Removed function declaration
TestPlatformParameterModule.forceEnableContinueButtonAnimation(false)
from setup() function
in ExploreActivityTest.kt

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

Fix #5230: VectorDrawableCompat error lint (#5237)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fix #5230: VectorDrawableCompat Lint error

Warning: VectorDrawableCompat
Fix: opened the app's build.gradle file.
Inside the android block, added the vectorDrawables section under
defaultConfig with the code below:
android { ... defaultConfig { ... vectorDrawables { useSupportLibrary
true } } }

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

Fix #3345 Images are not fully displayed  (#5234)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fixes #3345, substracting drawableWidth from maxContentItemPadding only
if drawableWidth >= (maxcontentWidth -
 maxContentItemPadding) would solve the issue.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

https://user-images.githubusercontent.com/82181327/122643554-54f1ac00-d108-11eb-906f-6748d2facf2f.jpg

https://user-images.githubusercontent.com/101634267/245153650-cd8335df-01ec-47e6-97b2-7f7244d95efb.jpg

https://user-images.githubusercontent.com/101634267/245153665-cb9506d2-6d0d-4231-8a42-5f8ab15cf4d4.jpg

![after_image_cut](https://github.com/oppia/oppia-android/assets/76042077/babacf98-1ef6-4f29-98a4-cc46c5f53303)

![after_imagecut](https://github.com/oppia/oppia-android/assets/76042077/e08cf8d0-5f25-46bb-afbe-124952d19bfc)

![after_image_cut2](https://github.com/oppia/oppia-android/assets/76042077/85e950df-1ae9-48f2-b46c-c610b838593a)

![After_RTL](https://github.com/oppia/oppia-android/assets/76042077/586ade28-a45e-4f0a-9ac8-51457ccf8b5a)

![after_no_effect_RTL](https://github.com/oppia/oppia-android/assets/76042077/75820a6d-cc76-4ec0-b90e-ede7e5a72643)

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Localisation updates from https://translatewiki.net. (#5242)

Translation updates

Fix #5137: Upgrade builds to target SDK 33 (#5222)

Fixes #5137
Partially mitigates #5233

This updates all build & target SDKs for Gradle & Bazel builds to now
target SDK 33, per the new Play Store mandate (see
https://support.google.com/googleplay/android-developer/answer/11926878?hl=en).
The analysis of SDK 33 features, potential issues, and potential
mitigations are described in #5137. It was determined that there are no
obvious code changes needed beyond the minimum to target SDK 33. We'll
be relying on some platform-level regression testing plus the Play
Console's pre-submit app analysis report for finalizing the go/no-go
decision for SDK 33 support.

Testing consisted of manually playing through the app and seeing if
there were any issues using emulators both with SDK 33 and lower
versions. #5137 documents the issues found and their ultimate causes.
Since no functionality changes were needed for SDK 33, no tests needed
updating. Separately, tests are still running on SDK 30 due to being
stuck (see #4748).

One issue unrelated to SDK 33 was observed: when playing through the
prototype lesson, I once again brought the app into a broken state
wherein I couldn't leave the lesson via navigation. We've hit this issue
a few times, but still don't know the root cause. #5233 was filed and a
mitigation was introduced in this PR (+ a test for it): if the
exploration player observes a failure when trying to stop the player
session (for whatever reason), it still proceeds with its "end of
activity" flow (e.g. finishing the activity). This provides at least an
escape hatch for users who somehow hit this state. Note that this isn't
a proper fix for #5233 given the lack of a known root cause, so this is
only considered a mitigation.

The new test was verified as failing if the mitigation is omitted:

![image](https://github.com/oppia/oppia-android/assets/12983742/10ddbf85-332c-4469-8efa-483a967170f9)

Note that ``ExplorationActivityTest``'s setup was tweaked to change the
parent screen since this affects back navigation routing. I opted to
using lessons tab as the parent screen since it's the most common route
for users to hit, so it makes sense for our tests to default to that if
they don't specifically need to use a different route (and none of the
existing tests seemed to have an issue with this change).

- [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
- [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)).

N/A -- This isn't changing any UIs directly, and per the analysis in
33 that would affect Oppia.

Technical Analytics: Milestone 1 - Add Feature Flag Statuses and Ability To Sync Them to Cache Store (#5203)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
When merged, this PR will:
- Create a new `FeatureFlagConstants.kt` file to contain all feature
flags.
- Create feature flag sync status trackers for each existing feature
flag.
- Modify the `PlatformParameterSyncUpWorker` to allow status trackers of
all feature flags to be synced and cached in the PersistentCacheStore.
- Merge the `TestBooleanPlatformParameter`,
`TestStringPlatformParameter` and the `TestIntegerPlatformParameter`
files into the `TestPlatformParameterConstants` file.
- Add syncing with the web for the `EnableDownloadsSupport`,
`EnableEditAccountsOptionsUi`, `EnableSpotlightUi`,
`EnableExtraTopicTabsUi`, `EnableInteractionConfigChangeStateRetention`,
and `EnableAppAndOsDeprecation` feature flags.
- Write tests for the changes made.

<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
- [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)).

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
Co-authored-by: Ben Henning <ben@oppia.org>

Refactor TestAuthenticationModule.kt to provide FirebaseAuthWrapper

Ensure firestore logs show in dev event logs view.

Removed redundant bindings.

Remove FakeAuthenticationController and refactored its usages to the new FakeFirebaseWrapperImpl

Remove DebugFirestoreEventLogger.kt and usages.

Update AuthenticationControllerTest tests

Fix static analysis checks

Fix bazel build errors

Reformat auth/BUILD.bazel

Create wrapper for firebase auth instance

Fix lint errors

Fix failing tests

Fix failing lint checks

Fix test failures

Fix BUILD file formatting

Fix DebugFirestoreEventLoggerImplTest

Fake a firebase instance.

Fix build failures

Fix test file exemption warnings

Fix bazel build input file

Fix bazel build input file

Fix failing tests
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: Low Solution is clear and broken into good-first-issue-sized chunks.
Projects
Development

No branches or pull requests

8 participants