From 5011f5ff37f3ee1645a77adbcfe6cdb87cdfa142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Thu, 11 Jul 2024 12:51:09 +0200 Subject: [PATCH 1/4] Ensure RMF is not shown on app start if browser showing --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 6 ++++++ .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 6 +++++- .../com/duckduckgo/app/browser/viewstate/CtaViewState.kt | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) 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 067c354770bc..0c7cc7d74d04 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -3808,6 +3808,10 @@ class BrowserTabFragment : showCta(viewState.cta) } + viewState.isBrowserShowing -> { + hideNewTab() + } + viewState.daxOnboardingComplete -> { showNewTab() } @@ -3914,6 +3918,7 @@ class BrowserTabFragment : } private fun showNewTab() { + Timber.d("New Tab: showNewTab") newTabPageProvider.provideNewTabPageVersion().onEach { newTabPage -> newBrowserTab.newTabContainerLayout.addView( newTabPage.getView(requireContext()), @@ -3928,6 +3933,7 @@ class BrowserTabFragment : } private fun hideNewTab() { + Timber.d("New Tab: hideNewTab") newBrowserTab.newTabContainerLayout.gone() } 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 3f1d84ea8dc9..fc8c8658e2b3 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -2431,7 +2431,11 @@ class BrowserTabViewModel @Inject constructor( ctaViewModel.daxDialogEndShown() } if (isBrowserShowing && cta != null) hasCtaBeenShownForCurrentPage.set(true) - ctaViewState.value = currentCtaViewState().copy(cta = cta, daxOnboardingComplete = isOnboardingComplete) + ctaViewState.value = currentCtaViewState().copy( + cta = cta, + daxOnboardingComplete = isOnboardingComplete, + isBrowserShowing = isBrowserShowing, + ) ctaChangedTicker.emit(System.currentTimeMillis().toString()) return cta } diff --git a/app/src/main/java/com/duckduckgo/app/browser/viewstate/CtaViewState.kt b/app/src/main/java/com/duckduckgo/app/browser/viewstate/CtaViewState.kt index 069432032598..b172de7a611d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/viewstate/CtaViewState.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/viewstate/CtaViewState.kt @@ -21,4 +21,5 @@ import com.duckduckgo.app.cta.ui.Cta data class CtaViewState( val cta: Cta? = null, val daxOnboardingComplete: Boolean = false, + val isBrowserShowing: Boolean = true, ) From 577a7eafd827d107fb6799a79ff01ed12fd139f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Thu, 11 Jul 2024 13:14:14 +0200 Subject: [PATCH 2/4] hide new tab in browser showing --- .../app/browser/BrowserTabViewModelTest.kt | 13 +++++++++++++ .../duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 85fde2a17dae..02f65efc20fa 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -2366,6 +2366,19 @@ class BrowserTabViewModelTest { testee.refreshCta() assertNull(testee.ctaViewState.value!!.cta) assertTrue(testee.ctaViewState.value!!.daxOnboardingComplete) + assertFalse(testee.ctaViewState.value!!.isBrowserShowing) + } + + @Test + fun whenCtaRefreshedAndBrowserShowingThenViewStateUpdated() = runTest { + setBrowserShowing(true) + whenever(mockWidgetCapabilities.supportsAutomaticWidgetAdd).thenReturn(false) + whenever(mockWidgetCapabilities.hasInstalledWidgets).thenReturn(true) + whenever(mockDismissedCtaDao.exists(DAX_END)).thenReturn(true) + testee.refreshCta() + assertNull(testee.ctaViewState.value!!.cta) + assertTrue(testee.ctaViewState.value!!.daxOnboardingComplete) + assertTrue(testee.ctaViewState.value!!.isBrowserShowing) } @Test 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 0c7cc7d74d04..0d403c2a02df 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1152,6 +1152,7 @@ class BrowserTabFragment : dismissAppLinkSnackBar() errorSnackbar.dismiss() newBrowserTab.newTabLayout.show() + newBrowserTab.newTabContainerLayout.show() binding.browserLayout.gone() webViewContainer.gone() omnibar.appBarLayout.setExpanded(true) @@ -1163,6 +1164,7 @@ class BrowserTabFragment : private fun showBrowser() { newBrowserTab.newTabLayout.gone() + newBrowserTab.newTabContainerLayout.gone() binding.browserLayout.show() webViewContainer.show() webView?.show() @@ -1177,6 +1179,7 @@ class BrowserTabFragment : ) { webViewContainer.gone() newBrowserTab.newTabLayout.gone() + newBrowserTab.newTabContainerLayout.gone() sslErrorView.gone() omnibar.appBarLayout.setExpanded(true) omnibar.shieldIcon.isInvisible = true @@ -1197,6 +1200,7 @@ class BrowserTabFragment : ) { webViewContainer.gone() newBrowserTab.newTabLayout.gone() + newBrowserTab.newTabContainerLayout.gone() webView?.onPause() webView?.hide() omnibar.appBarLayout.setExpanded(true) From c3ead67568132e2c00361607fc37f9b1d59261cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Thu, 11 Jul 2024 14:47:48 +0200 Subject: [PATCH 3/4] ensure RMF checks visibility --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++++ .../duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt | 8 +++++++- .../app/browser/newtab/NewTabLegacyPageViewModel.kt | 3 +++ .../impl/RemoteMessagingConfigDownloadScheduler.kt | 2 +- 4 files changed, 15 insertions(+), 2 deletions(-) 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 0d403c2a02df..1f8f1e187a0a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1148,6 +1148,7 @@ class BrowserTabFragment : } private fun showHome() { + Timber.d("New Tab: showHome") viewModel.onHomeShown() dismissAppLinkSnackBar() errorSnackbar.dismiss() @@ -1163,6 +1164,7 @@ class BrowserTabFragment : } private fun showBrowser() { + Timber.d("New Tab: showBrowser") newBrowserTab.newTabLayout.gone() newBrowserTab.newTabContainerLayout.gone() binding.browserLayout.show() @@ -1177,6 +1179,7 @@ class BrowserTabFragment : errorType: WebViewErrorResponse, url: String?, ) { + Timber.d("New Tab: showError") webViewContainer.gone() newBrowserTab.newTabLayout.gone() newBrowserTab.newTabContainerLayout.gone() @@ -3934,6 +3937,7 @@ class BrowserTabFragment : } .launchIn(lifecycleScope) newBrowserTab.newTabContainerLayout.show() + newBrowserTab.newTabLayout.show() } private fun hideNewTab() { diff --git a/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt b/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt index b4ad040e1f08..503fc3ee02bb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt @@ -23,9 +23,11 @@ import android.content.Context import android.content.Intent import android.text.Spanned import android.util.AttributeSet +import android.view.View import android.widget.LinearLayout import androidx.core.text.toSpannable import androidx.core.view.isGone +import androidx.core.view.isVisible import androidx.fragment.app.findFragment import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.findViewTreeLifecycleOwner @@ -217,6 +219,7 @@ class NewTabLegacyPageView @JvmOverloads constructor( } private fun render(viewState: ViewState) { + Timber.d("New Tab: render $viewState") if (viewState.message == null && viewState.favourites.isEmpty()) { homeBackgroundLogo.showLogo() } else { @@ -344,7 +347,10 @@ class NewTabLegacyPageView @JvmOverloads constructor( message: RemoteMessage, newMessage: Boolean, ) { - val shouldRender = newMessage || binding.messageCta.isGone + val parentVisible = (this.parent as? View)?.isVisible ?: false + Timber.d("New Tab: RMF isParentVisible $parentVisible") + + val shouldRender = parentVisible && (newMessage || binding.messageCta.isGone) if (shouldRender) { binding.messageCta.setMessage(message.asMessage()) diff --git a/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageViewModel.kt index 42902110e245..c4d20ff90967 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageViewModel.kt @@ -58,6 +58,7 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import timber.log.Timber @SuppressLint("NoLifecycleObserver") // we don't observe app lifecycle @ContributesViewModel(ViewScope::class) @@ -115,6 +116,7 @@ class NewTabLegacyPageViewModel @Inject constructor( override fun onStart(owner: LifecycleOwner) { super.onStart(owner) + Timber.d("New Tab: onStart") viewModelScope.launch(dispatchers.io()) { savedSitesRepository.getFavorites() .combine(hiddenIds) { favorites, hiddenIds -> @@ -128,6 +130,7 @@ class NewTabLegacyPageViewModel @Inject constructor( } .flowOn(dispatchers.io()) .onEach { snapshot -> + Timber.d("New Tab: $snapshot") val newMessage = snapshot.remoteMessage?.id != lastRemoteMessageSeen?.id if (newMessage) { lastRemoteMessageSeen = snapshot.remoteMessage diff --git a/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt b/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt index 6847dc0eda1f..71ed214acfc4 100644 --- a/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt +++ b/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt @@ -60,7 +60,7 @@ class RemoteMessagingConfigDownloadScheduler @Inject constructor( private val workManager: WorkManager, ) : MainProcessLifecycleObserver { - override fun onCreate(owner: LifecycleOwner) { + override fun onStart(owner: LifecycleOwner) { scheduleDownload() } From 087253aedb587c46a7b7779a10a025e3005c93ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Thu, 11 Jul 2024 15:15:44 +0200 Subject: [PATCH 4/4] rever to onCreate --- .../messaging/impl/RemoteMessagingConfigDownloadScheduler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt b/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt index 71ed214acfc4..6847dc0eda1f 100644 --- a/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt +++ b/remote-messaging/remote-messaging-impl/src/main/java/com/duckduckgo/remote/messaging/impl/RemoteMessagingConfigDownloadScheduler.kt @@ -60,7 +60,7 @@ class RemoteMessagingConfigDownloadScheduler @Inject constructor( private val workManager: WorkManager, ) : MainProcessLifecycleObserver { - override fun onStart(owner: LifecycleOwner) { + override fun onCreate(owner: LifecycleOwner) { scheduleDownload() }