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

Fix part of #5661: Add Missing style Attributes to TextViews #5682

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open
15 changes: 6 additions & 9 deletions app/src/main/res/layout/walkthrough_final_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,11 @@

<TextView
android:id="@+id/walkthrough_final_no_text_view"
style="@style/WalkthroughFinalNoTextViewStyle"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
android:fontFamily="sans-serif"
android:text="@string/no"
android:textColor="@color/component_color_shared_primary_text_color"
android:textSize="20sp" />
android:text="@string/no" />

<TextView
android:id="@+id/walkthrough_final_no_center_text_view"
Expand Down Expand Up @@ -93,13 +91,12 @@

<TextView
android:id="@+id/walkthrough_final_yes_text_view"
style="@style/WalkthroughFinalYesTextViewStyle"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal"
android:fontFamily="sans-serif"
android:text="@string/yes"
android:textColor="@color/component_color_shared_secondary_6_text_color"
android:textSize="20sp" />
android:text="@string/yes" />


<TextView
android:id="@+id/walkthrough_final_yes_center_text_view"
Expand Down Expand Up @@ -146,4 +143,4 @@
app:layout_constraintTop_toTopOf="parent" />
</androidx.constraintlayout.widget.ConstraintLayout>
</ScrollView>
</layout>
</layout>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always add an extra line at the end of the file

13 changes: 13 additions & 0 deletions app/src/main/res/values/styles.xml
Original file line number Diff line number Diff line change
Expand Up @@ -826,4 +826,17 @@
<item name="endIconDrawable">@drawable/ic_arrow_drop_down_black_24dp</item>
<item name="endIconTint">@color/component_color_shared_black_background_color</item>
</style>

<style name="WalkthroughFinalNoTextViewStyle" parent="TextView.Common1">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can reuse these styles for the respective IDs present in layout-land/walkthrough_final_fragment.xml

<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>
</style>

<style name="WalkthroughFinalYesTextViewStyle" parent="TextView.Common1">
<item name="android:fontFamily">sans-serif</item>
<item name="android:textSize">20sp</item>
<item name="android:textColor">@color/component_color_shared_secondary_6_text_color</item>
</style>

</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ private class TextViewStyleCheck {
private val attributeIds = listOf(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah looks like this was overlooked in #5599 the attributeIds has repeated IDs and could cause an issue it would be better to replace it with the below :

Set of ids

private val attributeIds = listOf(
  "@+id/developer_options_text_view",
  "@+id/onboarding_language_text_view",
  "@+id/walkthrough_final_no_text_view",
  "@+id/walkthrough_final_yes_text_view",
  "@+id/walkthrough_final_title_text_view",
  "@+id/chapter_index",
  "@+id/test_text_view",
  "@+id/feedback_text_view",
  "@+id/item_selection_contents_text_view",
  "@+id/learner_analytics_sync_status_text_view",
  "@+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"
)

I think we can handle this change in this PR only as opening a separate one for this issue doesn't seem necessary. Please replace the current attributeIds with the one I provided above after removing the IDs you have already added the style attribute for.

"@+id/developer_options_text_view",
"@+id/onboarding_language_text_view",
"@+id/walkthrough_final_no_text_view",
"@+id/walkthrough_final_yes_text_view",
"@+id/walkthrough_final_title_text_view",
"@+id/chapter_index",
"@+id/chapter_index",
Expand Down
Loading