Skip to content

Visual design: Privacy Shield #6000

New issue

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

#

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

Already on GitHub? # to your account

Merged
merged 15 commits into from
May 6, 2025
Merged

Visual design: Privacy Shield #6000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8aaf1f1
Visual design: Privacy Shield
malmstein Apr 29, 2025
88fe3e9
updated shield icon
malmstein Apr 29, 2025
1e7091e
rebase with experiment
malmstein May 2, 2025
3aa4a73
updated privacy shield
malmstein May 2, 2025
945cfd1
update privacy shield for visual designs
malmstein May 2, 2025
558aa50
clean up logs
malmstein May 2, 2025
e1a85a6
fix focus trackers test
malmstein May 2, 2025
04e95bd
add missing tests for resources
malmstein May 2, 2025
66bf2c1
fix last test
malmstein May 2, 2025
cd33166
clean up tests
malmstein May 5, 2025
f24d29a
first round of pr feedback
malmstein May 5, 2025
6d42771
updated privacy shields
malmstein May 5, 2025
d4d3232
proper animations being used
malmstein May 5, 2025
bd2deb9
fix asset reference in test
malmstein May 5, 2025
bfba565
clean up TODO
malmstein May 6, 2025
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
143 changes: 132 additions & 11 deletions ...a/com/duckduckgo/app/browser/omnibar/animations/LottiePrivacyShieldAnimationHelperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ import com.duckduckgo.app.browser.senseofprotection.SenseOfProtectionExperiment
import com.duckduckgo.app.global.model.PrivacyShield.MALICIOUS
import com.duckduckgo.app.global.model.PrivacyShield.PROTECTED
import com.duckduckgo.app.global.model.PrivacyShield.UNPROTECTED
import com.duckduckgo.common.ui.experiments.visual.store.VisualDesignExperimentDataStore
import com.duckduckgo.common.ui.experiments.visual.store.VisualDesignExperimentDataStore.FeatureState
import com.duckduckgo.common.ui.store.AppTheme
import kotlinx.coroutines.flow.MutableStateFlow
import org.junit.Before
import org.junit.Test
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
Expand All @@ -32,13 +36,25 @@ import org.mockito.kotlin.whenever
class LottiePrivacyShieldAnimationHelperTest {

private val senseOfProtectionExperiment: SenseOfProtectionExperiment = mock()
private val visualDesignExperimentDataStore: VisualDesignExperimentDataStore = mock()
private val enabledVisualExperimentStateFlow = MutableStateFlow(FeatureState(isAvailable = true, isEnabled = true))
private val disabledVisualExperimentStateFlow = MutableStateFlow(FeatureState(isAvailable = false, isEnabled = false))

@Before
fun setup() {
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
disabledVisualExperimentStateFlow,
)
}

@Test
fun whenLightModeAndPrivacyShieldProtectedThenSetLightShieldAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

Expand All @@ -47,10 +63,12 @@ class LottiePrivacyShieldAnimationHelperTest {

@Test
fun whenDarkModeAndPrivacyShieldProtectedThenSetDarkShieldAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

Expand All @@ -59,10 +77,12 @@ class LottiePrivacyShieldAnimationHelperTest {

@Test
fun whenLightModeAndPrivacyShieldUnProtectedThenUseLightAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

Expand All @@ -72,10 +92,12 @@ class LottiePrivacyShieldAnimationHelperTest {

@Test
fun whenDarkModeAndPrivacyShieldUnProtectedThenUseDarkAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

Expand All @@ -85,10 +107,12 @@ class LottiePrivacyShieldAnimationHelperTest {

@Test
fun whenLightModeAndPrivacyShieldMaliciousThenUseLightAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, MALICIOUS)

Expand All @@ -98,10 +122,12 @@ class LottiePrivacyShieldAnimationHelperTest {

@Test
fun whenDarkModeAndPrivacyShieldMaliciousThenUseDarkAnimation() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, MALICIOUS)

Expand All @@ -118,7 +144,7 @@ class LottiePrivacyShieldAnimationHelperTest {
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

Expand All @@ -134,7 +160,7 @@ class LottiePrivacyShieldAnimationHelperTest {
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

Expand All @@ -150,7 +176,7 @@ class LottiePrivacyShieldAnimationHelperTest {
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

Expand All @@ -166,7 +192,7 @@ class LottiePrivacyShieldAnimationHelperTest {
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

Expand All @@ -182,7 +208,102 @@ class LottiePrivacyShieldAnimationHelperTest {
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment)
val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

verify(holder).setAnimation(R.raw.protected_shield)
}

@SuppressLint("DenyListedApi")
@Test
fun whenLightModeAndProtectedAndSelfEnabledAndShouldShowNewVisualDesignShieldThenUseExperimentAssets() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
enabledVisualExperimentStateFlow,
)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

verify(holder).setAnimation(R.raw.protected_shield_visual_updates)
}

@SuppressLint("DenyListedApi")
@Test
fun whenLightModeAndUnprotectedAndSelfEnabledAndShouldShowNewVisualDesignShieldThenUseExperimentAssets() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
enabledVisualExperimentStateFlow,
)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

verify(holder).setAnimation(R.raw.unprotected_shield_visual_updates)
}

@SuppressLint("DenyListedApi")
@Test
fun whenDarkModeAndProtectedAndSelfEnabledAndShouldShowNewVisualDesignShieldThenUseExperimentAssets() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
enabledVisualExperimentStateFlow,
)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

verify(holder).setAnimation(R.raw.dark_protected_shield_visual_updates)
}

@SuppressLint("DenyListedApi")
@Test
fun whenDarkModeAndUnprotectedAndSelfEnabledAndShouldShowNewVisualDesignShieldThenUseExperimentAssets() {
whenever(senseOfProtectionExperiment.shouldShowNewPrivacyShield()).thenReturn(false)
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
enabledVisualExperimentStateFlow,
)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(false)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, UNPROTECTED)

verify(holder).setAnimation(R.raw.dark_unprotected_shield_visual_updates)
}

@SuppressLint("DenyListedApi")
@Test
fun whenLightModeAndProtectedAndSelfEnabledAndShouldShowNotNewVisualDesignShieldThenUseNonExperimentAssets() {
whenever(senseOfProtectionExperiment.isUserEnrolledInAVariantAndExperimentEnabled()).thenReturn(false)
whenever(visualDesignExperimentDataStore.experimentState).thenReturn(
disabledVisualExperimentStateFlow,
)

val holder: LottieAnimationView = mock()
val appTheme: AppTheme = mock()
whenever(appTheme.isLightModeEnabled()).thenReturn(true)

val testee = LottiePrivacyShieldAnimationHelper(appTheme, senseOfProtectionExperiment, visualDesignExperimentDataStore)

testee.setAnimationView(holder, PROTECTED)

Expand Down
33 changes: 27 additions & 6 deletions app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.Command.StartCo
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.Command.StartExperimentVariant1Animation
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.Command.StartExperimentVariant2OrVariant3Animation
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.Command.StartTrackersAnimation
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.Command.StartVisualDesignTrackersAnimation
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.LeadingIconState.PRIVACY_SHIELD
import com.duckduckgo.app.browser.omnibar.OmnibarLayoutViewModel.ViewState
import com.duckduckgo.app.browser.omnibar.animations.BrowserTrackersAnimatorHelper
Expand Down Expand Up @@ -496,7 +497,7 @@ open class OmnibarLayout @JvmOverloads constructor(
}

if (viewState.leadingIconState == PRIVACY_SHIELD) {
renderPrivacyShield(viewState.privacyShield, viewState.viewMode)
renderPrivacyShield(viewState.privacyShield, viewState.viewMode, viewState.isVisualDesignExperimentEnabled)
} else {
lastSeenPrivacyShield = null
}
Expand Down Expand Up @@ -526,6 +527,10 @@ open class OmnibarLayout @JvmOverloads constructor(
moveCaretToFront()
}

is StartVisualDesignTrackersAnimation -> {
startVisualDesignTrackersAnimation(command.entities)
}

is StartExperimentVariant1Animation -> {
startExperimentVariant1Animation()
}
Expand All @@ -549,8 +554,8 @@ open class OmnibarLayout @JvmOverloads constructor(
}
}

private fun renderLeadingIconState(iconState: OmnibarLayoutViewModel.LeadingIconState) {
when (iconState) {
private fun renderLeadingIconState(viewState: ViewState) {
when (viewState.leadingIconState) {
OmnibarLayoutViewModel.LeadingIconState.SEARCH -> {
searchIcon.show()
shieldIcon.gone()
Expand All @@ -561,7 +566,7 @@ open class OmnibarLayout @JvmOverloads constructor(
}

OmnibarLayoutViewModel.LeadingIconState.PRIVACY_SHIELD -> {
if (senseOfProtectionExperiment.shouldShowNewPrivacyShield()) {
if (shouldShowUpdatedPrivacyShield(viewState.isVisualDesignExperimentEnabled)) {
shieldIcon.gone()
shieldIconExperiment.show()
} else {
Expand Down Expand Up @@ -603,6 +608,10 @@ open class OmnibarLayout @JvmOverloads constructor(
}
}

private fun shouldShowUpdatedPrivacyShield(navigationBarEnabled: Boolean): Boolean {
return senseOfProtectionExperiment.shouldShowNewPrivacyShield() || navigationBarEnabled
}
Comment on lines +611 to +613
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of logic like this baked into the view (mainly for clarity and ease of testing reasons) - we make one if-check here and then another in BrowserLottieTrackersAnimatorHelper.kt to finally select the animation.

What do you think about turning OmnibarLayoutViewModel.LeadingIconState into a sealed class? That way, the view model could provide something like OmnibarLayoutViewModel.LeadingIconState.PRIVACY_SHIELD.resourceType, where resourceType defines which animation to use (out of production, sense of protection, or the new visual update).

I'm not going to block the PR over this, but if you agree this is a worthwhile improvement, it’d be great to incorporate here or as a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t like this logic either, and ideally we’d refactor BrowserLottieTrackersAnimatorHelper.kt but that’s not something I want to do now.

The problem is that now we have 3 different tracker animations and 2 of them have different layouts. This is far from optimal, but it solves the issue now knowing that this will be cleaned up once the experiments finish.


open fun renderButtons(viewState: ViewState) {
val newTransitionState = TransitionState(
showClearButton = viewState.showClearButton,
Expand Down Expand Up @@ -674,7 +683,7 @@ open class OmnibarLayout @JvmOverloads constructor(
renderTabIcon(viewState)
renderPulseAnimation(viewState)

renderLeadingIconState(viewState.leadingIconState)
renderLeadingIconState(viewState)

renderHint(viewState)
}
Expand Down Expand Up @@ -832,6 +841,17 @@ open class OmnibarLayout @JvmOverloads constructor(
)
}

private fun startVisualDesignTrackersAnimation(events: List<Entity>?) {
animatorHelper.startTrackersAnimation(
context = context,
shieldAnimationView = shieldIconExperiment,
trackersAnimationView = trackersAnimation,
omnibarViews = omnibarViews(),
entities = events,
visualDesignExperimentEnabled = true,
)
}

private fun startExperimentVariant1Animation() {
if (this::animatorHelper.isInitialized) {
animatorHelper.startExperimentVariant1Animation(
Expand Down Expand Up @@ -862,11 +882,12 @@ open class OmnibarLayout @JvmOverloads constructor(
private fun renderPrivacyShield(
privacyShield: PrivacyShield,
viewMode: ViewMode,
navigationBarEnabled: Boolean,
) {
renderIfChanged(privacyShield, lastSeenPrivacyShield) {
lastSeenPrivacyShield = privacyShield
val shieldIconView = if (viewMode is ViewMode.Browser) {
val isExperimentEnabled = senseOfProtectionExperiment.shouldShowNewPrivacyShield()
val isExperimentEnabled = shouldShowUpdatedPrivacyShield(navigationBarEnabled)
if (isExperimentEnabled) shieldIconExperiment else shieldIcon
} else {
customTabToolbarContainer.customTabShieldIcon
Expand Down
Loading
Loading