For #14529 - Show a dialog when the top sites limit has been reached

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -293,7 +293,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                 openInFenixIntent = openInFenixIntent,
                 bookmarkTapped = { viewLifecycleOwner.lifecycleScope.launch { bookmarkTapped(it) } },
                 scope = viewLifecycleOwner.lifecycleScope,
-                tabCollectionStorage = requireComponents.core.tabCollectionStorage
+                tabCollectionStorage = requireComponents.core.tabCollectionStorage,
+                topSitesStorage = requireComponents.core.topSitesStorage
             )
 
             _browserInteractor = BrowserInteractor(

app/src/main/java/org/mozilla/fenix/components/Components.kt

@@ -54,7 +54,7 @@ class Components(private val context: Context) {
             core.store,
             search.searchEngineManager,
             core.webAppShortcutManager,
-            core.topSiteStorage
+            core.topSitesStorage
         )
     }
     val intentProcessors by lazy {

app/src/main/java/org/mozilla/fenix/components/Core.kt

@@ -265,7 +265,7 @@ class Core(private val context: Context, private val crashReporter: CrashReporti
 
     val pinnedSiteStorage by lazy { PinnedSiteStorage(context) }
 
-    val topSiteStorage by lazy {
+    val topSitesStorage by lazy {
         val defaultTopSites = mutableListOf<Pair<String, String>>()
 
         StrictMode.allowThreadDiskReads().resetPoliciesAfter {

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt

@@ -6,6 +6,7 @@ package org.mozilla.fenix.components.toolbar
 
 import android.content.Intent
 import androidx.annotation.VisibleForTesting
+import androidx.appcompat.app.AlertDialog
 import androidx.navigation.NavController
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
 import com.google.android.material.snackbar.Snackbar
@@ -19,6 +20,8 @@ import mozilla.components.browser.session.SessionManager
 import mozilla.components.concept.engine.EngineSession.LoadUrlFlags
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.feature.session.SessionFeature
+import mozilla.components.feature.top.sites.DefaultTopSitesStorage
+import mozilla.components.feature.top.sites.TopSite
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.NavGraphDirections
@@ -62,7 +65,8 @@ class DefaultBrowserToolbarMenuController(
     private val openInFenixIntent: Intent,
     private val bookmarkTapped: (Session) -> Unit,
     private val scope: CoroutineScope,
-    private val tabCollectionStorage: TabCollectionStorage
+    private val tabCollectionStorage: TabCollectionStorage,
+    private val topSitesStorage: DefaultTopSitesStorage
 ) : BrowserToolbarMenuController {
 
     private val currentSession
@@ -124,6 +128,20 @@ class DefaultBrowserToolbarMenuController(
             )
             ToolbarMenu.Item.AddToTopSites -> {
                 scope.launch {
+                    val context = swipeRefresh.context
+                    val numPinnedSites =
+                        topSitesStorage.cachedTopSites.filter { it.type != TopSite.Type.FRECENT }.size
+
+                    if (numPinnedSites >= settings.topSitesMaxLimit) {
+                        AlertDialog.Builder(swipeRefresh.context).apply {
+                            setTitle(R.string.top_sites_max_limit_title)
+                            setMessage(R.string.top_sites_max_limit_content_2)
+                            setPositiveButton(R.string.top_sites_max_limit_confirmation_button) { dialog, _ ->
+                                dialog.dismiss()
+                            }
+                            create()
+                        }.show()
+                    } else {
                         ioScope.launch {
                             currentSession?.let {
                                 with(activity.components.useCases.topSitesUseCase) {
@@ -138,11 +156,12 @@ class DefaultBrowserToolbarMenuController(
                             isDisplayedWithBrowserToolbar = true
                         )
                             .setText(
-                            swipeRefresh.context.getString(R.string.snackbar_added_to_top_sites)
+                                context.getString(R.string.snackbar_added_to_top_sites)
                             )
                             .show()
                     }
                 }
+            }
             ToolbarMenu.Item.AddToHomeScreen, ToolbarMenu.Item.InstallToHomeScreen -> {
                 settings.installPwaOpened = true
                 MainScope().launch {

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -199,7 +199,7 @@ class HomeFragment : Fragment() {
                     collections = components.core.tabCollectionStorage.cachedTabCollections,
                     expandedCollections = emptySet(),
                     mode = currentMode.getCurrentMode(),
-                    topSites = components.core.topSiteStorage.cachedTopSites,
+                    topSites = components.core.topSitesStorage.cachedTopSites,
                     tip = StrictMode.allowThreadDiskReads().resetPoliciesAfter {
                         FenixTipManager(
                             listOf(
@@ -219,7 +219,7 @@ class HomeFragment : Fragment() {
         topSitesFeature.set(
             feature = TopSitesFeature(
                 view = DefaultTopSitesView(homeFragmentStore),
-                storage = components.core.topSiteStorage,
+                storage = components.core.topSitesStorage,
                 config = ::getTopSitesConfig
             ),
             owner = this,
@@ -552,7 +552,7 @@ class HomeFragment : Fragment() {
             HomeFragmentAction.Change(
                 collections = components.core.tabCollectionStorage.cachedTabCollections,
                 mode = currentMode.getCurrentMode(),
-                topSites = components.core.topSiteStorage.cachedTopSites,
+                topSites = components.core.topSitesStorage.cachedTopSites,
                 tip = StrictMode.allowThreadDiskReads().resetPoliciesAfter {
                     FenixTipManager(
                         listOf(

app/src/main/res/values/strings.xml

@@ -1534,7 +1534,7 @@
     <!-- Title text displayed in the dialog when top sites limit is reached. -->
     <string name="top_sites_max_limit_title">Top site limit reached</string>
     <!-- Content description text displayed in the dialog when top sites limit is reached. -->
-    <string name="top_sites_max_limit_content">To add a new top site, remove one. Long press the site and select remove.</string>
+    <string name="top_sites_max_limit_content_2">To add a new top site, remove one. Touch and hold the site and select remove.</string>
     <!-- Confirmation dialog button text when top sites limit is reached. -->
     <string name="top_sites_max_limit_confirmation_button">OK, Got It</string>
     <!-- Label for the show most visited sites preference -->

app/src/test/java/org/mozilla/fenix/components/TestComponents.kt

@@ -24,7 +24,7 @@ class TestComponents(private val context: Context) : Components(context) {
             core.store,
             search.searchEngineManager,
             core.webAppShortcutManager,
-            core.topSiteStorage
+            core.topSitesStorage
         )
     }
     override val intentProcessors by lazy { mockk<IntentProcessors>(relaxed = true) }

app/src/test/java/org/mozilla/fenix/components/TestCore.kt

@@ -27,5 +27,5 @@ class TestCore(context: Context, crashReporter: CrashReporting) : Core(context,
     override val client = mockk<Client>()
     override val webAppShortcutManager = mockk<WebAppShortcutManager>()
     override val thumbnailStorage = mockk<ThumbnailStorage>()
-    override val topSiteStorage = mockk<DefaultTopSitesStorage>()
+    override val topSitesStorage = mockk<DefaultTopSitesStorage>()
 }

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt

@@ -32,6 +32,7 @@ import mozilla.components.feature.search.SearchUseCases
 import mozilla.components.feature.session.SessionFeature
 import mozilla.components.feature.session.SessionUseCases
 import mozilla.components.feature.tab.collections.TabCollection
+import mozilla.components.feature.top.sites.DefaultTopSitesStorage
 import mozilla.components.feature.top.sites.TopSitesUseCases
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.test.rule.MainCoroutineRule
@@ -83,6 +84,7 @@ class DefaultBrowserToolbarMenuControllerTest {
     @RelaxedMockK private lateinit var readerModeController: ReaderModeController
     @MockK private lateinit var sessionFeatureWrapper: ViewBoundFeatureWrapper<SessionFeature>
     @RelaxedMockK private lateinit var sessionFeature: SessionFeature
+    @RelaxedMockK private lateinit var topSitesStorage: DefaultTopSitesStorage
 
     @Before
     fun setUp() {
@@ -105,6 +107,7 @@ class DefaultBrowserToolbarMenuControllerTest {
             every { id } returns R.id.browserFragment
         }
         every { currentSession.id } returns "1"
+        every { settings.topSitesMaxLimit } returns 16
 
         val onComplete = slot<() -> Unit>()
         every { browserAnimator.captureEngineViewAndDrawStatically(capture(onComplete)) } answers { onComplete.captured.invoke() }
@@ -487,7 +490,8 @@ class DefaultBrowserToolbarMenuControllerTest {
         bookmarkTapped = bookmarkTapped,
         readerModeController = readerModeController,
         sessionManager = sessionManager,
-        sessionFeature = sessionFeatureWrapper
+        sessionFeature = sessionFeatureWrapper,
+        topSitesStorage = topSitesStorage
     ).apply {
         ioScope = scope
     }