From 57598db48683b70aedf9a645d9cb91c783a64adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Thu, 10 Oct 2024 19:15:09 +0200 Subject: [PATCH] Omnibar: safe calling Android methods (#5117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1174433894299346/1208502796109620/f ### Description This PR adds a safe way to call methods in the `LegacyOmnibarView`. This is needed because the way we interact with the Omnibar doesn’t take into account that Dagger might not have injected yet. ### Steps to test this PR Smoke tests and QA --- .../app/browser/omnibar/LegacyOmnibarView.kt | 110 +++++++++++------- .../main/res/layout/view_legacy_omnibar.xml | 1 + .../res/layout/view_legacy_omnibar_bottom.xml | 1 + 3 files changed, 72 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/LegacyOmnibarView.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/LegacyOmnibarView.kt index 89b3906a8383..225b771b453f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/LegacyOmnibarView.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/LegacyOmnibarView.kt @@ -138,6 +138,8 @@ class LegacyOmnibarView @JvmOverloads constructor( internal val trackersAnimation: LottieAnimationView by lazy { findViewById(R.id.trackersAnimation) } internal val duckPlayerIcon: ImageView by lazy { findViewById(R.id.duckPlayerIcon) } + private var privacyShield: PrivacyShield? = null + init { val attr = context.theme.obtainStyledAttributes(attrs, R.styleable.LegacyOmnibarView, defStyle, 0) omnibarPosition = OmnibarPosition.entries[attr.getInt(R.styleable.LegacyOmnibarView_omnibarPosition, 0)] @@ -163,12 +165,20 @@ class LegacyOmnibarView @JvmOverloads constructor( super.onAttachedToWindow() pulseAnimation = PulseAnimation(findViewTreeLifecycleOwner()!!) + + val deferredPrivacyShield = privacyShield + if (deferredPrivacyShield != null) { + setPrivacyShield(false, privacyShield = deferredPrivacyShield) + privacyShield = null + } } override fun setExpanded(expanded: Boolean) { - when (omnibarPosition) { - OmnibarPosition.TOP -> super.setExpanded(expanded) - OmnibarPosition.BOTTOM -> (behavior as BottomAppBarBehavior).animateToolbarVisibility(expanded) + safeCall { + when (omnibarPosition) { + OmnibarPosition.TOP -> super.setExpanded(expanded) + OmnibarPosition.BOTTOM -> (behavior as BottomAppBarBehavior).animateToolbarVisibility(expanded) + } } } @@ -176,9 +186,11 @@ class LegacyOmnibarView @JvmOverloads constructor( expanded: Boolean, animate: Boolean, ) { - when (omnibarPosition) { - OmnibarPosition.TOP -> super.setExpanded(expanded, animate) - OmnibarPosition.BOTTOM -> (behavior as BottomAppBarBehavior).animateToolbarVisibility(expanded) + safeCall { + when (omnibarPosition) { + OmnibarPosition.TOP -> super.setExpanded(expanded, animate) + OmnibarPosition.BOTTOM -> (behavior as BottomAppBarBehavior).animateToolbarVisibility(expanded) + } } } @@ -193,45 +205,59 @@ class LegacyOmnibarView @JvmOverloads constructor( isCustomTab: Boolean, privacyShield: PrivacyShield, ) { - val animationViewHolder = if (isCustomTab) { - customTabToolbarContainer.customTabShieldIcon - } else { - shieldIcon + if (!isAttachedToWindow) { + this.privacyShield = privacyShield + } + + safeCall { + val animationViewHolder = if (isCustomTab) { + customTabToolbarContainer.customTabShieldIcon + } else { + shieldIcon + } + privacyShieldView.setAnimationView(animationViewHolder, privacyShield) + cancelTrackersAnimation() + } + } + + private fun safeCall(call: () -> Unit) { + if (isAttachedToWindow) { + call() } - privacyShieldView.setAnimationView(animationViewHolder, privacyShield) - cancelTrackersAnimation() } fun renderBrowserViewState( viewState: BrowserViewState, tabDisplayedInCustomTabScreen: Boolean, ) { - if (viewState.browserShowing) { - daxIcon.isVisible = viewState.showDaxIcon - duckPlayerIcon.isVisible = viewState.showDuckPlayerIcon - shieldIcon.isInvisible = - !viewState.showPrivacyShield.isEnabled() || viewState.showDaxIcon || viewState.showDuckPlayerIcon - clearTextButton.isVisible = viewState.showClearButton - searchIcon.isVisible = viewState.showSearchIcon - } else { - daxIcon.isVisible = false - duckPlayerIcon.isVisible = false - shieldIcon.isVisible = false - clearTextButton.isVisible = viewState.showClearButton - searchIcon.isVisible = true - } + safeCall { + if (viewState.browserShowing) { + daxIcon.isVisible = viewState.showDaxIcon + duckPlayerIcon.isVisible = viewState.showDuckPlayerIcon + shieldIcon.isInvisible = + !viewState.showPrivacyShield.isEnabled() || viewState.showDaxIcon || viewState.showDuckPlayerIcon + clearTextButton.isVisible = viewState.showClearButton + searchIcon.isVisible = viewState.showSearchIcon + } else { + daxIcon.isVisible = false + duckPlayerIcon.isVisible = false + shieldIcon.isVisible = false + clearTextButton.isVisible = viewState.showClearButton + searchIcon.isVisible = true + } - tabsMenu.isVisible = viewState.showTabsButton && !tabDisplayedInCustomTabScreen - fireIconMenu.isVisible = viewState.fireButton is HighlightableButton.Visible && !tabDisplayedInCustomTabScreen - browserMenu.isVisible = viewState.showMenuButton is HighlightableButton.Visible + tabsMenu.isVisible = viewState.showTabsButton && !tabDisplayedInCustomTabScreen + fireIconMenu.isVisible = viewState.fireButton is HighlightableButton.Visible && !tabDisplayedInCustomTabScreen + browserMenu.isVisible = viewState.showMenuButton is HighlightableButton.Visible - spacer.isVisible = viewState.showVoiceSearch && viewState.showClearButton + spacer.isVisible = viewState.showVoiceSearch && viewState.showClearButton - renderPulseAnimation(viewState) + renderPulseAnimation(viewState) + } } fun setScrollingEnabled(enabled: Boolean) { - if (isAttachedToWindow) { + safeCall { if (enabled) { omnibarScrolling.enableOmnibarScrolling(toolbarContainer) } else { @@ -297,19 +323,23 @@ class LegacyOmnibarView @JvmOverloads constructor( } fun startTrackersAnimation(events: List?) { - animatorHelper.startTrackersAnimation( - context = context, - shieldAnimationView = shieldIcon, - trackersAnimationView = trackersAnimation, - omnibarViews = omnibarViews(), - entities = events, - ) + if (this::animatorHelper.isInitialized) { + animatorHelper.startTrackersAnimation( + context = context, + shieldAnimationView = shieldIcon, + trackersAnimationView = trackersAnimation, + omnibarViews = omnibarViews(), + entities = events, + ) + } } fun onNewProgress( newProgress: Int, onAnimationEnd: (Animator?) -> Unit, ) { - smoothProgressAnimator.onNewProgress(newProgress, onAnimationEnd) + safeCall { + smoothProgressAnimator.onNewProgress(newProgress, onAnimationEnd) + } } } diff --git a/app/src/main/res/layout/view_legacy_omnibar.xml b/app/src/main/res/layout/view_legacy_omnibar.xml index 28f0854bf0ec..591e11873a70 100644 --- a/app/src/main/res/layout/view_legacy_omnibar.xml +++ b/app/src/main/res/layout/view_legacy_omnibar.xml @@ -116,6 +116,7 @@ android:gravity="center" android:importantForAccessibility="no" android:padding="6dp" + android:visibility="gone" android:src="@drawable/ic_find_search_20_a05" />