From b8468ee6802504f6a804d1c6960d9efe8dfdbbdf Mon Sep 17 00:00:00 2001 From: Aaditya Dhingra Date: Tue, 12 Nov 2024 14:13:47 +0000 Subject: [PATCH] Bug 1910687 - The long press CFR should point at the back button. r=android-reviewers,skhan Differential Revision: https://phabricator.services.mozilla.com/D228068 --- .../main/java/mozilla/components/compose/cfr/CFRPopup.kt | 7 +++++++ .../components/compose/cfr/CFRPopupFullscreenLayout.kt | 9 +++++++++ .../org/mozilla/fenix/browser/BaseBrowserFragment.kt | 3 ++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopup.kt b/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopup.kt index 587ac0dace22..f0195a997df9 100644 --- a/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopup.kt +++ b/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopup.kt @@ -38,6 +38,7 @@ import java.lang.ref.WeakReference * @property indicatorArrowStartOffset Maximum distance between the popup start and the indicator arrow. * If there isn't enough space this could automatically be overridden up to 0 such that * the indicator arrow will be pointing to the middle of the anchor. + * @property popupStartOffset Maximum distance between the popup and anchor start. */ data class CFRPopupProperties( val popupWidth: Dp = CFRPopup.DEFAULT_WIDTH.dp, @@ -51,6 +52,7 @@ data class CFRPopupProperties( val overlapAnchor: Boolean = false, val indicatorDirection: IndicatorDirection = IndicatorDirection.UP, val indicatorArrowStartOffset: Dp = CFRPopup.DEFAULT_INDICATOR_START_OFFSET.dp, + val popupStartOffset: Dp = CFRPopup.DEFAULT_EXTRA_HORIZONTAL_PADDING.dp, ) /** @@ -144,6 +146,11 @@ class CFRPopup( */ BODY_TO_ANCHOR_START, + /** + * The popup body will be shown aligned to exactly the anchor start with offset. + */ + BODY_TO_ANCHOR_START_WITH_OFFSET, + /** * The popup will be aligned such that the indicator arrow will point to exactly the middle of the anchor. * Recommended to be used when there are multiple widgets displayed horizontally so that this will allow diff --git a/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopupFullscreenLayout.kt b/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopupFullscreenLayout.kt index adf37d57bdd3..0700e631dafc 100644 --- a/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopupFullscreenLayout.kt +++ b/mobile/android/android-components/components/compose/cfr/src/main/java/mozilla/components/compose/cfr/CFRPopupFullscreenLayout.kt @@ -40,6 +40,7 @@ import mozilla.components.compose.cfr.CFRPopup.IndicatorDirection.UP import mozilla.components.compose.cfr.CFRPopup.PopupAlignment.BODY_CENTERED_IN_SCREEN import mozilla.components.compose.cfr.CFRPopup.PopupAlignment.BODY_TO_ANCHOR_CENTER import mozilla.components.compose.cfr.CFRPopup.PopupAlignment.BODY_TO_ANCHOR_START +import mozilla.components.compose.cfr.CFRPopup.PopupAlignment.BODY_TO_ANCHOR_START_WITH_OFFSET import mozilla.components.compose.cfr.CFRPopup.PopupAlignment.INDICATOR_CENTERED_IN_ANCHOR import mozilla.components.compose.cfr.CFRPopupShape.Companion import mozilla.components.compose.cfr.helper.DisplayOrientationListener @@ -313,6 +314,10 @@ internal class CFRPopupFullscreenLayout( Pixels(anchor.x.roundToInt() + leftInsets.value) } + BODY_TO_ANCHOR_START_WITH_OFFSET -> { + Pixels(anchor.x.roundToInt() + leftInsets.value + properties.popupStartOffset.toPx()) + } + BODY_TO_ANCHOR_CENTER -> { Pixels( anchor.x.roundToInt() @@ -394,6 +399,9 @@ internal class CFRPopupFullscreenLayout( BODY_TO_ANCHOR_START -> { Pixels(anchor.x.roundToInt() + anchor.width + leftInsets.value) } + BODY_TO_ANCHOR_START_WITH_OFFSET -> { + Pixels(anchor.x.roundToInt() + anchor.width + leftInsets.value + properties.popupStartOffset.toPx()) + } BODY_TO_ANCHOR_CENTER -> { val anchorEndCoord = anchor.x.roundToInt() + anchor.width Pixels( @@ -477,6 +485,7 @@ internal class CFRPopupFullscreenLayout( ): Pixels { return when (properties.popupAlignment) { BODY_TO_ANCHOR_START, + BODY_TO_ANCHOR_START_WITH_OFFSET, BODY_TO_ANCHOR_CENTER, -> Pixels(properties.indicatorArrowStartOffset.toPx()) BODY_CENTERED_IN_SCREEN, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 6a6fc4d2e9f4..a3ca89e993a7 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -241,7 +241,7 @@ import mozilla.components.ui.widgets.behavior.ToolbarPosition as OldToolbarPosit import org.mozilla.fenix.GleanMetrics.TabStrip as TabStripMetrics private const val NAVIGATION_CFR_VERTICAL_OFFSET = 10 -private const val NAVIGATION_CFR_ARROW_OFFSET = 48 +private const val NAVIGATION_CFR_ARROW_OFFSET = 24 private const val NAVIGATION_CFR_MAX_MS_BETWEEN_CLICKS = 5000 /** @@ -1586,6 +1586,7 @@ abstract class BaseBrowserFragment : indicatorDirection = CFRPopup.IndicatorDirection.DOWN, popupVerticalOffset = NAVIGATION_CFR_VERTICAL_OFFSET.dp, indicatorArrowStartOffset = NAVIGATION_CFR_ARROW_OFFSET.dp, + popupAlignment = CFRPopup.PopupAlignment.BODY_TO_ANCHOR_START_WITH_OFFSET, ), onCFRShown = { NavigationBar.navigationButtonsCfrShown.record(NoExtras())