Skip to content

Commit c457d58

Browse files
authored
fix onboarding privacy shield and fire button highlights (#5963)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1208671518894266/task/1210067401123378 ### Description This PR fixes the position and clipping of the privacy shield highlight (pulsating animation) during onboarding. The class responsible for the animation (`PulseAnimation`) makes _a lot_ of assumptions about the target view hierarchy into which it injects an animated image view. I made the simplest fix possible to adjust the animated circle's position while fitting these assumptions but the solution comes with some hardcoded values. We should separately consider revisiting how the `PulseAnimation` class is built to allow more flexibility. Additionally, to fix the clipping I moved the animation outside of the omnibar card. Even though the `PulseAnimation` forcefully disables clipping for all parents within the view hierarchy (another assumption), the `MaterialCardView` ignores these calls and will still clip the views. This is justified in the impl by the fact that cards have elevation, etc. I found some ways to disable clipping for the card: ``` app:cardPreventCornerOverlap="false" app:cardUseCompatPadding="true" android:clipChildren="false" android:clipToPadding="false" android:outlineProvider="none" ``` but I don't like hacking around the card's functionality and relying on some compat properties to maintain things like shadows. Also in this PR I'm adding a pulse animation to the fire button found in the bottom navigation bar.
1 parent 48594b5 commit c457d58

File tree

5 files changed

+70
-9
lines changed

5 files changed

+70
-9
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4111,6 +4111,7 @@ class BrowserTabFragment :
41114111
}
41124112

41134113
omnibar.renderBrowserViewState(viewState)
4114+
browserNavigationBarIntegration.configureFireButtonHighlight(highlighted = viewState.fireButton.isHighlighted())
41144115
if (omnibar.isPulseAnimationPlaying()) {
41154116
webView?.setBottomMatchingBehaviourEnabled(true) // only execute if animation is playing
41164117
}

app/src/main/java/com/duckduckgo/app/browser/navigation/bar/BrowserNavigationBarViewIntegration.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ class BrowserNavigationBarViewIntegration(
8080
navigationBarView.setViewMode(NewTab)
8181
}
8282

83+
fun configureFireButtonHighlight(highlighted: Boolean) {
84+
navigationBarView.setFireButtonHighlight(highlighted)
85+
}
86+
8387
fun onDestroyView() {
8488
stateObserverJob?.cancel()
8589
onDisabled()

app/src/main/java/com/duckduckgo/app/browser/navigation/bar/view/BrowserNavigationBarView.kt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,15 @@ import androidx.coordinatorlayout.widget.CoordinatorLayout
2424
import androidx.coordinatorlayout.widget.CoordinatorLayout.AttachedBehavior
2525
import androidx.coordinatorlayout.widget.CoordinatorLayout.Behavior
2626
import androidx.core.view.doOnAttach
27+
import androidx.core.view.doOnLayout
2728
import androidx.core.view.isVisible
29+
import androidx.lifecycle.LifecycleOwner
2830
import androidx.lifecycle.ViewModelProvider
2931
import androidx.lifecycle.findViewTreeLifecycleOwner
3032
import androidx.lifecycle.findViewTreeViewModelStoreOwner
3133
import androidx.lifecycle.lifecycleScope
3234
import com.duckduckgo.anvil.annotations.InjectWith
35+
import com.duckduckgo.app.browser.PulseAnimation
3336
import com.duckduckgo.app.browser.databinding.ViewBrowserNavigationBarBinding
3437
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarViewModel.Command
3538
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarViewModel.Command.NotifyAutofillButtonClicked
@@ -94,6 +97,14 @@ class BrowserNavigationBarView @JvmOverloads constructor(
9497
private var conflatedCommandsJob: ConflatedJob = ConflatedJob()
9598
private var conflatedStateJob: ConflatedJob = ConflatedJob()
9699

100+
private val lifecycleOwner: LifecycleOwner by lazy {
101+
requireNotNull(findViewTreeLifecycleOwner())
102+
}
103+
104+
private val pulseAnimation: PulseAnimation by lazy {
105+
PulseAnimation(lifecycleOwner)
106+
}
107+
97108
val popupMenuAnchor: View = binding.menuButton
98109

99110
var browserNavigationBarObserver: BrowserNavigationBarObserver? = null
@@ -110,6 +121,12 @@ class BrowserNavigationBarView @JvmOverloads constructor(
110121
}
111122
}
112123

124+
fun setFireButtonHighlight(highlighted: Boolean) {
125+
doOnAttach {
126+
viewModel.setFireButtonHighlight(highlighted)
127+
}
128+
}
129+
113130
override fun onAttachedToWindow() {
114131
AndroidSupportInjection.inject(this)
115132
super.onAttachedToWindow()
@@ -175,6 +192,8 @@ class BrowserNavigationBarView @JvmOverloads constructor(
175192
binding.bookmarksButton.isVisible = viewState.bookmarksButtonVisible
176193
binding.fireButton.isVisible = viewState.fireButtonVisible
177194
binding.tabsButton.isVisible = viewState.tabsButtonVisible
195+
196+
renderFireButtonPulseAnimation(enabled = viewState.fireButtonHighlighted)
178197
}
179198

180199
private fun processCommands(command: Command) {
@@ -192,6 +211,18 @@ class BrowserNavigationBarView @JvmOverloads constructor(
192211
}
193212
}
194213

214+
private fun renderFireButtonPulseAnimation(enabled: Boolean) {
215+
if (enabled) {
216+
if (!pulseAnimation.isActive) {
217+
doOnLayout {
218+
pulseAnimation.playOn(binding.fireIconImageView, isExperimentAndShieldView = false)
219+
}
220+
}
221+
} else {
222+
pulseAnimation.stop()
223+
}
224+
}
225+
195226
enum class ViewMode {
196227
NewTab,
197228
Browser,

app/src/main/java/com/duckduckgo/app/browser/navigation/bar/view/BrowserNavigationBarViewModel.kt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,14 @@ class BrowserNavigationBarViewModel @Inject constructor(
130130
}
131131
}
132132

133+
fun setFireButtonHighlight(highlighted: Boolean) {
134+
_viewState.update {
135+
it.copy(
136+
fireButtonHighlighted = highlighted,
137+
)
138+
}
139+
}
140+
133141
sealed class Command {
134142
data object NotifyFireButtonClicked : Command()
135143
data object NotifyTabsButtonClicked : Command()
@@ -149,6 +157,7 @@ class BrowserNavigationBarViewModel @Inject constructor(
149157
val autofillButtonVisible: Boolean = true,
150158
val bookmarksButtonVisible: Boolean = true,
151159
val fireButtonVisible: Boolean = true,
160+
val fireButtonHighlighted: Boolean = false,
152161
val tabsButtonVisible: Boolean = true,
153162
val tabsCount: Int = 0,
154163
val shouldUpdateTabsCount: Boolean = false,

app/src/main/res/layout/view_fade_omnibar.xml

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,6 @@
180180
<include layout="@layout/cookie_scene_1" />
181181
</FrameLayout>
182182

183-
<!-- Placeholder should have same size a sibling ImageViews. size = image_width + padding -->
184-
<View
185-
android:id="@+id/placeholder"
186-
android:layout_width="32dp"
187-
android:layout_height="32dp"
188-
android:layout_gravity="center"
189-
android:layout_marginEnd="@dimen/keyline_1"
190-
android:visibility="invisible" />
191-
192183
</FrameLayout>
193184

194185
<View
@@ -325,6 +316,31 @@
325316

326317
</androidx.appcompat.widget.Toolbar>
327318

319+
<FrameLayout
320+
android:layout_width="76dp"
321+
android:layout_height="match_parent"
322+
android:paddingTop="@dimen/experimentalOmnibarCardMarginTop"
323+
android:paddingBottom="@dimen/experimentalOmnibarCardMarginBottom"
324+
app:layout_constraintBottom_toBottomOf="parent"
325+
app:layout_constraintStart_toStartOf="parent"
326+
app:layout_constraintTop_toTopOf="parent"
327+
tools:documentation="The values of layout width and paddings is tweaked manually to position the pulsing circle
328+
precisely on top of the lottie-animated shield, keeping in mind that the added animated ImageView will use Gravity.CENTER.">
329+
330+
<View
331+
android:id="@+id/placeholder"
332+
android:layout_width="28dp"
333+
android:layout_height="28dp"
334+
android:layout_gravity="center"
335+
android:visibility="invisible"
336+
tools:documentation="This view is a reference for the PulseAnimation.kt.
337+
PulseAnimation class will add a new ImageView to the parent layout, with a Gravity.CENTER, and size equal to the size of this view.
338+
However, the animator will scale the resulting ImageView from 0.95x to 2.0x size of this view.
339+
Keep this in mind when considering clipping - the PulseAnimation will try to disable clipping for all parents in the hierarchy,
340+
but you might start running against views that always have clipping enabled (like material cards), or system UI." />
341+
342+
</FrameLayout>
343+
328344
<androidx.constraintlayout.widget.ConstraintLayout
329345
android:id="@+id/customTabToolbarContainerWrapper"
330346
android:layout_width="0dp"

0 commit comments

Comments
 (0)