Issue mozilla-mobile#879: Address review comments

pocmo · Sep 1, 2018 · ec097ab · ec097ab
1 parent bb3909d
commit ec097ab
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 10 deletions.
diff --git a/app/src/main/java/org/mozilla/focus/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/focus/browser/BrowserFragment.kt
@@ -126,11 +126,11 @@ class BrowserFragment : IWebViewLifecycleFragment(), Session.Observer {
             NavigationEvent.SETTINGS -> ScreenController.showSettingsScreen(fragmentManager!!)
             NavigationEvent.LOAD_URL -> {
                 (activity as MainActivity).onTextInputUrlEntered(value!!, autocompleteResult!!, UrlTextInputLocation.MENU)
-                setOverlayVisibleByUser(false)
+                setOverlayVisible(false)
             }
             NavigationEvent.LOAD_TILE -> {
                 (activity as MainActivity).onNonTextInputUrlEntered(value!!)
-                setOverlayVisibleByUser(false)
+                setOverlayVisible(false)
             }
             NavigationEvent.POCKET -> ScreenController.showPocketScreen(fragmentManager!!)
             NavigationEvent.PIN_ACTION -> {
@@ -234,7 +234,7 @@ class BrowserFragment : IWebViewLifecycleFragment(), Session.Observer {
     fun onBackPressed(): Boolean {
         when {
             browserOverlay.isVisible && !isUrlEqualToHomepage -> {
-                setOverlayVisibleByUser(false)
+                setOverlayVisible(false)
                 TelemetryWrapper.userShowsHidesDrawerEvent(false)
             }
             webView?.canGoBack() ?: false -> {
@@ -277,7 +277,7 @@ class BrowserFragment : IWebViewLifecycleFragment(), Session.Observer {
         if (event.keyCode == KeyEvent.KEYCODE_MENU && !isUrlEqualToHomepage) {
             if (actionIsDown) {
                 val toShow = !browserOverlay.isVisible
-                setOverlayVisibleByUser(toShow)
+                setOverlayVisible(toShow)
                 TelemetryWrapper.userShowsHidesDrawerEvent(toShow)
             }
             return true
@@ -295,11 +295,8 @@ class BrowserFragment : IWebViewLifecycleFragment(), Session.Observer {
     /**
      * Changes the overlay visibility: this should be called instead of changing
      * [BrowserNavigationOverlay.isVisible] directly.
-     *
-     * It's important this is only called for user actions because our Telemetry
-     * is dependent on it.
      */
-    private fun setOverlayVisibleByUser(toShow: Boolean) {
+    private fun setOverlayVisible(toShow: Boolean) {
         browserOverlay.visibility = if (toShow) View.VISIBLE else View.GONE
         if (toShow) cursor?.onPause() else cursor?.onResume()
         cursor?.setEnabledForCurrentState()

diff --git a/app/src/main/java/org/mozilla/focus/telemetry/MenuInteractionMonitor.kt b/app/src/main/java/org/mozilla/focus/telemetry/MenuInteractionMonitor.kt
@@ -9,8 +9,9 @@ package org.mozilla.focus.telemetry
  * understanding the menu overlay.
  *
  * If a user opens a menu and then closes it without performing any action
- * (represented here by clicking), it is assumed that they were unable to find
- * what they were looking for. This is used as a heuristic for confusion.
+ * (represented here by clicking select), it is assumed that they were unable
+ * to find what they were looking for. This is used as a heuristic for
+ * confusion.
  */
 object MenuInteractionMonitor {
     private var selectPressedDuringCurrentMenu = true