Skip to content

Ensure RMF is not shown on app start if browser showing #4753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 11, 2024
Merged

Ensure RMF is not shown on app start if browser showing #4753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5011f5f
Ensure RMF is not shown on app start if browser showing
malmstein Jul 11, 2024
577a7ea
hide new tab in browser showing
malmstein Jul 11, 2024
c3ead67
ensure RMF checks visibility
malmstein Jul 11, 2024
087253a
rever to onCreate
malmstein Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,10 +1148,12 @@ class BrowserTabFragment :
}

private fun showHome() {
Timber.d("New Tab: showHome")
viewModel.onHomeShown()
dismissAppLinkSnackBar()
errorSnackbar.dismiss()
newBrowserTab.newTabLayout.show()
newBrowserTab.newTabContainerLayout.show()
binding.browserLayout.gone()
webViewContainer.gone()
omnibar.appBarLayout.setExpanded(true)
Expand All @@ -1162,7 +1164,9 @@ class BrowserTabFragment :
}

private fun showBrowser() {
Timber.d("New Tab: showBrowser")
newBrowserTab.newTabLayout.gone()
newBrowserTab.newTabContainerLayout.gone()
binding.browserLayout.show()
webViewContainer.show()
webView?.show()
Expand All @@ -1175,8 +1179,10 @@ class BrowserTabFragment :
errorType: WebViewErrorResponse,
url: String?,
) {
Timber.d("New Tab: showError")
webViewContainer.gone()
newBrowserTab.newTabLayout.gone()
newBrowserTab.newTabContainerLayout.gone()
sslErrorView.gone()
omnibar.appBarLayout.setExpanded(true)
omnibar.shieldIcon.isInvisible = true
Expand All @@ -1197,6 +1203,7 @@ class BrowserTabFragment :
) {
webViewContainer.gone()
newBrowserTab.newTabLayout.gone()
newBrowserTab.newTabContainerLayout.gone()
Copy link
Contributor

@cmonfortep cmonfortep Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge case landing here during onboarding, but this can be removed based on what we saw during testing

Copy link
Contributor

@cmonfortep cmonfortep Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scratch that, it's correct to set newtab gone here.

webView?.onPause()
webView?.hide()
omnibar.appBarLayout.setExpanded(true)
Expand Down Expand Up @@ -3808,6 +3815,10 @@ class BrowserTabFragment :
showCta(viewState.cta)
}

viewState.isBrowserShowing -> {
hideNewTab()
}

viewState.daxOnboardingComplete -> {
showNewTab()
}
Expand Down Expand Up @@ -3914,6 +3925,7 @@ class BrowserTabFragment :
}

private fun showNewTab() {
Timber.d("New Tab: showNewTab")
newTabPageProvider.provideNewTabPageVersion().onEach { newTabPage ->
newBrowserTab.newTabContainerLayout.addView(
newTabPage.getView(requireContext()),
Expand All @@ -3925,9 +3937,11 @@ class BrowserTabFragment :
}
.launchIn(lifecycleScope)
newBrowserTab.newTabContainerLayout.show()
newBrowserTab.newTabLayout.show()
}

private fun hideNewTab() {
Timber.d("New Tab: hideNewTab")
newBrowserTab.newTabContainerLayout.gone()
}

Expand Down
6 changes: 5 additions & 1 deletion app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 7 additions & 1 deletion app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/com/duckduckgo/app/browser/newtab/NewTabLegacyPageViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 ->
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/src/main/java/com/duckduckgo/app/browser/viewstate/CtaViewState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)