Skip to content

Commit

Permalink
Fix #3972: Added TextViewStyleCheck script (#5599)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix #3972 

This PR adds a script, `TextViewStyleCheck.kt`, which ensures that all
`TextView` elements in the layout XML files use centrally managed
styles. If any `TextView` does not have a style attribute that starts
with `@style/`, an error is reported, indicating the file path where the
violation occurs.

`bazel run //scripts:check_textview_styles -- $(pwd)`

To execute the tests, use:
`bazel test
//scripts/src/javatests/org/oppia/android/scripts/xml:TextViewStyleCheckTest`

made changes to the `scripts/assets/todo_open_exemptions.textproto` as
the TextviewStyleCheck script was added

## 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)).
  • Loading branch information
manas-yu authored Jan 28, 2025
1 parent 0717ce8 commit f6d43d1
Show file tree
Hide file tree
Showing 18 changed files with 689 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ jobs:
run: |
bazel run //scripts:xml_syntax_check -- $(pwd)
- name: TextView Style Validation Check
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:check_textview_styles -- $(pwd)
- name: Testfile Presence Check
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout-land/profile_chooser_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
android:contentDescription="@string/language_icon_content_description" />

<TextView
android:id="@+id/profile_chooser_language_text_view"
style="@style/Subtitle1ViewCenter"
android:minHeight="48dp"
android:paddingTop="20dp"
Expand Down Expand Up @@ -100,6 +101,7 @@
app:layout_constraintEnd_toEndOf="parent">

<TextView
android:id="@+id/administrator_controls_text_view"
style="@style/Subtitle1ViewCenter"
android:layout_marginBottom="24dp"
android:minHeight="48dp"
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/res/layout-land/walkthrough_final_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
app:contentPadding="@dimen/walkthrough_final_fragment_card_content_padding">

<TextView
android:id="@+id/walkthrough_final_no_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
Expand All @@ -61,6 +62,7 @@
android:textSize="20sp" />

<TextView
android:id="@+id/walkthrough_final_no_center_text_view"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand All @@ -85,6 +87,7 @@
app:contentPadding="@dimen/walkthrough_final_fragment_card_content_padding">

<TextView
android:id="@+id/walkthrough_final_yes_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
Expand All @@ -94,6 +97,7 @@
android:textSize="20sp" />

<TextView
android:id="@+id/walkthrough_final_yes_center_text_view"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
</FrameLayout>

<TextView
android:id="@+id/story_progress_percentage_text_view"
style="@style/TextViewCenterHorizontal"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout-sw600dp/profile_chooser_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
android:contentDescription="@string/language_icon_content_description" />

<TextView
android:id="@+id/profile_chooser_language_text_view"
style="@style/Subtitle1ViewCenter"
android:minHeight="48dp"
android:paddingTop="20dp"
Expand Down Expand Up @@ -108,6 +109,7 @@
app:layout_constraintEnd_toEndOf="parent">

<TextView
android:id="@+id/administrator_controls_text_view"
style="@style/Heading1ViewCenter"
android:layout_marginBottom="@dimen/profile_chooser_administrator_controls_margin_bottom"
android:minHeight="48dp"
Expand Down
11 changes: 4 additions & 7 deletions app/src/main/res/layout/drawer_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
android:contentDescription="@string/developer_options_icon_content_description" />

<TextView
android:id="@+id/developer_options_text_view"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
Expand Down Expand Up @@ -103,14 +104,10 @@
android:contentDescription="@string/administrator_controls_icon_content_description" />

<TextView
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
android:layout_marginStart="12dp"
android:fontFamily="sans-serif-medium"
android:id="@+id/administrator_controls_text_view"
style="@style/AdministratorControlsText"
android:text="@string/administrator_controls"
android:textColor="@{footerViewModel.isAdministratorControlsSelected ? @color/component_color_drawer_fragment_admin_controls_selected_text_color : @color/component_color_shared_primary_dark_text_color}"
android:textSize="14sp" />
android:textColor="@{footerViewModel.isAdministratorControlsSelected ? @color/component_color_drawer_fragment_admin_controls_selected_text_color : @color/component_color_shared_primary_dark_text_color}" />
</LinearLayout>
</LinearLayout>
</androidx.core.widget.NestedScrollView>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/numeric_input_interaction_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
app:textChangedListener="@{viewModel.answerTextWatcher}" />

<TextView
android:id="@+id/numeric_input_interaction_text_view"
style="@style/InputInteractionErrorTextView"
android:text="@{viewModel.errorMessage}"
android:textColor="@color/component_color_shared_input_error_color"
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout/profile_chooser_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
android:contentDescription="@string/language_icon_content_description" />

<TextView
android:id="@+id/profile_chooser_language_text_view"
style="@style/Subtitle1ViewCenter"
android:minHeight="48dp"
android:paddingTop="20dp"
Expand Down Expand Up @@ -100,6 +101,7 @@
app:layout_constraintEnd_toEndOf="parent">

<TextView
android:id="@+id/administrator_controls_text_view"
style="@style/Subtitle1ViewCenter"
android:layout_marginBottom="24dp"
android:minHeight="48dp"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/topic_lessons_story_summary.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
</FrameLayout>

<TextView
android:id="@+id/story_progress_percentage_text_view"
style="@style/TextViewCenterHorizontal"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/res/layout/walkthrough_final_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
app:contentPadding="@dimen/walkthrough_final_fragment_card_content_padding">

<TextView
android:id="@+id/walkthrough_final_no_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
Expand All @@ -66,6 +67,7 @@
android:textSize="20sp" />

<TextView
android:id="@+id/walkthrough_final_no_center_text_view"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand All @@ -90,6 +92,7 @@
app:contentPadding="@dimen/walkthrough_final_fragment_card_content_padding">

<TextView
android:id="@+id/walkthrough_final_yes_text_view"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
Expand All @@ -99,6 +102,7 @@
android:textSize="20sp" />

<TextView
android:id="@+id/walkthrough_final_yes_center_text_view"
style="@style/TextViewCenter"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
Expand Down
9 changes: 9 additions & 0 deletions app/src/main/res/values/styles.xml
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,15 @@
<item name="android:textSize">@dimen/onboarding_shared_text_size_large</item>
</style>

<style name="AdministratorControlsText">
<item name="android:layout_width">match_parent</item>
<item name="android:layout_height">wrap_content</item>
<item name="android:layout_gravity">center_vertical</item>
<item name="android:layout_marginStart">12dp</item>
<item name="android:fontFamily">sans-serif-medium</item>
<item name="android:textSize">14sp</item>
</style>

<style name="LanguageDropdownStyle" parent="Widget.MaterialComponents.TextInputLayout.FilledBox.ExposedDropdownMenu">
<item name="android:layout_width">match_parent</item>
<item name="android:layout_height">wrap_content</item>
Expand Down
7 changes: 7 additions & 0 deletions scripts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,10 @@ java_binary(
main_class = "org.oppia.android.scripts.telemetry.DecodeUserStudyEventStringKt",
runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/telemetry:decode_user_study_event_string_lib"],
)

kt_jvm_binary(
name = "check_textview_styles",
testonly = True,
main_class = "org.oppia.android.scripts.xml.TextViewStyleCheckKt",
runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles"],
)
2 changes: 1 addition & 1 deletion scripts/assets/todo_open_exemptions.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ todo_open_exemption {
}
todo_open_exemption {
exempted_file_path: "scripts/static_checks.sh"
line_number: 114
line_number: 121
}
todo_open_exemption {
exempted_file_path: "wiki/Coding-style-guide.md"
Expand Down
11 changes: 11 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,14 @@ kt_jvm_library(
"//scripts/src/java/org/oppia/android/scripts/common:repository_file",
],
)

kt_jvm_library(
name = "check_textview_styles",
testonly = True,
srcs = ["TextViewStyleCheck.kt"],
visibility = ["//scripts:oppia_script_binary_visibility"],
deps = [
":xml_syntax_error_handler",
"//scripts/src/java/org/oppia/android/scripts/common:repository_file",
],
)
Loading

0 comments on commit f6d43d1

Please sign in to comment.