From 10ceb6145d5ff0e6fccc3e1f3e103eae3fe544fb Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 11:15:13 +0100 Subject: [PATCH 01/20] Fix the scroll to current tab --- .../duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index b967db7b2621..18906168addf 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -195,6 +195,8 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine if (noTabSelected && tabSwitcherItems.isNotEmpty()) { updateTabGridItemDecorator(tabSwitcherItems.last().id) } + + scrollToCurrentTab() } viewModel.activeTab.observe(this) { tab -> if (tab != null && tab.tabId != tabItemDecorator.tabSwitcherItemId && !tab.deletable) { @@ -242,17 +244,18 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.onLayoutTypeChanged(layoutType) tabTouchHelper.onLayoutTypeChanged(layoutType) - if (firstTimeLoadingTabsList) { - firstTimeLoadingTabsList = false - - scrollToShowCurrentTab() - } else { + if (!firstTimeLoadingTabsList) { scrollToPreviousCenterOffset(centerOffsetPercent) } tabsRecycler.show() } + private fun scrollToCurrentTab() { + firstTimeLoadingTabsList = false + scrollToPositionOfCurrentTab() + } + private fun scrollToPreviousCenterOffset(centerOffsetPercent: Float) { tabsRecycler.post { val newRange = tabsRecycler.computeVerticalScrollRange() @@ -290,7 +293,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.updateData(tabs) } - private fun scrollToShowCurrentTab() { + private fun scrollToPositionOfCurrentTab() { val index = tabsAdapter.adapterPositionForTab(selectedTabId) if (index != -1) { scrollToPosition(index) From f5c128644bfb5396fef0fc5a0cef38aa9c4004a1 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 11:22:49 +0100 Subject: [PATCH 02/20] Fix tab favicon and preview for new tabs --- .../app/tabs/ui/TabSwitcherAdapter.kt | 19 ++++++++++++++++--- app/src/main/res/layout/item_tab_grid.xml | 14 +++++++++++++- app/src/main/res/layout/item_tab_list.xml | 2 +- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index cb262e566aae..a43a6181d015 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -48,6 +48,7 @@ import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Companion.GRID_TAB 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.hide import com.duckduckgo.common.ui.view.show import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.swap @@ -240,13 +241,24 @@ class TabSwitcherAdapter( } private fun loadFavicon(tab: TabEntity, view: ImageView) { - val url = tab.url ?: return - lifecycleOwner.lifecycleScope.launch { - faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view) + if (tab.url == null) { + Glide.with(view).load(com.duckduckgo.mobile.android.R.drawable.ic_dax_icon).into(view) + } else { + lifecycleOwner.lifecycleScope.launch { + faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, tab.url!!, view) + } } } private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) { + if (tab.url == null) { + holder.newTabImage.show() + holder.tabPreview.hide() + } else { + holder.newTabImage.hide() + holder.tabPreview.show() + } + val previewFile = tab.tabPreviewFile ?: return glide.clear(holder.tabPreview) lifecycleOwner.lifecycleScope.launch { @@ -340,6 +352,7 @@ class TabSwitcherAdapter( override val close: ImageView = binding.close, override val tabUnread: ImageView = binding.tabUnread, val tabPreview: ImageView = binding.tabPreview, + val newTabImage: ImageView = binding.newTabImage, ) : TabSwitcherViewHolder(binding.root), TabViewHolder data class ListTabViewHolder( diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index ffbc5863ea8c..a022ac1d82f8 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/app/src/main/res/layout/item_tab_grid.xml @@ -33,7 +33,7 @@ android:layout_marginStart="10dp" android:layout_marginTop="10dp" android:contentDescription="@string/faviconContentDescription" - android:src="@drawable/ic_globe_gray_16dp" + android:src="@drawable/ic_dax_icon" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" /> @@ -92,6 +92,18 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/title" /> + + \ No newline at end of file diff --git a/app/src/main/res/layout/item_tab_list.xml b/app/src/main/res/layout/item_tab_list.xml index 325fd4dc9708..4d99b72386b7 100644 --- a/app/src/main/res/layout/item_tab_list.xml +++ b/app/src/main/res/layout/item_tab_list.xml @@ -35,7 +35,7 @@ android:layout_width="@dimen/listItemImageMediumSize" android:layout_height="@dimen/listItemImageMediumSize" android:contentDescription="@string/faviconContentDescription" - android:src="@drawable/ic_globe_gray_16dp" + android:src="@drawable/ic_dax_icon" android:layout_marginHorizontal="@dimen/keyline_2" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" From 868a3c37b8d2bed85e93933ebc02bcfb56886e4f Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 11:29:07 +0100 Subject: [PATCH 03/20] Generate tab preview image when a tab is swiped --- .../main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 1 + .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++++ .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 14daeff7f43a..1e92602e42a4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -219,6 +219,7 @@ open class BrowserActivity : DuckDuckGoActivity() { if (wasSwipingStarted) { wasSwipingStarted = false + currentTab?.onTabSwipedAway() viewModel.onTabsSwiped() onTabPageSwiped(position) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index aa4f96061314..de5bd55319cd 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -4207,6 +4207,10 @@ class BrowserTabFragment : val roomParameters = "?skipMediaPermissionPrompt" webView?.loadUrl("${webView?.url.orEmpty()}$roomParameters") } + + fun onTabSwipedAway() { + viewModel.onTabSwipedAway() + } } private class JsOrientationHandler { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index f55044459275..002d20c48fac 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -3847,6 +3847,10 @@ class BrowserTabViewModel @Inject constructor( } } + fun onTabSwipedAway() { + command.value = GenerateWebViewPreviewImage + } + companion object { private const val FIXED_PROGRESS = 50 From 3ef78136e0a463d06431b24b775cb82051990c4c Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 11:41:20 +0100 Subject: [PATCH 04/20] Fix the scroll to current tab --- .../java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index 18906168addf..3068a018f01c 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -225,7 +225,6 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine val centerOffsetPercent = getCurrentCenterOffset() - this.layoutType = layoutType when (layoutType) { LayoutType.GRID -> { val gridLayoutManager = GridLayoutManager( @@ -244,10 +243,12 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.onLayoutTypeChanged(layoutType) tabTouchHelper.onLayoutTypeChanged(layoutType) - if (!firstTimeLoadingTabsList) { + if (!firstTimeLoadingTabsList && this.layoutType != null) { scrollToPreviousCenterOffset(centerOffsetPercent) } + this.layoutType = layoutType + tabsRecycler.show() } From 7d767d2afe94927220f6f942a907aa806bd53235 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 16:38:13 +0100 Subject: [PATCH 05/20] Fix the tab switcher grid item paddings --- app/src/main/res/layout/item_tab_grid.xml | 29 ++++++++++------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index a022ac1d82f8..e102ff2d0451 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/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"> + android:layout_marginStart="@dimen/keyline_2" + app:layout_constraintTop_toTopOf="@id/close" + app:layout_constraintBottom_toBottomOf="@id/close" + app:layout_constraintStart_toStartOf="parent" /> + app:layout_constraintTop_toTopOf="parent" /> + tools:text="Slashdot test tes tes tes tes tes tew tes tes tes tes tes tes tes" /> + app:layout_constraintTop_toBottomOf="@id/close" /> + app:layout_constraintTop_toBottomOf="@id/close" /> From cccaec5a3c89acde0f3fa23100e83a1299b8d1b3 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 5 Mar 2025 17:13:08 +0100 Subject: [PATCH 06/20] Fix the list item paddings --- app/src/main/res/layout/item_tab_list.xml | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/app/src/main/res/layout/item_tab_list.xml b/app/src/main/res/layout/item_tab_list.xml index 4d99b72386b7..11b378da1d56 100644 --- a/app/src/main/res/layout/item_tab_list.xml +++ b/app/src/main/res/layout/item_tab_list.xml @@ -18,17 +18,17 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - app:cardCornerRadius="10dp" - android:layout_marginStart="7dp" - android:layout_marginTop="6dp" - android:layout_marginEnd="7dp" - android:layout_marginBottom="10dp"> + android:layout_marginHorizontal="@dimen/keyline_2" + android:layout_marginVertical="6dp" + android:minHeight="@dimen/twoLineItemHeight" + app:cardCornerRadius="@dimen/smallShapeCornerRadius"> + android:layout_height="match_parent"> @@ -94,6 +94,7 @@ android:lines="1" android:textIsSelectable="false" android:textColor="?attr/daxColorSecondaryText" + android:layout_marginBottom="@dimen/keyline_2" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="@id/title" app:layout_constraintStart_toStartOf="@id/title" From 715f7ffc3a49fd268468f60290bf286c49eb7d52 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 6 Mar 2025 09:26:13 +0100 Subject: [PATCH 07/20] Revert "Fix the list item paddings" This reverts commit 25987d5e66336c64c81d4bcc4918f6782a9d044d. --- app/src/main/res/layout/item_tab_list.xml | 27 +++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/app/src/main/res/layout/item_tab_list.xml b/app/src/main/res/layout/item_tab_list.xml index 11b378da1d56..4d99b72386b7 100644 --- a/app/src/main/res/layout/item_tab_list.xml +++ b/app/src/main/res/layout/item_tab_list.xml @@ -18,17 +18,17 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_marginHorizontal="@dimen/keyline_2" - android:layout_marginVertical="6dp" - android:minHeight="@dimen/twoLineItemHeight" - app:cardCornerRadius="@dimen/smallShapeCornerRadius"> + app:cardCornerRadius="10dp" + android:layout_marginStart="7dp" + android:layout_marginTop="6dp" + android:layout_marginEnd="7dp" + android:layout_marginBottom="10dp"> + android:layout_height="match_parent" + android:padding="@dimen/twoLineItemVerticalPadding"> @@ -94,7 +94,6 @@ android:lines="1" android:textIsSelectable="false" android:textColor="?attr/daxColorSecondaryText" - android:layout_marginBottom="@dimen/keyline_2" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="@id/title" app:layout_constraintStart_toStartOf="@id/title" From 9f38fea9a4c569da7e02196eed6c0890afd15c38 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 6 Mar 2025 10:19:47 +0100 Subject: [PATCH 08/20] Use a single ImageView for all tabs --- .../duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt | 15 +++++++-------- app/src/main/res/layout/item_tab_grid.xml | 11 ----------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index a43a6181d015..bee5b835988c 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -23,6 +23,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.ImageView +import android.widget.ImageView.ScaleType import android.widget.TextView import androidx.annotation.VisibleForTesting import androidx.lifecycle.LifecycleOwner @@ -48,12 +49,11 @@ import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabSwitcherViewHolder.Companion.GRID_TAB 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.hide import com.duckduckgo.common.ui.view.show 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 kotlinx.coroutines.launch import kotlinx.coroutines.withContext import timber.log.Timber @@ -242,7 +242,7 @@ class TabSwitcherAdapter( private fun loadFavicon(tab: TabEntity, view: ImageView) { if (tab.url == null) { - Glide.with(view).load(com.duckduckgo.mobile.android.R.drawable.ic_dax_icon).into(view) + Glide.with(view).load(AndroidR.drawable.ic_dax_icon).into(view) } else { lifecycleOwner.lifecycleScope.launch { faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, tab.url!!, view) @@ -252,11 +252,11 @@ class TabSwitcherAdapter( private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) { if (tab.url == null) { - holder.newTabImage.show() - holder.tabPreview.hide() + holder.tabPreview.scaleType = ScaleType.FIT_CENTER + Glide.with(holder.tabPreview).load(AndroidR.drawable.ic_dax_splash_screen_icon).into(holder.tabPreview) + return } else { - holder.newTabImage.hide() - holder.tabPreview.show() + holder.tabPreview.scaleType = ScaleType.MATRIX } val previewFile = tab.tabPreviewFile ?: return glide.clear(holder.tabPreview) @@ -352,7 +352,6 @@ class TabSwitcherAdapter( override val close: ImageView = binding.close, override val tabUnread: ImageView = binding.tabUnread, val tabPreview: ImageView = binding.tabPreview, - val newTabImage: ImageView = binding.newTabImage, ) : TabSwitcherViewHolder(binding.root), TabViewHolder data class ListTabViewHolder( diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index e102ff2d0451..1faed82b596d 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/app/src/main/res/layout/item_tab_grid.xml @@ -90,17 +90,6 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/close" /> - - \ No newline at end of file From 94becfbd0fe45bd758b401c068616eb1e8820efb Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 6 Mar 2025 10:25:05 +0100 Subject: [PATCH 09/20] Clean up the code --- .../duckduckgo/app/browser/BrowserActivity.kt | 16 +++++++++------- app/src/main/res/layout/item_tab_grid.xml | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 1e92602e42a4..a3108c3dd0cb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -219,10 +219,7 @@ open class BrowserActivity : DuckDuckGoActivity() { if (wasSwipingStarted) { wasSwipingStarted = false - currentTab?.onTabSwipedAway() - viewModel.onTabsSwiped() onTabPageSwiped(position) - enableWebViewScrolling() } } @@ -919,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) + } } } diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index 1faed82b596d..26f42a9c11fd 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/app/src/main/res/layout/item_tab_grid.xml @@ -77,7 +77,7 @@ app:layout_constraintBottom_toBottomOf="@id/close" app:layout_constraintHorizontal_bias="0.0" app:typography="h5" - tools:text="Slashdot test tes tes tes tes tes tew tes tes tes tes tes tes tes" /> + tools:text="Slashdot" /> Date: Thu, 6 Mar 2025 11:39:43 +0100 Subject: [PATCH 10/20] Fix the grid item logo and paddings to match the specs --- .../app/tabs/ui/TabRendererExtension.kt | 2 +- .../app/tabs/ui/TabSwitcherAdapter.kt | 6 ++- app/src/main/res/layout/item_tab_grid.xml | 2 +- .../src/main/res/drawable/ic_dax_icon_72.xml | 42 +++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 common/common-ui/src/main/res/drawable/ic_dax_icon_72.xml diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabRendererExtension.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabRendererExtension.kt index f029ae7d9c1c..6b5864980972 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabRendererExtension.kt +++ b/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 ?: "" diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index bee5b835988c..87528f2774ab 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -252,8 +252,10 @@ class TabSwitcherAdapter( private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) { if (tab.url == null) { - holder.tabPreview.scaleType = ScaleType.FIT_CENTER - Glide.with(holder.tabPreview).load(AndroidR.drawable.ic_dax_splash_screen_icon).into(holder.tabPreview) + holder.tabPreview.scaleType = ScaleType.CENTER + Glide.with(holder.tabPreview) + .load(AndroidR.drawable.ic_dax_icon_72) + .into(holder.tabPreview) return } else { holder.tabPreview.scaleType = ScaleType.MATRIX diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index 26f42a9c11fd..43583dfd7adb 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/app/src/main/res/layout/item_tab_grid.xml @@ -32,7 +32,7 @@ android:layout_height="@dimen/listItemImageSmallSize" android:contentDescription="@string/faviconContentDescription" android:src="@drawable/ic_dax_icon" - android:layout_marginStart="@dimen/keyline_2" + android:layout_marginStart="@dimen/keyline_3" app:layout_constraintTop_toTopOf="@id/close" app:layout_constraintBottom_toBottomOf="@id/close" app:layout_constraintStart_toStartOf="parent" /> diff --git a/common/common-ui/src/main/res/drawable/ic_dax_icon_72.xml b/common/common-ui/src/main/res/drawable/ic_dax_icon_72.xml new file mode 100644 index 000000000000..20e8d3c504bf --- /dev/null +++ b/common/common-ui/src/main/res/drawable/ic_dax_icon_72.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + From 4c3971bb0b93af06d17c51fb3a0001b75b4a358e Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 6 Mar 2025 11:40:12 +0100 Subject: [PATCH 11/20] Reapply "Fix the list item paddings" This reverts commit bbf050fb906c27b4584e85591c4c56d8b89e2428. --- app/src/main/res/layout/item_tab_list.xml | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/app/src/main/res/layout/item_tab_list.xml b/app/src/main/res/layout/item_tab_list.xml index 4d99b72386b7..11b378da1d56 100644 --- a/app/src/main/res/layout/item_tab_list.xml +++ b/app/src/main/res/layout/item_tab_list.xml @@ -18,17 +18,17 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - app:cardCornerRadius="10dp" - android:layout_marginStart="7dp" - android:layout_marginTop="6dp" - android:layout_marginEnd="7dp" - android:layout_marginBottom="10dp"> + android:layout_marginHorizontal="@dimen/keyline_2" + android:layout_marginVertical="6dp" + android:minHeight="@dimen/twoLineItemHeight" + app:cardCornerRadius="@dimen/smallShapeCornerRadius"> + android:layout_height="match_parent"> @@ -94,6 +94,7 @@ android:lines="1" android:textIsSelectable="false" android:textColor="?attr/daxColorSecondaryText" + android:layout_marginBottom="@dimen/keyline_2" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="@id/title" app:layout_constraintStart_toStartOf="@id/title" From 6c81ce4f2c7b3905e2ab03d25646335488fe20e3 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 6 Mar 2025 12:01:14 +0100 Subject: [PATCH 12/20] Fix the list item dimensions and paddings --- .../duckduckgo/app/tabs/ui/TabItemDecorator.kt | 2 +- app/src/main/res/layout/item_tab_list.xml | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabItemDecorator.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabItemDecorator.kt index 381b778bd49a..47a953c1ae87 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabItemDecorator.kt +++ b/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() } diff --git a/app/src/main/res/layout/item_tab_list.xml b/app/src/main/res/layout/item_tab_list.xml index 11b378da1d56..deb20f8cdc21 100644 --- a/app/src/main/res/layout/item_tab_list.xml +++ b/app/src/main/res/layout/item_tab_list.xml @@ -18,14 +18,13 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_marginHorizontal="@dimen/keyline_2" - android:layout_marginVertical="6dp" - android:minHeight="@dimen/twoLineItemHeight" + android:layout_margin="@dimen/keyline_2" + android:minHeight="72dp" app:cardCornerRadius="@dimen/smallShapeCornerRadius"> @@ -51,13 +50,10 @@ android:elevation="30dp" android:importantForAccessibility="no" android:src="@drawable/tab_unread_indicator" - app:layout_constraintBottom_toBottomOf="@id/favicon" app:layout_constraintCircle="@id/favicon" app:layout_constraintCircleAngle="135" - app:layout_constraintCircleRadius="8dp" - app:layout_constraintEnd_toEndOf="@id/favicon" - app:layout_constraintStart_toEndOf="@id/favicon" - app:layout_constraintTop_toBottomOf="@id/favicon" /> + app:layout_constraintCircleRadius="@dimen/keyline_3" + tools:ignore="MissingConstraints" /> Date: Thu, 6 Mar 2025 12:12:58 +0100 Subject: [PATCH 13/20] Clean up the tab switcher adapter --- .../app/tabs/ui/TabSwitcherAdapter.kt | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index 87528f2774ab..9f789913b8ed 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -133,11 +133,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 }, @@ -150,8 +151,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 }, @@ -201,7 +202,7 @@ 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_TITLE)?.let { @@ -240,9 +241,9 @@ class TabSwitcherAdapter( } } - private fun loadFavicon(tab: TabEntity, view: ImageView) { + private fun loadFavicon(tab: TabEntity, glide: RequestManager, view: ImageView) { if (tab.url == null) { - Glide.with(view).load(AndroidR.drawable.ic_dax_icon).into(view) + glide.load(AndroidR.drawable.ic_dax_icon).into(view) } else { lifecycleOwner.lifecycleScope.launch { faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, tab.url!!, view) @@ -250,18 +251,17 @@ class TabSwitcherAdapter( } } - private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, holder: TabSwitcherViewHolder.GridTabViewHolder) { + private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, tabPreview: ImageView) { if (tab.url == null) { - holder.tabPreview.scaleType = ScaleType.CENTER - Glide.with(holder.tabPreview) - .load(AndroidR.drawable.ic_dax_icon_72) - .into(holder.tabPreview) + tabPreview.scaleType = ScaleType.CENTER + glide.load(AndroidR.drawable.ic_dax_icon_72) + .into(tabPreview) return } else { - holder.tabPreview.scaleType = ScaleType.MATRIX + tabPreview.scaleType = ScaleType.MATRIX } - val previewFile = tab.tabPreviewFile ?: return glide.clear(holder.tabPreview) + val previewFile = tab.tabPreviewFile ?: return glide.clear(tabPreview) lifecycleOwner.lifecycleScope.launch { val cachedWebViewPreview = withContext(dispatchers.io()) { @@ -269,15 +269,15 @@ class TabSwitcherAdapter( } if (cachedWebViewPreview == null) { - glide.clear(holder.tabPreview) + glide.clear(tabPreview) return@launch } glide.load(cachedWebViewPreview) .transition(DrawableTransitionOptions.withCrossFade()) - .into(holder.tabPreview) + .into(tabPreview) - holder.tabPreview.show() + tabPreview.show() } } From f80c7ce07925090e817d07872de3f75e6bbf574b Mon Sep 17 00:00:00 2001 From: 0nko Date: Tue, 11 Mar 2025 20:41:34 +0100 Subject: [PATCH 14/20] Fix the favicon and preview loading --- .../app/tabs/ui/TabSwitcherAdapter.kt | 70 +++++++++++++------ app/src/main/res/layout/item_tab_grid.xml | 2 +- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index 9f789913b8ed..86a4411ae3dd 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -18,12 +18,12 @@ 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 import android.view.ViewGroup import android.widget.ImageView -import android.widget.ImageView.ScaleType import android.widget.TextView import androidx.annotation.VisibleForTesting import androidx.lifecycle.LifecycleOwner @@ -34,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 @@ -50,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 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, @@ -205,6 +211,10 @@ class TabSwitcherAdapter( 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 { viewHolder.title.text = it } @@ -229,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 { @@ -242,42 +253,55 @@ class TabSwitcherAdapter( } private fun loadFavicon(tab: TabEntity, glide: RequestManager, view: ImageView) { - if (tab.url == null) { + val url = tab.url + if (url == null) { + glide.clear(view) glide.load(AndroidR.drawable.ic_dax_icon).into(view) } else { lifecycleOwner.lifecycleScope.launch { - faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, tab.url!!, view) + faviconManager.loadToViewFromLocalWithPlaceholder(tab.tabId, url, view) } } } private fun loadTabPreviewImage(tab: TabEntity, glide: RequestManager, tabPreview: ImageView) { - if (tab.url == null) { - tabPreview.scaleType = ScaleType.CENTER - glide.load(AndroidR.drawable.ic_dax_icon_72) - .into(tabPreview) - return - } else { - tabPreview.scaleType = ScaleType.MATRIX - } - - val previewFile = tab.tabPreviewFile ?: return glide.clear(tabPreview) - - lifecycleOwner.lifecycleScope.launch { - val cachedWebViewPreview = withContext(dispatchers.io()) { - File(webViewPreviewPersister.fullPathForFile(tab.tabId, previewFile)).takeIf { it.exists() } + fun fitAndClipBottom() = object : Transformation { + override fun transform( + context: Context, + resource: Resource, + outWidth: Int, + outHeight: Int, + ): Resource { + resource.get().height = GRID_ITEM_HEIGHT_DP.toPx() + return resource } - if (cachedWebViewPreview == null) { - glide.clear(tabPreview) - return@launch + override fun updateDiskCacheKey(messageDigest: MessageDigest) { } + } - glide.load(cachedWebViewPreview) - .transition(DrawableTransitionOptions.withCrossFade()) + val previewFile = tab.tabPreviewFile + if (tab.url == null) { + 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() } + } + + if (cachedWebViewPreview == null) { + glide.clear(tabPreview) + return@launch + } - tabPreview.show() + glide.load(cachedWebViewPreview) + .transition(DrawableTransitionOptions.withCrossFade()) + .optionalTransform(fitAndClipBottom()) + .into(tabPreview) + } + } else { + glide.clear(tabPreview) } } diff --git a/app/src/main/res/layout/item_tab_grid.xml b/app/src/main/res/layout/item_tab_grid.xml index 43583dfd7adb..c8b17c6d26f5 100644 --- a/app/src/main/res/layout/item_tab_grid.xml +++ b/app/src/main/res/layout/item_tab_grid.xml @@ -84,7 +84,7 @@ android:layout_width="match_parent" android:layout_height="@dimen/gridItemPreviewHeight" android:importantForAccessibility="no" - android:scaleType="matrix" + android:scaleType="center" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" From 14fc760bf50faec1c895f9a36bd3ac215532ccae Mon Sep 17 00:00:00 2001 From: 0nko Date: Tue, 11 Mar 2025 20:53:11 +0100 Subject: [PATCH 15/20] Fix the unit tests --- .../com/duckduckgo/app/tabs/ui/TabRendererExtensionTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/test/java/com/duckduckgo/app/tabs/ui/TabRendererExtensionTest.kt b/app/src/test/java/com/duckduckgo/app/tabs/ui/TabRendererExtensionTest.kt index 325f009f55cd..90509a72a5d9 100644 --- a/app/src/test/java/com/duckduckgo/app/tabs/ui/TabRendererExtensionTest.kt +++ b/app/src/test/java/com/duckduckgo/app/tabs/ui/TabRendererExtensionTest.kt @@ -33,8 +33,8 @@ class TabRendererExtensionTest { @Test fun whenTabIsBlankThenDisplayTitleIsDuckDuckGo() { - whenever(context.getString(R.string.homeTab)).thenReturn("DuckDuckGo") - assertEquals("DuckDuckGo", TabEntity("", position = 0).displayTitle(context)) + whenever(context.getString(R.string.newTabMenuItem)).thenReturn("New Tab") + assertEquals("New Tab", TabEntity("", position = 0).displayTitle(context)) } @Test From 5d29d5c13279ef230867fd6b51707f36747c267a Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 19 Mar 2025 13:35:17 +0100 Subject: [PATCH 16/20] Revert "Fix the scroll to current tab" This reverts commit 3ef78136e0a463d06431b24b775cb82051990c4c. --- .../java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index 3068a018f01c..18906168addf 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -225,6 +225,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine val centerOffsetPercent = getCurrentCenterOffset() + this.layoutType = layoutType when (layoutType) { LayoutType.GRID -> { val gridLayoutManager = GridLayoutManager( @@ -243,12 +244,10 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.onLayoutTypeChanged(layoutType) tabTouchHelper.onLayoutTypeChanged(layoutType) - if (!firstTimeLoadingTabsList && this.layoutType != null) { + if (!firstTimeLoadingTabsList) { scrollToPreviousCenterOffset(centerOffsetPercent) } - this.layoutType = layoutType - tabsRecycler.show() } From c789b40a3030f0fb809250af68a40dd57e728009 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 19 Mar 2025 13:35:43 +0100 Subject: [PATCH 17/20] Revert "Fix the scroll to current tab" This reverts commit 10ceb6145d5ff0e6fccc3e1f3e103eae3fe544fb. --- .../duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt index 18906168addf..b967db7b2621 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -195,8 +195,6 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine if (noTabSelected && tabSwitcherItems.isNotEmpty()) { updateTabGridItemDecorator(tabSwitcherItems.last().id) } - - scrollToCurrentTab() } viewModel.activeTab.observe(this) { tab -> if (tab != null && tab.tabId != tabItemDecorator.tabSwitcherItemId && !tab.deletable) { @@ -244,18 +242,17 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.onLayoutTypeChanged(layoutType) tabTouchHelper.onLayoutTypeChanged(layoutType) - if (!firstTimeLoadingTabsList) { + if (firstTimeLoadingTabsList) { + firstTimeLoadingTabsList = false + + scrollToShowCurrentTab() + } else { scrollToPreviousCenterOffset(centerOffsetPercent) } tabsRecycler.show() } - private fun scrollToCurrentTab() { - firstTimeLoadingTabsList = false - scrollToPositionOfCurrentTab() - } - private fun scrollToPreviousCenterOffset(centerOffsetPercent: Float) { tabsRecycler.post { val newRange = tabsRecycler.computeVerticalScrollRange() @@ -293,7 +290,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine tabsAdapter.updateData(tabs) } - private fun scrollToPositionOfCurrentTab() { + private fun scrollToShowCurrentTab() { val index = tabsAdapter.adapterPositionForTab(selectedTabId) if (index != -1) { scrollToPosition(index) From 4590bf870d21978f56f1254dfc87d42175deea33 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 19 Mar 2025 14:20:11 +0100 Subject: [PATCH 18/20] Retry the favicon load after 2 seconds if not available --- .../browser/favicon/DuckDuckGoFaviconManager.kt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt b/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt index 16b60d1e11f1..68f11785c9a5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt +++ b/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 @@ -36,9 +38,13 @@ import com.duckduckgo.common.utils.touchFaviconLocation import com.duckduckgo.savedsites.api.SavedSitesRepository import com.duckduckgo.savedsites.store.SavedSitesEntitiesDao import com.duckduckgo.sync.api.favicons.FaviconsFetchingStore +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import java.io.File import kotlinx.coroutines.withContext +private const val FAVICON_LOAD_RETRY_DELAY = 2000L + class DuckDuckGoFaviconManager constructor( private val faviconPersister: FaviconPersister, private val savedSitesDao: SavedSitesEntitiesDao, @@ -170,8 +176,15 @@ 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) + if (bitmap == null) { + view.findViewTreeLifecycleOwner()?.lifecycleScope?.launch { + delay(FAVICON_LOAD_RETRY_DELAY) + bitmap = loadFromDisk(tabId, url) + view.loadFavicon(bitmap, url, placeholder) + } + } } override suspend fun persistCachedFavicon( From 661920c98d9b0c5477f48fdb3054eb7214f4a3cd Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 19 Mar 2025 14:20:35 +0100 Subject: [PATCH 19/20] Update the NTP check condition --- .../java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index 86a4411ae3dd..ca5628bbd277 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -254,7 +254,7 @@ class TabSwitcherAdapter( private fun loadFavicon(tab: TabEntity, glide: RequestManager, view: ImageView) { val url = tab.url - if (url == null) { + if (url.isNullOrBlank()) { glide.clear(view) glide.load(AndroidR.drawable.ic_dax_icon).into(view) } else { @@ -281,7 +281,7 @@ class TabSwitcherAdapter( } val previewFile = tab.tabPreviewFile - if (tab.url == null) { + if (tab.url.isNullOrBlank()) { glide.load(AndroidR.drawable.ic_dax_icon_72) .into(tabPreview) } else if (previewFile != null) { From 56089cc9387a4e7a05d886e2e8c24ecd3dc55e02 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 19 Mar 2025 21:26:46 +0100 Subject: [PATCH 20/20] Retry the favicon load 3 times --- .../favicon/DuckDuckGoFaviconManager.kt | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt b/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt index 68f11785c9a5..5472dd31849a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/favicon/DuckDuckGoFaviconManager.kt @@ -38,12 +38,13 @@ import com.duckduckgo.common.utils.touchFaviconLocation 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 java.io.File 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, @@ -178,11 +179,17 @@ class DuckDuckGoFaviconManager constructor( override suspend fun loadToViewFromLocalWithPlaceholder(tabId: String?, url: String, view: ImageView, placeholder: String?) { var bitmap = loadFromDisk(tabId, url) view.loadFavicon(bitmap, url, placeholder) - if (bitmap == null) { - view.findViewTreeLifecycleOwner()?.lifecycleScope?.launch { - delay(FAVICON_LOAD_RETRY_DELAY) - 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) + } + } } } }