Fix oppia#3746: Add regex to ban <include> tag in layout files. (oppi…

…a#4656)  ## Explanation Fixes [oppia#3746](oppia#3746) This PR bans all usage of the <include...> tag in XML layout files that reference it. It does so by adding a check block inside the file_content_validation_checks text proto script file. This pr also removes the toolbar.xml file, since its no longer being used.  ## Essential Checklist  - [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  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 ## SCREENSHOT FOR THE REGEX PATTERN CHECKS FAILED ![checksfailed](https://user-images.githubusercontent.com/31986629/196036816-ddb6e065-d4bc-4f96-95e7-f5228ae3e23d.PNG) ## SCREENSHOT FOR THE REGEX PATTERN CHECKS PASSED ![passed](https://user-images.githubusercontent.com/31986629/196038476-e54e2319-6567-4b96-934d-c040b36360c5.PNG) ## A LIST OF THE FILES THAT HAVE THE <INCLUDE.../> TAG - app/src/main/res/layout/add_profile_activity.xml - addprofileactivity - app/src/main/res/layout/administrator_controls_activity.xml:15: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/help_without_drawer_activity.xml:11: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/option_activity.xml:13: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/options_without_drawer_activity.xml:11: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/developer_options_activity.xml:15: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/home_activity.xml:14: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/help_activity.xml:13: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout/profile_progress_activity.xml:9: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout-sw600dp/administrator_controls_activity.xml:15: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout-sw600dp/option_activity.xml:13: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout-sw600dp/options_without_drawer_activity.xml:12: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead - app/src/main/res/layout-sw600dp/help_activity.xml:13: Remove <include .../> tag from layout and use the appropriate widget, e.g like AppBarLayout widget instead Co-authored-by: MAZAKPE <[email protected]> Co-authored-by: Ben Henning <[email protected]>
adhiamboperes · Nov 10, 2022 · 40bf82b · 40bf82b
1 parent da9b945
commit 40bf82b
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 40 deletions.
diff --git a/app/src/main/res/layout-sw600dp/administrator_controls_activity.xml b/app/src/main/res/layout-sw600dp/administrator_controls_activity.xml
@@ -12,12 +12,18 @@
       android:layout_height="match_parent"
       android:background="@color/oppia_grey_background">
 
-      <include
+      <androidx.appcompat.widget.Toolbar
         android:id="@+id/administrator_controls_activity_toolbar"
-        layout="@layout/toolbar"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:background="@color/component_color_shared_activity_toolbar_color"
+        android:minHeight="?attr/actionBarSize"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent" />
+        app:layout_constraintTop_toTopOf="parent"
+        app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+        app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+        app:titleTextAppearance="@style/ToolbarTitle" />
 
       <FrameLayout
         android:id="@+id/administrator_controls_fragment_placeholder"

diff --git a/app/src/main/res/layout-sw600dp/help_activity.xml b/app/src/main/res/layout-sw600dp/help_activity.xml
@@ -10,9 +10,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/help_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <androidx.constraintlayout.widget.ConstraintLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout-sw600dp/option_activity.xml b/app/src/main/res/layout-sw600dp/option_activity.xml
@@ -10,9 +10,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/options_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <androidx.constraintlayout.widget.ConstraintLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout-sw600dp/options_without_drawer_activity.xml b/app/src/main/res/layout-sw600dp/options_without_drawer_activity.xml
@@ -9,9 +9,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/options_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <androidx.constraintlayout.widget.ConstraintLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/add_profile_activity.xml b/app/src/main/res/layout/add_profile_activity.xml
@@ -16,9 +16,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
-      android:id="@+id/add_profile_activity_toolbar"
-      layout="@layout/toolbar" />
+    <androidx.appcompat.widget.Toolbar
+       android:id="@+id/add_profile_activity_toolbar"
+       android:layout_width="match_parent"
+       android:layout_height="wrap_content"
+       android:background="@color/component_color_shared_activity_toolbar_color"
+       android:minHeight="?attr/actionBarSize"
+       app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+       app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+       app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/administrator_controls_activity.xml b/app/src/main/res/layout/administrator_controls_activity.xml
@@ -12,9 +12,15 @@
       android:layout_height="match_parent"
       android:orientation="vertical">
 
-      <include
+      <androidx.appcompat.widget.Toolbar
         android:id="@+id/administrator_controls_activity_toolbar"
-        layout="@layout/toolbar" />
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:background="@color/component_color_shared_activity_toolbar_color"
+        android:minHeight="?attr/actionBarSize"
+        app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+        app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+        app:titleTextAppearance="@style/ToolbarTitle" />
 
       <FrameLayout
         android:id="@+id/administrator_controls_fragment_placeholder"
@@ -31,4 +37,4 @@
       app:layout="@layout/drawer_fragment"
       tools:layout="@layout/drawer_fragment" />
   </androidx.drawerlayout.widget.DrawerLayout>
-</layout>
+</layout>
diff --git a/app/src/main/res/layout/developer_options_activity.xml b/app/src/main/res/layout/developer_options_activity.xml
@@ -12,9 +12,15 @@
       android:layout_height="match_parent"
       android:orientation="vertical">
 
-      <include
+      <androidx.appcompat.widget.Toolbar
         android:id="@+id/developer_options_activity_toolbar"
-        layout="@layout/toolbar" />
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:background="@color/component_color_shared_activity_toolbar_color"
+        android:minHeight="?attr/actionBarSize"
+        app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+        app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+        app:titleTextAppearance="@style/ToolbarTitle" />
 
       <FrameLayout
         android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/help_activity.xml b/app/src/main/res/layout/help_activity.xml
@@ -10,9 +10,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/help_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/help_without_drawer_activity.xml b/app/src/main/res/layout/help_without_drawer_activity.xml
@@ -1,4 +1,5 @@
 <androidx.drawerlayout.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto"
   android:id="@+id/help_activity_drawer_layout"
   android:layout_width="match_parent"
   android:layout_height="match_parent">
@@ -8,9 +9,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/help_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/home_activity.xml b/app/src/main/res/layout/home_activity.xml
@@ -11,9 +11,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/home_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:id="@+id/home_fragment_placeholder"

diff --git a/app/src/main/res/layout/option_activity.xml b/app/src/main/res/layout/option_activity.xml
@@ -10,9 +10,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/options_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/options_without_drawer_activity.xml b/app/src/main/res/layout/options_without_drawer_activity.xml
@@ -1,4 +1,5 @@
 <androidx.drawerlayout.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto"
   android:id="@+id/options_activity_drawer_layout"
   android:layout_width="match_parent"
   android:layout_height="match_parent">
@@ -8,9 +9,15 @@
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <include
+    <androidx.appcompat.widget.Toolbar
       android:id="@+id/options_activity_toolbar"
-      layout="@layout/toolbar" />
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:background="@color/component_color_shared_activity_toolbar_color"
+      android:minHeight="?attr/actionBarSize"
+      app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+      app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+      app:titleTextAppearance="@style/ToolbarTitle" />
 
     <FrameLayout
       android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/profile_progress_activity.xml b/app/src/main/res/layout/profile_progress_activity.xml
@@ -1,14 +1,21 @@
 <?xml version="1.0" encoding="utf-8"?>
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto"
   xmlns:tools="http://schemas.android.com/tools"
   android:layout_width="match_parent"
   android:layout_height="match_parent"
   android:orientation="vertical"
   tools:context=".app.profileprogress.ProfileProgressActivity">
 
-  <include
+  <androidx.appcompat.widget.Toolbar
     android:id="@+id/profile_progress_activity_toolbar"
-    layout="@layout/toolbar" />
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content"
+    android:background="@color/component_color_shared_activity_toolbar_color"
+    android:minHeight="?attr/actionBarSize"
+    app:popupTheme="@style/ThemeOverlay.AppCompat.Light"
+    app:theme="@style/ThemeOverlay.AppCompat.Dark.ActionBar"
+    app:titleTextAppearance="@style/ToolbarTitle" />
 
   <FrameLayout
     android:layout_width="match_parent"

diff --git a/app/src/main/res/layout/toolbar.xml b/app/src/main/res/layout/toolbar.xml
diff --git a/scripts/assets/file_content_validation_checks.textproto b/scripts/assets/file_content_validation_checks.textproto
@@ -36,6 +36,11 @@ file_content_checks {
   exempted_file_name: "domain/src/main/java/org/oppia/android/domain/oppialogger/logscheduler/MetricLogSchedulingWorker.kt"
   exempted_file_name: "scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt"
 }
+file_content_checks {
+  file_path_regex: ".+?\\.xml"
+  prohibited_content_regex: "<include"
+  failure_message: "Remove <include .../> tag from layouts and instead use the widget directly, e.g. AppBarLayout."
+}
 file_content_checks {
   file_path_regex: ".+?\\.xml"
   prohibited_content_regex: "android:gravity=\"left\""

diff --git a/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt
@@ -34,6 +34,8 @@ class RegexPatternValidationCheckTest {
   private val settableFutureUsageErrorMessage =
     "SettableFuture should only be used in pre-approved locations since it's easy to potentially " +
       "mess up & lead to a hanging ListenableFuture."
+  private val androidLayoutIncludeTagErrorMessage =
+    "Remove <include .../> tag from layouts and instead use the widget directly, e.g. AppBarLayout."
   private val androidGravityLeftErrorMessage =
     "Use android:gravity=\"start\", instead, for proper RTL support"
   private val androidGravityRightErrorMessage =
@@ -384,6 +386,26 @@ class RegexPatternValidationCheckTest {
       )
   }
 
+  @Test
+  fun testFileContent_androidLayoutIncludeTag_fileContentIsNotCorrect() {
+    val prohibitedContent = "<include"
+    val fileContainsSupportLibraryImport = tempFolder.newFile("testfiles/test_layout.xml")
+    fileContainsSupportLibraryImport.writeText(prohibitedContent)
+
+    val exception = assertThrows(Exception::class) {
+      runScript()
+    }
+
+    assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)
+    assertThat(outContent.toString().trim())
+      .isEqualTo(
+        """
+        test_layout.xml:1: $androidLayoutIncludeTagErrorMessage
+        $wikiReferenceNote
+        """.trimIndent()
+      )
+  }
+
   @Test
   fun testFileContent_androidGravityLeft_fileContentIsNotCorrect() {
     val prohibitedContent = "android:gravity=\"left\""