From bef98698cb7db00ebe90e5fa0f275bf17e5658ea Mon Sep 17 00:00:00 2001
From: Kainalu Hagiwara <kainaluh808@gmail.com>
Date: Fri, 24 Jul 2020 09:42:01 -0700
Subject: [PATCH] For #12904 - Conditionally inflate SwipeGestureLayout in
 BaseBrowserFragment.

Changing the root view of BaseBrowserFragment from a CoordinatorLayout
to a SwipeGestureLayout has caused some regressions, particularly in
snackbar behavior. Lets prevent those regressions from occuring in
builds where the feature flag for gestures is off by only adding the
SwipeGestureLayout when the feature flag is on.
---
 .../java/org/mozilla/fenix/FeatureFlags.kt    |  4 +-
 .../fenix/browser/BaseBrowserFragment.kt      |  8 +-
 .../mozilla/fenix/browser/BrowserFragment.kt  |  5 +-
 .../res/layout/browser_gesture_wrapper.xml    | 17 ++++
 app/src/main/res/layout/fragment_browser.xml  | 94 ++++++++-----------
 5 files changed, 70 insertions(+), 58 deletions(-)
 create mode 100644 app/src/main/res/layout/browser_gesture_wrapper.xml

diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
index d37025e14..530d77be8 100644
--- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
+++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
@@ -30,9 +30,9 @@ object FeatureFlags {
     val tabTray = Config.channel.isNightlyOrDebug
 
     /**
-     * Enables swipe on toolbar to switch tabs
+     * Enables gestures on the browser chrome that depend on a [SwipeGestureLayout]
      */
-    val swipeToSwitchTabs = Config.channel.isNightlyOrDebug
+    val browserChromeGestures = Config.channel.isNightlyOrDebug
 
     /**
      * Enables viewing tab history
diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
index 88e151f17..296769258 100644
--- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
@@ -168,7 +168,13 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
         require(arguments != null)
         customTabSessionId = arguments?.getString(EXTRA_SESSION_ID)
 
-        val view = inflater.inflate(R.layout.fragment_browser, container, false)
+        val view = if (FeatureFlags.browserChromeGestures) {
+            inflater.inflate(R.layout.browser_gesture_wrapper, container, false).apply {
+                inflater.inflate(R.layout.fragment_browser, this as SwipeGestureLayout, true)
+            }
+        } else {
+            inflater.inflate(R.layout.fragment_browser, container, false)
+        }
 
         val activity = activity as HomeActivity
         activity.themeManager.applyStatusBarTheme(activity)
diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
index 9c4f33728..1fd155f63 100644
--- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
@@ -15,6 +15,7 @@ import androidx.core.content.ContextCompat
 import androidx.lifecycle.Observer
 import androidx.navigation.fragment.findNavController
 import com.google.android.material.snackbar.Snackbar
+import kotlinx.android.synthetic.main.browser_gesture_wrapper.*
 import kotlinx.android.synthetic.main.fragment_browser.*
 import kotlinx.android.synthetic.main.fragment_browser.view.*
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -74,7 +75,9 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         val components = context.components
 
         return super.initializeUI(view)?.also {
-            if (FeatureFlags.swipeToSwitchTabs) {
+            // We need to wrap this whole thing in an if here because gestureLayout will not exist
+            // if the feature flag is off
+            if (FeatureFlags.browserChromeGestures) {
                 gestureLayout.addGestureListener(
                     ToolbarGestureHandler(
                         activity = requireActivity(),
diff --git a/app/src/main/res/layout/browser_gesture_wrapper.xml b/app/src/main/res/layout/browser_gesture_wrapper.xml
new file mode 100644
index 000000000..94f515687
--- /dev/null
+++ b/app/src/main/res/layout/browser_gesture_wrapper.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<org.mozilla.fenix.browser.SwipeGestureLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:id="@+id/gestureLayout"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent">
+
+    <org.mozilla.fenix.browser.TabPreview
+        android:id="@+id/tabPreview"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:clickable="false"
+        android:focusable="false"
+        android:visibility="gone" />
+</org.mozilla.fenix.browser.SwipeGestureLayout>
diff --git a/app/src/main/res/layout/fragment_browser.xml b/app/src/main/res/layout/fragment_browser.xml
index 63e9fde8d..328f9bedd 100644
--- a/app/src/main/res/layout/fragment_browser.xml
+++ b/app/src/main/res/layout/fragment_browser.xml
@@ -2,66 +2,52 @@
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
-<org.mozilla.fenix.browser.SwipeGestureLayout xmlns:android="http://schemas.android.com/apk/res/android"
+<androidx.coordinatorlayout.widget.CoordinatorLayout 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:id="@+id/gestureLayout"
+    android:id="@+id/browserLayout"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     tools:context="browser.BrowserFragment">
 
-    <androidx.coordinatorlayout.widget.CoordinatorLayout
-        android:id="@+id/browserLayout"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent">
-
-        <androidx.swiperefreshlayout.widget.SwipeRefreshLayout
-            android:id="@+id/swipeRefresh"
-            android:layout_width="match_parent"
-            android:layout_height="match_parent"
-            android:alpha="0"
-            app:layout_behavior="@string/appbar_scrolling_view_behavior">
-
-            <mozilla.components.concept.engine.EngineView
-                android:id="@+id/engineView"
-                android:layout_width="match_parent"
-                android:layout_height="match_parent"
-                android:visibility="gone" />
-        </androidx.swiperefreshlayout.widget.SwipeRefreshLayout>
-
-
-        <ViewStub
-            android:id="@+id/stubFindInPage"
-            android:layout_width="match_parent"
-            android:layout_height="56dp"
-            android:layout_gravity="bottom"
-            android:inflatedId="@+id/findInPageView"
-            android:layout="@layout/stub_find_in_page" />
-
-        <include
-            android:id="@+id/viewDynamicDownloadDialog"
-            layout="@layout/download_dialog_layout"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_gravity="bottom"
-            android:visibility="gone" />
-
-        <mozilla.components.feature.readerview.view.ReaderViewControlsBar
-            android:id="@+id/readerViewControlsBar"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:layout_gravity="bottom"
-            android:background="?foundation"
-            android:elevation="24dp"
-            android:visibility="gone" />
-
-    </androidx.coordinatorlayout.widget.CoordinatorLayout>
-
-    <org.mozilla.fenix.browser.TabPreview
-        android:id="@+id/tabPreview"
+    <androidx.swiperefreshlayout.widget.SwipeRefreshLayout
+        android:id="@+id/swipeRefresh"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
-        android:clickable="false"
-        android:focusable="false"
+        android:alpha="0"
+        app:layout_behavior="@string/appbar_scrolling_view_behavior">
+
+        <mozilla.components.concept.engine.EngineView
+            android:id="@+id/engineView"
+            android:layout_width="match_parent"
+            android:layout_height="match_parent"
+            android:visibility="gone" />
+    </androidx.swiperefreshlayout.widget.SwipeRefreshLayout>
+
+
+    <ViewStub
+        android:id="@+id/stubFindInPage"
+        android:layout_width="match_parent"
+        android:layout_height="56dp"
+        android:layout_gravity="bottom"
+        android:inflatedId="@+id/findInPageView"
+        android:layout="@layout/stub_find_in_page" />
+
+    <include
+        android:id="@+id/viewDynamicDownloadDialog"
+        layout="@layout/download_dialog_layout"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:layout_gravity="bottom"
         android:visibility="gone" />
-</org.mozilla.fenix.browser.SwipeGestureLayout>
+
+    <mozilla.components.feature.readerview.view.ReaderViewControlsBar
+        android:id="@+id/readerViewControlsBar"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:layout_gravity="bottom"
+        android:background="?foundation"
+        android:elevation="24dp"
+        android:visibility="gone" />
+
+</androidx.coordinatorlayout.widget.CoordinatorLayout>