Fix: Tab switcher UI issues

app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt

@@ -219,9 +219,7 @@ open class BrowserActivity : DuckDuckGoActivity() {
             if (wasSwipingStarted) {
                 wasSwipingStarted = false
 
-                viewModel.onTabsSwiped()
                 onTabPageSwiped(position)
-
                 enableWebViewScrolling()
             }
         }
@@ -918,10 +916,15 @@ open class BrowserActivity : DuckDuckGoActivity() {
         }
     }
 
-    private fun onTabPageSwiped(newPosition: Int) = lifecycleScope.launch {
-        val tabId = tabPagerAdapter.getTabIdAtPosition(newPosition)
-        if (tabId != null) {
-            tabManager.switchToTab(tabId)
+    private fun onTabPageSwiped(newPosition: Int) {
+        currentTab?.onTabSwipedAway()
+        viewModel.onTabsSwiped()
+
+        lifecycleScope.launch {
+            val tabId = tabPagerAdapter.getTabIdAtPosition(newPosition)
+            if (tabId != null) {
+                tabManager.switchToTab(tabId)
+            }
         }
     }
 

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

@@ -4333,6 +4333,10 @@ class BrowserTabFragment :
         val roomParameters = "?skipMediaPermissionPrompt"
         webView?.loadUrl("${webView?.url.orEmpty()}$roomParameters")
     }
+
+    fun onTabSwipedAway() {
+        viewModel.onTabSwipedAway()
+    }
 }
 
 private class JsOrientationHandler {

app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

@@ -4006,6 +4006,10 @@ class BrowserTabViewModel @Inject constructor(
         }
     }
 
+    fun onTabSwipedAway() {
+        command.value = GenerateWebViewPreviewImage
+    }
+
     companion object {
         private const val FIXED_PROGRESS = 50
 

app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt

@@ -22,6 +22,8 @@ import android.graphics.drawable.Drawable
 import android.net.Uri
 import android.widget.ImageView
 import androidx.core.net.toUri
+import androidx.lifecycle.findViewTreeLifecycleOwner
+import androidx.lifecycle.lifecycleScope
 import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.FAVICON_PERSISTED_DIR
 import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.FAVICON_TEMP_DIR
 import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.NO_SUBFOLDER
@@ -37,8 +39,13 @@ import com.duckduckgo.savedsites.api.SavedSitesRepository
 import com.duckduckgo.savedsites.store.SavedSitesEntitiesDao
 import com.duckduckgo.sync.api.favicons.FaviconsFetchingStore
 import java.io.File
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 
+private const val FAVICON_LOAD_RETRY_DELAY = 2000L
+private const val FAVICON_LOAD_RETRIES = 3
+
 class DuckDuckGoFaviconManager constructor(
     private val faviconPersister: FaviconPersister,
     private val savedSitesDao: SavedSitesEntitiesDao,
@@ -170,8 +177,21 @@ class DuckDuckGoFaviconManager constructor(
     }
 
     override suspend fun loadToViewFromLocalWithPlaceholder(tabId: String?, url: String, view: ImageView, placeholder: String?) {
-        val bitmap = loadFromDisk(tabId, url)
+        var bitmap = loadFromDisk(tabId, url)
         view.loadFavicon(bitmap, url, placeholder)
+
+        // sometimes it takes a while to fetch a favicon, so we try to load it from disk again
+        repeat(FAVICON_LOAD_RETRIES) {
+            if (bitmap == null) {
+                view.findViewTreeLifecycleOwner()?.lifecycleScope?.launch {
+                    delay(FAVICON_LOAD_RETRY_DELAY)
+                    bitmap = loadFromDisk(tabId, url)
+                    if (bitmap != null) {
+                        view.loadFavicon(bitmap, url, placeholder)
+                    }
+                }
+            }
+        }
     }
 
     override suspend fun persistCachedFavicon(

app/src/main/java/com/duckduckgo/app/tabs/ui/TabItemDecorator.kt

@@ -82,7 +82,7 @@ class TabItemDecorator(
     }
 
     companion object {
-        private val BORDER_RADIUS = 12.toPx().toFloat()
+        private val BORDER_RADIUS = 10.toPx().toFloat()
         private val BORDER_WIDTH = 2.toPx().toFloat()
         private val BORDER_PADDING = 3.toPx().toFloat()
     }

app/src/main/java/com/duckduckgo/app/tabs/ui/TabRendererExtension.kt

@@ -25,7 +25,7 @@ import com.duckduckgo.common.utils.AppUrl
 
 fun TabEntity.displayTitle(context: Context): String {
     if (isBlank) {
-        return context.getString(R.string.homeTab)
+        return context.getString(R.string.newTabMenuItem)
     }
 
     return title ?: Uri.parse(resolvedUrl()).host ?: ""

app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt

@@ -18,6 +18,7 @@ package com.duckduckgo.app.tabs.ui
 
 import android.annotation.SuppressLint
 import android.content.Context
+import android.graphics.Bitmap
 import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
@@ -33,6 +34,8 @@ import androidx.recyclerview.widget.RecyclerView.Adapter
 import androidx.recyclerview.widget.RecyclerView.ViewHolder
 import com.bumptech.glide.Glide
 import com.bumptech.glide.RequestManager
+import com.bumptech.glide.load.Transformation
+import com.bumptech.glide.load.engine.Resource
 import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions
 import com.duckduckgo.app.browser.databinding.ItemTabGridBinding
 import com.duckduckgo.app.browser.databinding.ItemTabListBinding
@@ -49,14 +52,18 @@ import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Compa
 import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Companion.LIST_TAB
 import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.TabViewHolder
 import com.duckduckgo.common.ui.view.show
+import com.duckduckgo.common.ui.view.toPx
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.common.utils.swap
+import com.duckduckgo.mobile.android.R as AndroidR
 import java.io.File
-import kotlin.Int
+import java.security.MessageDigest
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 
+private const val GRID_ITEM_HEIGHT_DP = 170
+
 class TabSwitcherAdapter(
     private val itemClickListener: TabSwitcherListener,
     private val webViewPreviewPersister: WebViewPreviewPersister,
@@ -132,11 +139,12 @@ class TabSwitcherAdapter(
 
     private fun bindListTab(holder: TabSwitcherViewHolder.ListTabViewHolder, tab: TabEntity) {
         val context = holder.binding.root.context
+        val glide = Glide.with(context)
         holder.title.text = extractTabTitle(tab, context)
         holder.url.text = tab.url ?: ""
         holder.url.visibility = if (tab.url.isNullOrEmpty()) View.GONE else View.VISIBLE
         updateUnreadIndicator(holder, tab)
-        loadFavicon(tab, holder.favicon)
+        loadFavicon(tab, glide, holder.favicon)
         attachTabClickListeners(
             tabViewHolder = holder,
             bindingAdapterPosition = { holder.bindingAdapterPosition },
@@ -149,8 +157,8 @@ class TabSwitcherAdapter(
         val glide = Glide.with(context)
         holder.title.text = extractTabTitle(tab, context)
         updateUnreadIndicator(holder, tab)
-        loadFavicon(tab, holder.favicon)
-        loadTabPreviewImage(tab, glide, holder)
+        loadFavicon(tab, glide, holder.favicon)
+        loadTabPreviewImage(tab, glide, holder.tabPreview)
         attachTabClickListeners(
             tabViewHolder = holder,
             bindingAdapterPosition = { holder.bindingAdapterPosition },
@@ -200,7 +208,11 @@ class TabSwitcherAdapter(
             }
 
             if (bundle.containsKey(DIFF_KEY_PREVIEW)) {
-                loadTabPreviewImage(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder)
+                loadTabPreviewImage(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.tabPreview)
+            }
+
+            bundle.getString(DIFF_KEY_URL)?.let {
+                loadFavicon(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.favicon)
             }
 
             bundle.getString(DIFF_KEY_TITLE)?.let {
@@ -227,6 +239,7 @@ class TabSwitcherAdapter(
             bundle.getString(DIFF_KEY_URL)?.let {
                 viewHolder.url.show()
                 viewHolder.url.text = it
+                loadFavicon(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.favicon)
             }
 
             bundle.getString(DIFF_KEY_TITLE)?.let {
@@ -239,31 +252,56 @@ class TabSwitcherAdapter(
         }
     }
 
-    private fun loadFavicon(tab: TabEntity, view: ImageView) {
-        val url = tab.url ?: return
-        lifecycleOwner.lifecycleScope.launch {
-            faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view)
+    private fun loadFavicon(tab: TabEntity, glide: RequestManager, view: ImageView) {
+        val url = tab.url
+        if (url.isNullOrBlank()) {
+            glide.clear(view)
+            glide.load(AndroidR.drawable.ic_dax_icon).into(view)
+        } else {
+            lifecycleOwner.lifecycleScope.launch {
+                faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view)
+            }
         }
     }
 
-    private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) {
-        val previewFile = tab.tabPreviewFile ?: return glide.clear(holder.tabPreview)
-
-        lifecycleOwner.lifecycleScope.launch {
-            val cachedWebViewPreview = withContext(dispatchers.io()) {
-                File(webViewPreviewPersister.fullPathForFile(tab.tabId, previewFile)).takeIf { it.exists() }
+    private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, tabPreview: ImageView) {
+        fun fitAndClipBottom() = object : Transformation<Bitmap> {
+            override fun transform(
+                context: Context,
+                resource: Resource<Bitmap>,
+                outWidth: Int,
+                outHeight: Int,
+            ): Resource<Bitmap> {
+                resource.get().height = GRID_ITEM_HEIGHT_DP.toPx()
+                return resource
             }
 
-            if (cachedWebViewPreview == null) {
-                glide.clear(holder.tabPreview)
-                return@launch
+            override fun updateDiskCacheKey(messageDigest: MessageDigest) {
             }
+        }
+
+        val previewFile = tab.tabPreviewFile
+        if (tab.url.isNullOrBlank()) {
+            glide.load(AndroidR.drawable.ic_dax_icon_72)
+                .into(tabPreview)
+        } else if (previewFile != null) {
+            lifecycleOwner.lifecycleScope.launch {
+                val cachedWebViewPreview = withContext(dispatchers.io()) {
+                    File(webViewPreviewPersister.fullPathForFile(tab.tabId, previewFile)).takeIf { it.exists() }
+                }
 
-            glide.load(cachedWebViewPreview)
-                .transition(DrawableTransitionOptions.withCrossFade())
-                .into(holder.tabPreview)
+                if (cachedWebViewPreview == null) {
+                    glide.clear(tabPreview)
+                    return@launch
+                }
 
-            holder.tabPreview.show()
+                glide.load(cachedWebViewPreview)
+                    .transition(DrawableTransitionOptions.withCrossFade())
+                    .optionalTransform(fitAndClipBottom())
+                    .into(tabPreview)
+            }
+        } else {
+            glide.clear(tabPreview)
         }
     }
 

app/src/main/res/layout/item_tab_grid.xml

@@ -19,7 +19,7 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
     android:layout_margin="@dimen/keyline_2"
-    app:cardCornerRadius="10dp">
+    app:cardCornerRadius="@dimen/smallShapeCornerRadius">
 
     <androidx.constraintlayout.widget.ConstraintLayout
         android:id="@+id/cardContentsContainer"
@@ -30,12 +30,12 @@
             android:id="@+id/favicon"
             android:layout_width="@dimen/listItemImageSmallSize"
             android:layout_height="@dimen/listItemImageSmallSize"
-            android:layout_marginStart="10dp"
-            android:layout_marginTop="10dp"
             android:contentDescription="@string/faviconContentDescription"
-            android:src="@drawable/ic_globe_gray_16dp"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toTopOf="parent" />
+            android:src="@drawable/ic_dax_icon"
+            android:layout_marginStart="@dimen/keyline_3"
+            app:layout_constraintTop_toTopOf="@id/close"
+            app:layout_constraintBottom_toBottomOf="@id/close"
+            app:layout_constraintStart_toStartOf="parent" />
 
         <ImageView
             android:id="@+id/tabUnread"
@@ -56,26 +56,25 @@
             android:background="@drawable/selectable_circular_ripple"
             android:contentDescription="@string/closeContentDescription"
             android:scaleType="center"
-            android:src="@drawable/ic_close_24_small"
             android:padding="@dimen/keyline_2"
-            app:layout_constraintBottom_toBottomOf="@id/favicon"
+            android:src="@drawable/ic_close_24_small"
             app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintTop_toTopOf="@id/favicon" />
+            app:layout_constraintTop_toTopOf="parent" />
 
         <com.duckduckgo.common.ui.view.text.DaxTextView
             android:id="@+id/title"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
-            android:layout_marginStart="@dimen/keyline_2"
             android:ellipsize="end"
             android:lines="1"
             android:maxLines="1"
             android:textIsSelectable="false"
+            android:layout_marginStart="@dimen/keyline_2"
             app:layout_constrainedWidth="true"
-            app:layout_constraintBottom_toBottomOf="@id/favicon"
             app:layout_constraintEnd_toStartOf="@+id/close"
             app:layout_constraintStart_toEndOf="@id/favicon"
-            app:layout_constraintTop_toTopOf="@id/favicon"
+            app:layout_constraintTop_toTopOf="@id/close"
+            app:layout_constraintBottom_toBottomOf="@id/close"
             app:layout_constraintHorizontal_bias="0.0"
             app:typography="h5"
             tools:text="Slashdot" />
@@ -84,13 +83,12 @@
             android:id="@+id/tabPreview"
             android:layout_width="match_parent"
             android:layout_height="@dimen/gridItemPreviewHeight"
-            android:layout_marginTop="@dimen/keyline_3"
             android:importantForAccessibility="no"
-            android:scaleType="matrix"
+            android:scaleType="center"
             app:layout_constraintBottom_toBottomOf="parent"
             app:layout_constraintEnd_toEndOf="parent"
             app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toBottomOf="@id/title" />
+            app:layout_constraintTop_toBottomOf="@id/close" />
 
     </androidx.constraintlayout.widget.ConstraintLayout>