For mozilla-mobile#9692 - Fix "Install" PWA menu item labeling

sblatz · Apr 13, 2020 · 8c23840 · 8c23840
1 parent 85eef1f
commit 8c23840
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 16 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt
@@ -40,8 +40,8 @@ import org.mozilla.fenix.ext.getRootView
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.home.SharedViewModel
-import org.mozilla.fenix.utils.Do
 import org.mozilla.fenix.settings.deletebrowsingdata.deleteAndQuit
+import org.mozilla.fenix.utils.Do
 
 /**
  * An interface that handles the view manipulation of the BrowserToolbar, triggered by the Interactor
@@ -179,7 +179,7 @@ class DefaultBrowserToolbarController(
                     }
                 }
             }
-            ToolbarMenu.Item.AddToHomeScreen -> {
+            ToolbarMenu.Item.AddToHomeScreen, ToolbarMenu.Item.InstallToHomeScreen -> {
                 activity.settings().installPwaOpened = true
                 MainScope().launch {
                     with(activity.components.useCases.webAppUseCases) {
@@ -349,6 +349,7 @@ class DefaultBrowserToolbarController(
             ToolbarMenu.Item.SaveToCollection -> Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION
             ToolbarMenu.Item.AddToTopSites -> Event.BrowserMenuItemTapped.Item.ADD_TO_TOP_SITES
             ToolbarMenu.Item.AddToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN
+            ToolbarMenu.Item.InstallToHomeScreen -> Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN
             ToolbarMenu.Item.Quit -> Event.BrowserMenuItemTapped.Item.QUIT
             is ToolbarMenu.Item.ReaderMode ->
                 if (item.isChecked) {

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt
@@ -137,8 +137,8 @@ class DefaultToolbarMenu(
 
     internal fun getLowPrioHighlightItems(): List<ToolbarMenu.Item> {
         val lowPrioHighlightItems: MutableList<ToolbarMenu.Item> = mutableListOf()
-        if (shouldShowAddToHomescreen() && addToHomescreen.isHighlighted()) {
-            lowPrioHighlightItems.add(ToolbarMenu.Item.AddToHomeScreen)
+        if (canInstall() && installToHomescreen.isHighlighted()) {
+            lowPrioHighlightItems.add(ToolbarMenu.Item.InstallToHomeScreen)
         }
         if (shouldShowReaderMode() && readerMode.isHighlighted()) {
             lowPrioHighlightItems.add(ToolbarMenu.Item.ReaderMode(false))
@@ -150,8 +150,13 @@ class DefaultToolbarMenu(
     }
 
     // Predicates that need to be repeatedly called as the session changes
-    private fun shouldShowAddToHomescreen(): Boolean =
-        session != null && context.components.useCases.webAppUseCases.isPinningSupported()
+    private fun canAddToHomescreen(): Boolean =
+        session != null && context.components.useCases.webAppUseCases.isPinningSupported() &&
+                !context.components.useCases.webAppUseCases.isInstallable()
+
+    private fun canInstall(): Boolean =
+        session != null && context.components.useCases.webAppUseCases.isPinningSupported() &&
+                context.components.useCases.webAppUseCases.isInstallable()
 
     private fun shouldShowReaderMode(): Boolean = session?.let {
         store.state.findTab(it.id)?.readerState?.readerable
@@ -187,7 +192,8 @@ class DefaultToolbarMenu(
             if (shouldShowWebcompatReporter) reportIssue else null,
             findInPage,
             addToTopSites,
-            addToHomescreen.apply { visible = ::shouldShowAddToHomescreen },
+            addToHomescreen.apply { visible = ::canAddToHomescreen },
+            installToHomescreen.apply { visible = ::canInstall },
             if (shouldShowSaveToCollection) saveToCollection else null,
             desktopMode,
             openInApp.apply { visible = ::shouldShowOpenInApp },
@@ -253,22 +259,27 @@ class DefaultToolbarMenu(
         onItemTapped.invoke(ToolbarMenu.Item.AddToTopSites)
     }
 
-    private val addToHomescreen = BrowserMenuHighlightableItem(
+    private val addToHomescreen = BrowserMenuImageText(
         label = context.getString(R.string.browser_menu_add_to_homescreen),
+        imageResource = R.drawable.ic_add_to_homescreen,
+        iconTintColorResource = primaryTextColor()
+    ) {
+        onItemTapped.invoke(ToolbarMenu.Item.AddToHomeScreen)
+    }
+
+    private val installToHomescreen = BrowserMenuHighlightableItem(
+        label = context.getString(R.string.browser_menu_install_on_homescreen),
         startImageResource = R.drawable.ic_add_to_homescreen,
         iconTintColorResource = primaryTextColor(),
         highlight = BrowserMenuHighlight.LowPriority(
             label = context.getString(R.string.browser_menu_install_on_homescreen),
             notificationTint = getColor(context, R.color.whats_new_notification_color)
         ),
         isHighlighted = {
-            val webAppUseCases = context.components.useCases.webAppUseCases
-            webAppUseCases.isPinningSupported() &&
-                    webAppUseCases.isInstallable() &&
-                    !context.settings().installPwaOpened
+            !context.settings().installPwaOpened
         }
     ) {
-        onItemTapped.invoke(ToolbarMenu.Item.AddToHomeScreen)
+        onItemTapped.invoke(ToolbarMenu.Item.InstallToHomeScreen)
     }
 
     private val findInPage = BrowserMenuImageText(

diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarMenu.kt
@@ -22,6 +22,7 @@ interface ToolbarMenu {
         object OpenInFenix : Item()
         object SaveToCollection : Item()
         object AddToTopSites : Item()
+        object InstallToHomeScreen : Item()
         object AddToHomeScreen : Item()
         object AddonsManager : Item()
         object Quit : Item()

diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenuTest.kt
@@ -75,7 +75,7 @@ class DefaultToolbarMenuTest {
 
         val list = defaultToolbarMenu.getLowPrioHighlightItems()
 
-        assertEquals(ToolbarMenu.Item.AddToHomeScreen, list[0])
+        assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0])
         assertEquals(ToolbarMenu.Item.ReaderMode(false), list[1])
         assertEquals(ToolbarMenu.Item.OpenInApp, list[2])
     }
@@ -115,7 +115,7 @@ class DefaultToolbarMenuTest {
 
         val list = defaultToolbarMenu.getLowPrioHighlightItems()
 
-        assertEquals(ToolbarMenu.Item.AddToHomeScreen, list[0])
+        assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0])
         assertEquals(ToolbarMenu.Item.OpenInApp, list[1])
     }
 
@@ -134,7 +134,7 @@ class DefaultToolbarMenuTest {
 
         val list = defaultToolbarMenu.getLowPrioHighlightItems()
 
-        assertEquals(ToolbarMenu.Item.AddToHomeScreen, list[0])
+        assertEquals(ToolbarMenu.Item.InstallToHomeScreen, list[0])
         assertEquals(ToolbarMenu.Item.ReaderMode(false), list[1])
     }
 }