Skip to content

Commit 02811ad

Browse files
0nko0nko
authored
Fix: Tab switcher UI issues (#5736)
Task/Issue URL: https://app.asana.com/0/1207418217763355/1209581022862153 ### Description This PR fixes a couple of minor UI issues in the tab switcher: - [Centrally align all elements in the tab card](https://app.asana.com/0/0/1209573295253128) - [Updates to the New tab page card - Dax logo and title change](https://app.asana.com/0/1207418217763355/1209573295253132) - Tab preview not generated when tabs swiped away - Fixed favicon loading when a NTP becomes a regular web page tab ### Steps to test this PR _Tab switcher item layout_ - [x] Verify the tab layout matches the design [here](https://app.asana.com/0/0/1209573295253128) and [here](https://app.asana.com/0/1207418217763355/1209573295253132) ![image](https://github.com/user-attachments/assets/822724b8-85e3-4de3-aeba-6fe543ad33db) _Tab switcher item layout_ - [x] Open the tab switcher - [x] Verify the screen scrolls to the current tab - [x] Scroll away from the current tab - [x] Switch the tab layout - [x] Verify the screen keeps the previous scroll position (doesn't scroll to current tab) _Tab preview image_ - [x] Make sure you have a few tabs - [x] Swipe between them - [x] Open the tab switcher - [x] Verify they have a preview image _Favicon_ - [x] Clear all tabs - [x] Start with a new tab in the browser - [x] Go to the tab switcher - [x] Verify the dax icon and dax logo are displayed - [x] Go back to the browser - [ ] Enter `cnn.com` and don't wait for it to fully load and open the tab switcher - [ ] Verify the favicon is eventually updated to the correct icon
1 parent 185a725 commit 02811ad

File tree

11 files changed

+173
-67
lines changed

11 files changed

+173
-67
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,7 @@ open class BrowserActivity : DuckDuckGoActivity() {
219219
if (wasSwipingStarted) {
220220
wasSwipingStarted = false
221221

222-
viewModel.onTabsSwiped()
223222
onTabPageSwiped(position)
224-
225223
enableWebViewScrolling()
226224
}
227225
}
@@ -918,10 +916,15 @@ open class BrowserActivity : DuckDuckGoActivity() {
918916
}
919917
}
920918

921-
private fun onTabPageSwiped(newPosition: Int) = lifecycleScope.launch {
922-
val tabId = tabPagerAdapter.getTabIdAtPosition(newPosition)
923-
if (tabId != null) {
924-
tabManager.switchToTab(tabId)
919+
private fun onTabPageSwiped(newPosition: Int) {
920+
currentTab?.onTabSwipedAway()
921+
viewModel.onTabsSwiped()
922+
923+
lifecycleScope.launch {
924+
val tabId = tabPagerAdapter.getTabIdAtPosition(newPosition)
925+
if (tabId != null) {
926+
tabManager.switchToTab(tabId)
927+
}
925928
}
926929
}
927930

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4333,6 +4333,10 @@ class BrowserTabFragment :
43334333
val roomParameters = "?skipMediaPermissionPrompt"
43344334
webView?.loadUrl("${webView?.url.orEmpty()}$roomParameters")
43354335
}
4336+
4337+
fun onTabSwipedAway() {
4338+
viewModel.onTabSwipedAway()
4339+
}
43364340
}
43374341

43384342
private class JsOrientationHandler {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4006,6 +4006,10 @@ class BrowserTabViewModel @Inject constructor(
40064006
}
40074007
}
40084008

4009+
fun onTabSwipedAway() {
4010+
command.value = GenerateWebViewPreviewImage
4011+
}
4012+
40094013
companion object {
40104014
private const val FIXED_PROGRESS = 50
40114015

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import android.graphics.drawable.Drawable
2222
import android.net.Uri
2323
import android.widget.ImageView
2424
import androidx.core.net.toUri
25+
import androidx.lifecycle.findViewTreeLifecycleOwner
26+
import androidx.lifecycle.lifecycleScope
2527
import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.FAVICON_PERSISTED_DIR
2628
import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.FAVICON_TEMP_DIR
2729
import com.duckduckgo.app.browser.favicon.FileBasedFaviconPersister.Companion.NO_SUBFOLDER
@@ -37,8 +39,13 @@ import com.duckduckgo.savedsites.api.SavedSitesRepository
3739
import com.duckduckgo.savedsites.store.SavedSitesEntitiesDao
3840
import com.duckduckgo.sync.api.favicons.FaviconsFetchingStore
3941
import java.io.File
42+
import kotlinx.coroutines.delay
43+
import kotlinx.coroutines.launch
4044
import kotlinx.coroutines.withContext
4145

46+
private const val FAVICON_LOAD_RETRY_DELAY = 2000L
47+
private const val FAVICON_LOAD_RETRIES = 3
48+
4249
class DuckDuckGoFaviconManager constructor(
4350
private val faviconPersister: FaviconPersister,
4451
private val savedSitesDao: SavedSitesEntitiesDao,
@@ -170,8 +177,21 @@ class DuckDuckGoFaviconManager constructor(
170177
}
171178

172179
override suspend fun loadToViewFromLocalWithPlaceholder(tabId: String?, url: String, view: ImageView, placeholder: String?) {
173-
val bitmap = loadFromDisk(tabId, url)
180+
var bitmap = loadFromDisk(tabId, url)
174181
view.loadFavicon(bitmap, url, placeholder)
182+
183+
// sometimes it takes a while to fetch a favicon, so we try to load it from disk again
184+
repeat(FAVICON_LOAD_RETRIES) {
185+
if (bitmap == null) {
186+
view.findViewTreeLifecycleOwner()?.lifecycleScope?.launch {
187+
delay(FAVICON_LOAD_RETRY_DELAY)
188+
bitmap = loadFromDisk(tabId, url)
189+
if (bitmap != null) {
190+
view.loadFavicon(bitmap, url, placeholder)
191+
}
192+
}
193+
}
194+
}
175195
}
176196

177197
override suspend fun persistCachedFavicon(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class TabItemDecorator(
8282
}
8383

8484
companion object {
85-
private val BORDER_RADIUS = 12.toPx().toFloat()
85+
private val BORDER_RADIUS = 10.toPx().toFloat()
8686
private val BORDER_WIDTH = 2.toPx().toFloat()
8787
private val BORDER_PADDING = 3.toPx().toFloat()
8888
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import com.duckduckgo.common.utils.AppUrl
2525

2626
fun TabEntity.displayTitle(context: Context): String {
2727
if (isBlank) {
28-
return context.getString(R.string.homeTab)
28+
return context.getString(R.string.newTabMenuItem)
2929
}
3030

3131
return title ?: Uri.parse(resolvedUrl()).host ?: ""

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

Lines changed: 60 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package com.duckduckgo.app.tabs.ui
1818

1919
import android.annotation.SuppressLint
2020
import android.content.Context
21+
import android.graphics.Bitmap
2122
import android.os.Bundle
2223
import android.view.LayoutInflater
2324
import android.view.View
@@ -33,6 +34,8 @@ import androidx.recyclerview.widget.RecyclerView.Adapter
3334
import androidx.recyclerview.widget.RecyclerView.ViewHolder
3435
import com.bumptech.glide.Glide
3536
import com.bumptech.glide.RequestManager
37+
import com.bumptech.glide.load.Transformation
38+
import com.bumptech.glide.load.engine.Resource
3639
import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions
3740
import com.duckduckgo.app.browser.databinding.ItemTabGridBinding
3841
import com.duckduckgo.app.browser.databinding.ItemTabListBinding
@@ -49,14 +52,18 @@ import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Compa
4952
import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Companion.LIST_TAB
5053
import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.TabViewHolder
5154
import com.duckduckgo.common.ui.view.show
55+
import com.duckduckgo.common.ui.view.toPx
5256
import com.duckduckgo.common.utils.DispatcherProvider
5357
import com.duckduckgo.common.utils.swap
58+
import com.duckduckgo.mobile.android.R as AndroidR
5459
import java.io.File
55-
import kotlin.Int
60+
import java.security.MessageDigest
5661
import kotlinx.coroutines.launch
5762
import kotlinx.coroutines.withContext
5863
import timber.log.Timber
5964

65+
private const val GRID_ITEM_HEIGHT_DP = 170
66+
6067
class TabSwitcherAdapter(
6168
private val itemClickListener: TabSwitcherListener,
6269
private val webViewPreviewPersister: WebViewPreviewPersister,
@@ -132,11 +139,12 @@ class TabSwitcherAdapter(
132139

133140
private fun bindListTab(holder: TabSwitcherViewHolder.ListTabViewHolder, tab: TabEntity) {
134141
val context = holder.binding.root.context
142+
val glide = Glide.with(context)
135143
holder.title.text = extractTabTitle(tab, context)
136144
holder.url.text = tab.url ?: ""
137145
holder.url.visibility = if (tab.url.isNullOrEmpty()) View.GONE else View.VISIBLE
138146
updateUnreadIndicator(holder, tab)
139-
loadFavicon(tab, holder.favicon)
147+
loadFavicon(tab, glide, holder.favicon)
140148
attachTabClickListeners(
141149
tabViewHolder = holder,
142150
bindingAdapterPosition = { holder.bindingAdapterPosition },
@@ -149,8 +157,8 @@ class TabSwitcherAdapter(
149157
val glide = Glide.with(context)
150158
holder.title.text = extractTabTitle(tab, context)
151159
updateUnreadIndicator(holder, tab)
152-
loadFavicon(tab, holder.favicon)
153-
loadTabPreviewImage(tab, glide, holder)
160+
loadFavicon(tab, glide, holder.favicon)
161+
loadTabPreviewImage(tab, glide, holder.tabPreview)
154162
attachTabClickListeners(
155163
tabViewHolder = holder,
156164
bindingAdapterPosition = { holder.bindingAdapterPosition },
@@ -200,7 +208,11 @@ class TabSwitcherAdapter(
200208
}
201209

202210
if (bundle.containsKey(DIFF_KEY_PREVIEW)) {
203-
loadTabPreviewImage(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder)
211+
loadTabPreviewImage(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.tabPreview)
212+
}
213+
214+
bundle.getString(DIFF_KEY_URL)?.let {
215+
loadFavicon(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.favicon)
204216
}
205217

206218
bundle.getString(DIFF_KEY_TITLE)?.let {
@@ -227,6 +239,7 @@ class TabSwitcherAdapter(
227239
bundle.getString(DIFF_KEY_URL)?.let {
228240
viewHolder.url.show()
229241
viewHolder.url.text = it
242+
loadFavicon(tab.tabEntity, Glide.with(viewHolder.rootView), viewHolder.favicon)
230243
}
231244

232245
bundle.getString(DIFF_KEY_TITLE)?.let {
@@ -239,31 +252,56 @@ class TabSwitcherAdapter(
239252
}
240253
}
241254

242-
private fun loadFavicon(tab: TabEntity, view: ImageView) {
243-
val url = tab.url ?: return
244-
lifecycleOwner.lifecycleScope.launch {
245-
faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view)
255+
private fun loadFavicon(tab: TabEntity, glide: RequestManager, view: ImageView) {
256+
val url = tab.url
257+
if (url.isNullOrBlank()) {
258+
glide.clear(view)
259+
glide.load(AndroidR.drawable.ic_dax_icon).into(view)
260+
} else {
261+
lifecycleOwner.lifecycleScope.launch {
262+
faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view)
263+
}
246264
}
247265
}
248266

249-
private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) {
250-
val previewFile = tab.tabPreviewFile ?: return glide.clear(holder.tabPreview)
251-
252-
lifecycleOwner.lifecycleScope.launch {
253-
val cachedWebViewPreview = withContext(dispatchers.io()) {
254-
File(webViewPreviewPersister.fullPathForFile(tab.tabId, previewFile)).takeIf { it.exists() }
267+
private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, tabPreview: ImageView) {
268+
fun fitAndClipBottom() = object : Transformation<Bitmap> {
269+
override fun transform(
270+
context: Context,
271+
resource: Resource<Bitmap>,
272+
outWidth: Int,
273+
outHeight: Int,
274+
): Resource<Bitmap> {
275+
resource.get().height = GRID_ITEM_HEIGHT_DP.toPx()
276+
return resource
255277
}
256278

257-
if (cachedWebViewPreview == null) {
258-
glide.clear(holder.tabPreview)
259-
return@launch
279+
override fun updateDiskCacheKey(messageDigest: MessageDigest) {
260280
}
281+
}
282+
283+
val previewFile = tab.tabPreviewFile
284+
if (tab.url.isNullOrBlank()) {
285+
glide.load(AndroidR.drawable.ic_dax_icon_72)
286+
.into(tabPreview)
287+
} else if (previewFile != null) {
288+
lifecycleOwner.lifecycleScope.launch {
289+
val cachedWebViewPreview = withContext(dispatchers.io()) {
290+
File(webViewPreviewPersister.fullPathForFile(tab.tabId, previewFile)).takeIf { it.exists() }
291+
}
261292

262-
glide.load(cachedWebViewPreview)
263-
.transition(DrawableTransitionOptions.withCrossFade())
264-
.into(holder.tabPreview)
293+
if (cachedWebViewPreview == null) {
294+
glide.clear(tabPreview)
295+
return@launch
296+
}
265297

266-
holder.tabPreview.show()
298+
glide.load(cachedWebViewPreview)
299+
.transition(DrawableTransitionOptions.withCrossFade())
300+
.optionalTransform(fitAndClipBottom())
301+
.into(tabPreview)
302+
}
303+
} else {
304+
glide.clear(tabPreview)
267305
}
268306
}
269307

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
android:layout_width="match_parent"
2020
android:layout_height="wrap_content"
2121
android:layout_margin="@dimen/keyline_2"
22-
app:cardCornerRadius="10dp">
22+
app:cardCornerRadius="@dimen/smallShapeCornerRadius">
2323

2424
<androidx.constraintlayout.widget.ConstraintLayout
2525
android:id="@+id/cardContentsContainer"
@@ -30,12 +30,12 @@
3030
android:id="@+id/favicon"
3131
android:layout_width="@dimen/listItemImageSmallSize"
3232
android:layout_height="@dimen/listItemImageSmallSize"
33-
android:layout_marginStart="10dp"
34-
android:layout_marginTop="10dp"
3533
android:contentDescription="@string/faviconContentDescription"
36-
android:src="@drawable/ic_globe_gray_16dp"
37-
app:layout_constraintStart_toStartOf="parent"
38-
app:layout_constraintTop_toTopOf="parent" />
34+
android:src="@drawable/ic_dax_icon"
35+
android:layout_marginStart="@dimen/keyline_3"
36+
app:layout_constraintTop_toTopOf="@id/close"
37+
app:layout_constraintBottom_toBottomOf="@id/close"
38+
app:layout_constraintStart_toStartOf="parent" />
3939

4040
<ImageView
4141
android:id="@+id/tabUnread"
@@ -56,26 +56,25 @@
5656
android:background="@drawable/selectable_circular_ripple"
5757
android:contentDescription="@string/closeContentDescription"
5858
android:scaleType="center"
59-
android:src="@drawable/ic_close_24_small"
6059
android:padding="@dimen/keyline_2"
61-
app:layout_constraintBottom_toBottomOf="@id/favicon"
60+
android:src="@drawable/ic_close_24_small"
6261
app:layout_constraintEnd_toEndOf="parent"
63-
app:layout_constraintTop_toTopOf="@id/favicon" />
62+
app:layout_constraintTop_toTopOf="parent" />
6463

6564
<com.duckduckgo.common.ui.view.text.DaxTextView
6665
android:id="@+id/title"
6766
android:layout_width="wrap_content"
6867
android:layout_height="wrap_content"
69-
android:layout_marginStart="@dimen/keyline_2"
7068
android:ellipsize="end"
7169
android:lines="1"
7270
android:maxLines="1"
7371
android:textIsSelectable="false"
72+
android:layout_marginStart="@dimen/keyline_2"
7473
app:layout_constrainedWidth="true"
75-
app:layout_constraintBottom_toBottomOf="@id/favicon"
7674
app:layout_constraintEnd_toStartOf="@+id/close"
7775
app:layout_constraintStart_toEndOf="@id/favicon"
78-
app:layout_constraintTop_toTopOf="@id/favicon"
76+
app:layout_constraintTop_toTopOf="@id/close"
77+
app:layout_constraintBottom_toBottomOf="@id/close"
7978
app:layout_constraintHorizontal_bias="0.0"
8079
app:typography="h5"
8180
tools:text="Slashdot" />
@@ -84,13 +83,12 @@
8483
android:id="@+id/tabPreview"
8584
android:layout_width="match_parent"
8685
android:layout_height="@dimen/gridItemPreviewHeight"
87-
android:layout_marginTop="@dimen/keyline_3"
8886
android:importantForAccessibility="no"
89-
android:scaleType="matrix"
87+
android:scaleType="center"
9088
app:layout_constraintBottom_toBottomOf="parent"
9189
app:layout_constraintEnd_toEndOf="parent"
9290
app:layout_constraintStart_toStartOf="parent"
93-
app:layout_constraintTop_toBottomOf="@id/title" />
91+
app:layout_constraintTop_toBottomOf="@id/close" />
9492

9593
</androidx.constraintlayout.widget.ConstraintLayout>
9694

0 commit comments

Comments
 (0)