Fix part of #5661: Add Missing style Attributes to TextViews #5682
Conversation
|
am i heading right? @manas-yu |
| @@ -146,4 +143,4 @@ | |||
| app:layout_constraintTop_toTopOf="parent" /> | |||
| </androidx.constraintlayout.widget.ConstraintLayout> | |||
| </ScrollView> | |||
| </layout> | |||
| </layout> | |||
There was a problem hiding this comment.
Always add an extra line at the end of the file
|
Hi @Harsh2118-hub , Ensure that all You can even confirm after removing the IDs if any reference is left out by running the command: |
|
@manas-yu thanks for guidance |
|
please take a look @manas-yu , this time the command- bazel run //scripts:check_textview_styles -- $(pwd) is working |
|
Unassigning @Harsh2118-hub since a re-review was requested. @Harsh2118-hub, please make sure you have addressed all review comments. Thanks! |
scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt
Outdated
Show resolved
Hide resolved
| </layout> | ||
| style="@style/TestTextViewStyle" | ||
| android:text="@string/app_name" /> | ||
| </layout> |
There was a problem hiding this comment.
Add an extra line at the end of the file, same for the rest.
|
Thanks @Harsh2118-hub for the changes and PTAL at this comment and re-run the command. The only reference left is the Also add to the PR description for replacing the list of |
|
screenshots for reference |
|
@manas-yu PTAL , can i move forward with remaning attributes id which are left . |
|
Unassigning @Harsh2118-hub since a re-review was requested. @Harsh2118-hub, please make sure you have addressed all review comments. Thanks! |
|
Thanks @Harsh2118-hub for the changes! You can work on three more IDs to complete the PR as mentioned here, and it will be good to go. Also please update the PR description and title as I mentioned earlier |
|
screenshots for refernce |
|
Thanks, @Harsh2118-hub. This looks good. Just update the PR title and description with "Fix part of #5661" and resolve the left out comment. |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 3636 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 367 bytes (Added) Method count: 260299 (old), 260299 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6830 (old), 6840 (new), 10 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 3596 bytes (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 1940 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1505 bytes (Added) Method count: 115822 (old), 115822 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 1908 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2180 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 28 bytes (Removed) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2140 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2144 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 17 bytes (Added) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2108 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
Harsh2118-hub
changed the title
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 3640 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 324 bytes (Added) Method count: 260299 (old), 260299 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6830 (old), 6840 (new), 10 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 3604 bytes (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 1928 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1459 bytes (Added) Method count: 115822 (old), 115822 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 1892 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2168 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 15 bytes (Removed) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2132 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2140 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 67 bytes (Added) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2108 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
|
PTAL @adhiamboperes |
|
Hi @manas-yu , I wanted to follow up on this PR and check if there are any updates. Please let me know if there's anything I can do to assist or anything I can do to expedite the review. |
|
Hi @Harsh2118-hub, a final review and approval from (@)adhiamboperes is required and will be provided in the coming days. In the meantime, you can work on other good first issues. |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 3544 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 1075 bytes (Added) Method count: 260299 (old), 260299 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6830 (old), 6840 (new), 10 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 3504 bytes (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2008 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 2499 bytes (Added) Method count: 115822 (old), 115822 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 1976 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2140 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 43 bytes (Added) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2104 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 2164 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 3964 bytes (Added) Method count: 115828 (old), 115828 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5798 (old), 5799 (new), 1 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 2128 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
|
Apologies @Harsh2118-hub, I am currently reviewing this PR. |
There was a problem hiding this comment.
Thanks @ Harsh2118-hub, and apologies for the delayed review. I have left some comments inline. PTAL.
Also, @manas-yu, thanks for taking a first review pass!
app/src/main/res/values/styles.xml
Outdated
| <item name="android:layout_width">wrap_content</item> | ||
| <item name="android:layout_height">wrap_content</item> |
There was a problem hiding this comment.
These attributes are duplicated in the view using this style. Android will prioritize the attributes added directly to the view over the ones from the style, and if they are the same, then these here become redudant
app/src/main/res/values/styles.xml
Outdated
| <item name="android:fontFamily">sans-serif</item> | ||
| </style> | ||
|
|
||
| <style name="ChapterIndexTextViewStyle" parent="TextView.Common1"> |
There was a problem hiding this comment.
This style is used in 4 places, but it contains some attributes that dont have the same values for the different screens, e.g minWidth and minHeight, as well as padding values, are not consistent. The style should be trimmed down to what is similar accross all 4 views, and whatever is different should be maintained directly in the view.
|
tthanks, @adhiamboperes for review , i will be soon resolving the comments . |
|
@Harsh2118-hub, do you require any support in this PR? |
|
@adhiamboperes ,PTAL i have resolved the inline comments. |
|
Unassigning @Harsh2118-hub since a re-review was requested. @Harsh2118-hub, please make sure you have addressed all review comments. Thanks! |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 552 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 964 bytes (Removed) Method count: 260329 (old), 260299 (new), 30 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6832 (old), 6840 (new), 8 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 512 bytes (Added)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 768 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 52 bytes (Removed) Method count: 115836 (old), 115822 (new), 14 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5807 (new), 7 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 724 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 888 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1831 bytes (Removed) Method count: 115842 (old), 115828 (new), 14 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5807 (new), 7 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 848 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 864 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1669 bytes (Added) Method count: 115842 (old), 115828 (new), 14 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5807 (new), 7 (Added)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 820 bytes (Added)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
app/src/main/res/values/styles.xml
Outdated
| <item name="android:layout_height">wrap_content</item> | ||
| <item name="android:gravity">bottom</item> | ||
| <item name="android:textSize">16sp</item> | ||
| <item name="android:textColor">@color/component_color_shared_primary_text_color</item> |
There was a problem hiding this comment.
this textColor attribute was not present in the original "@+id/learner_analytics_sync_status_text_view" and should be removed
app/src/main/res/values/styles.xml
Outdated
| <item name="android:layout_height">wrap_content</item> | ||
| <item name="android:fontFamily">sans-serif</item> | ||
| <item name="android:textSize">16sp</item> | ||
| <item name="android:textColor">@color/component_color_shared_primary_text_color</item> |
There was a problem hiding this comment.
the textColor attribute is not needed.
app/src/main/res/values/styles.xml
Outdated
| <item name="android:fontFamily">sans-serif-medium</item> | ||
| <item name="android:padding">10dp</item> | ||
| <item name="android:textSize">18sp</item> | ||
| <item name="android:textColor">@color/component_color_shared_primary_text_color</item> |
There was a problem hiding this comment.
textColor attribute not needed
app/src/main/res/values/styles.xml
Outdated
| <item name="android:fontFamily">sans-serif</item> | ||
| </style> | ||
|
|
||
| <style name="ChapterIndexTextViewStyle" parent="TextView.Common1"> |
There was a problem hiding this comment.
There are inconsistencies in how this style is applied across different TextViews. Extract the common attributes from the TextViews and define them in the style to ensure uniformity.
app/src/main/res/values/styles.xml
Outdated
| <item name="android:padding">8dp</item> | ||
| </style> | ||
|
|
||
| <style name="WalkthroughFinalTitleTextViewStyle" parent="TextView.Common1"> |
There was a problem hiding this comment.
android:text="@string/great" need to be included
app/src/main/res/values/styles.xml
Outdated
| <style name="DeveloperOptionsTextViewStyle" parent="TextView.Common1"> | ||
| <item name="android:fontFamily">sans-serif-medium</item> | ||
| <item name="android:textSize">14sp</item> | ||
| <item name="android:textColor">@color/component_color_shared_primary_dark_text_color</item> |
There was a problem hiding this comment.
Thanks for making the changes @Harsh2118-hub!
Some general feedback that I have:
- It’s generally expected to respond to a reviewer comment, especially questions. If it is not a question, responding with “done” is sufficient. This is important because:
- we can discuss the reasoning behind the changes made or the requested changes.
- Helps both you and the reviewer to keep track of which comments have been addressed. - The screenshots you have shared so far are from the layout viewer. We generally expect real screenshots after the app has been deployed to a device or emulator. Additionally, some layouts are not rendered in the layout window in AS since they contain custom views.
- In case you haven’t set up the app to build with Bazel, please refer to our discussions section for guidance.
| @@ -38,6 +38,9 @@ | |||
| style="@style/ItemSelectionContentsTextViewStyle" | |||
| android:layout_toEndOf="@+id/item_selection_checkbox" | |||
| android:text="@{htmlContent}" | |||
| android:layout_marginStart="8dp" | |||
There was a problem hiding this comment.
Please check the indentation level of these attributes.
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="Language" | ||
| app:layout_constraintStart_toStartOf="parent" |
There was a problem hiding this comment.
I’ve noticed that you have correctly returned this attributes, but may have forgotten to remove them from styles. Please check.
There was a problem hiding this comment.
can you please elaborate what are you saying . That might be helpful @adhiamboperes
Harsh2118-hub
force-pushed
the
fix5661
branch
from
db0b840 to
f13ac52
Compare
|
Apologies for the inconvenience, but the previous PR was deleted due to a mistake on my end. I’ve created a new PR with the same content, and everything should be in order now. Thank you for your understanding and support! #5710 |
"Fix part of #5661"
-Edited attributeIds list in TextViewStyleCheck which don't has repeated IDs and id's for those text view and style.xml file has been updated.
-edited list
private val attributeIds = listOf(
"@+id/text_view_for_int_no_data_binding",
"@+id/walkthrough_topic_name_text_view",
"@+id/walkthrough_lesson_count_text_view",
"@+id/hint_bar_title",
"@+id/coming_soon_text_view",
"@+id/topic_name_text_view",
"@+id/lesson_count_text_view",
"@+id/multiple_choice_content_text_view",
"@+id/language_text_view",
"@+id/welcome_text_view",
"@+id/app_version_text_view",
"@+id/app_last_update_date_text_view",
"@+id/test_margin_text_view",
"@+id/content_text_view",
"@+id/action_options",
"@+id/action_help",
"@+id/action_close",
"@+id/continue_studying_text_view",
"@+id/language_unavailable_notice",
"@+id/story_progress_chapter_completed_text",
"@+id/profile_id_view_profile_name",
"@+id/profile_id_view_learner_id",
"@+id/learner_events_waiting_upload_label",
"@+id/learner_events_waiting_upload_count",
"@+id/learner_events_uploaded_label",
"@+id/learner_events_uploaded_count",
"@+id/uncategorized_events_waiting_upload_label",
"@+id/uncategorized_events_waiting_upload_count",
"@+id/uncategorized_events_uploaded_label",
"@+id/uncategorized_events_uploaded_count",
"@+id/text_view_for_live_data_no_data_binding",
"@+id/selection_interaction_textview",
"@+id/onboarding_steps_count",
"@+id/profile_picture_edit_dialog_view_picture",
"@+id/profile_picture_edit_dialog_change_picture",
"@+id/chapter_title",
"@+id/walkthrough_welcome_title_text_view",
"@+id/story_name_text_view",
"@+id/copyright_license_text_view",
"@+id/ga_update_notice_dialog_message",
"@+id/create_profile_picture_prompt",
"@+id/profile_reset_pin_main",
"@+id/submitted_answer_text_view",
"@+id/end_session_header_text_view",
"@+id/end_session_body_text_view",
"@+id/question_progress_text",
"@+id/congratulations_text_view",
"@+id/beta_notice_dialog_message",
"@+id/onboarding_language_explanation",
"@+id/create_profile_title",
"@+id/profile_name_text_view",
"@+id/resume_lesson_chapter_title_text_view",
"@+id/story_count_text_view",
"@+id/download_size_text_view",
"@+id/options_activity_selected_options_title",
"@+id/profile_select_text",
"@+id/extra_controls_title",
"@+id/view_all_text_view"
)
Applied the new styles in the respective layout XML files
Removed TextView attribute IDs from the exception list in TextViewStyleCheck.kt
-Created a new styles in styles.xml to define text appearance consistently.
-Updated textview to apply the newly created style .
from the exception list in TextViewStyleCheck.kt since it now follows the required style guidelines.
Essential Checklist