From 8f5a37733dbc3cff33a242cdeefd2f68be92d7c8 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Wed, 22 Jul 2020 19:23:38 -0700 Subject: [PATCH] Create ToolbarPosition enum (#12747) --- .../fenix/browser/BaseBrowserFragment.kt | 13 ++-- .../mozilla/fenix/browser/BrowserAnimator.kt | 16 ++-- .../org/mozilla/fenix/cfr/SearchWidgetCFR.kt | 14 ++-- .../components/metrics/GleanMetricsService.kt | 8 +- .../components/toolbar/BrowserToolbarView.kt | 70 ++++++++--------- .../toolbar/TabCounterToolbarButton.kt | 7 +- .../components/toolbar/ToolbarPosition.kt | 19 +++++ .../org/mozilla/fenix/home/HomeFragment.kt | 78 +++++++++---------- ...boardingToolbarPositionPickerViewHolder.kt | 8 +- .../fenix/settings/CustomizationFragment.kt | 6 +- .../TrackingProtectionOverlay.kt | 14 ++-- .../java/org/mozilla/fenix/utils/Settings.kt | 4 + ...dingToolbarPositionPickerViewHolderTest.kt | 5 +- 13 files changed, 141 insertions(+), 121 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarPosition.kt diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 0c5387476..98f0fc53e 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -93,6 +93,7 @@ import org.mozilla.fenix.components.toolbar.BrowserToolbarViewInteractor import org.mozilla.fenix.components.toolbar.DefaultBrowserToolbarController import org.mozilla.fenix.components.toolbar.SwipeRefreshScrollingViewBehavior import org.mozilla.fenix.components.toolbar.ToolbarIntegration +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.downloads.DynamicDownloadDialog import org.mozilla.fenix.ext.components @@ -264,7 +265,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session _browserToolbarView = BrowserToolbarView( container = view.browserLayout, - shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar, + toolbarPosition = context.settings().toolbarPosition, interactor = browserInteractor, customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) }, lifecycleOwner = viewLifecycleOwner @@ -677,10 +678,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session private fun initializeEngineView(toolbarHeight: Int) { engineView.setDynamicToolbarMaxHeight(toolbarHeight) - val behavior = if (requireContext().settings().shouldUseBottomToolbar) { - EngineViewBottomBehavior(context, null) - } else { - SwipeRefreshScrollingViewBehavior(requireContext(), null, engineView, browserToolbarView) + val context = requireContext() + val behavior = when (context.settings().toolbarPosition) { + ToolbarPosition.BOTTOM -> EngineViewBottomBehavior(context, null) + ToolbarPosition.TOP -> SwipeRefreshScrollingViewBehavior(context, null, engineView, browserToolbarView) } (swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams).behavior = behavior @@ -843,7 +844,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session * Returns the layout [android.view.Gravity] for the quick settings and ETP dialog. */ protected fun getAppropriateLayoutGravity(): Int = - if (context?.settings()?.shouldUseBottomToolbar == true) Gravity.BOTTOM else Gravity.TOP + context?.settings()?.toolbarPosition?.androidGravity ?: Gravity.BOTTOM /** * Updates the site permissions rules based on user settings. diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt index 5bb2fc83f..02a94f2d1 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt @@ -21,6 +21,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import mozilla.components.concept.engine.EngineView import org.mozilla.fenix.R +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.settings import java.lang.ref.WeakReference @@ -155,12 +156,15 @@ class BrowserAnimator( fun getToolbarNavOptions(context: Context): NavOptions { val navOptions = NavOptions.Builder() - if (!context.settings().shouldUseBottomToolbar) { - navOptions.setEnterAnim(R.anim.fade_in) - navOptions.setExitAnim(R.anim.fade_out) - } else { - navOptions.setEnterAnim(R.anim.fade_in_up) - navOptions.setExitAnim(R.anim.fade_out_down) + when (context.settings().toolbarPosition) { + ToolbarPosition.TOP -> { + navOptions.setEnterAnim(R.anim.fade_in) + navOptions.setExitAnim(R.anim.fade_out) + } + ToolbarPosition.BOTTOM -> { + navOptions.setEnterAnim(R.anim.fade_in_up) + navOptions.setExitAnim(R.anim.fade_out_down) + } } return navOptions.build() diff --git a/app/src/main/java/org/mozilla/fenix/cfr/SearchWidgetCFR.kt b/app/src/main/java/org/mozilla/fenix/cfr/SearchWidgetCFR.kt index 1bbe94e05..07d017ce0 100644 --- a/app/src/main/java/org/mozilla/fenix/cfr/SearchWidgetCFR.kt +++ b/app/src/main/java/org/mozilla/fenix/cfr/SearchWidgetCFR.kt @@ -11,7 +11,6 @@ import android.graphics.drawable.ColorDrawable import android.view.Gravity import android.view.LayoutInflater import android.view.View -import androidx.core.view.isGone import androidx.core.view.isVisible import androidx.core.view.marginTop import kotlinx.android.synthetic.main.search_widget_cfr.view.* @@ -21,6 +20,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.SearchWidgetCreator import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.utils.Settings /** @@ -50,18 +50,14 @@ class SearchWidgetCFR( val searchWidgetCFRDialog = Dialog(context) val layout = LayoutInflater.from(context) .inflate(R.layout.search_widget_cfr, null) - val isBottomToolbar = settings.shouldUseBottomToolbar + val toolbarPosition = settings.toolbarPosition - layout.drop_down_triangle.isGone = isBottomToolbar - layout.pop_up_triangle.isVisible = isBottomToolbar + layout.drop_down_triangle.isVisible = toolbarPosition == ToolbarPosition.TOP + layout.pop_up_triangle.isVisible = toolbarPosition == ToolbarPosition.BOTTOM val toolbar = getToolbar() - val gravity = if (isBottomToolbar) { - Gravity.CENTER_HORIZONTAL or Gravity.BOTTOM - } else { - Gravity.CENTER_HORIZONTAL or Gravity.TOP - } + val gravity = Gravity.CENTER_HORIZONTAL or toolbarPosition.androidGravity layout.cfr_neg_button.setOnClickListener { metrics.track(Event.SearchWidgetCFRNotNowPressed) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 677e0aba7..4bf89abbd 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -50,6 +50,7 @@ import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.GleanMetrics.TrackingProtection import org.mozilla.fenix.GleanMetrics.UserSpecifiedSearchEngines import org.mozilla.fenix.GleanMetrics.VoiceSearch +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.utils.BrowsersCache @@ -724,10 +725,9 @@ class GleanMetricsService(private val context: Context) : MetricsService { } toolbarPosition.set( - if (context.settings().shouldUseBottomToolbar) { - Event.ToolbarPositionChanged.Position.BOTTOM.name - } else { - Event.ToolbarPositionChanged.Position.TOP.name + when (context.settings().toolbarPosition) { + ToolbarPosition.BOTTOM -> Event.ToolbarPositionChanged.Position.BOTTOM.name + ToolbarPosition.TOP -> Event.ToolbarPositionChanged.Position.TOP.name } ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt index c5011f1e5..d725a9f3d 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarView.kt @@ -17,6 +17,7 @@ import androidx.annotation.VisibleForTesting import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.core.content.ContextCompat import androidx.core.view.isVisible +import androidx.core.view.updateLayoutParams import androidx.lifecycle.LifecycleOwner import com.google.android.material.appbar.AppBarLayout import com.google.android.material.appbar.AppBarLayout.LayoutParams.SCROLL_FLAG_ENTER_ALWAYS @@ -59,7 +60,7 @@ interface BrowserToolbarViewInteractor { @SuppressWarnings("LargeClass") class BrowserToolbarView( private val container: ViewGroup, - private val shouldUseBottomToolbar: Boolean, + private val toolbarPosition: ToolbarPosition, private val interactor: BrowserToolbarViewInteractor, private val customTabSession: Session?, private val lifecycleOwner: LifecycleOwner @@ -71,9 +72,9 @@ class BrowserToolbarView( private val settings = container.context.settings() @LayoutRes - private val toolbarLayout = when { - settings.shouldUseBottomToolbar -> R.layout.component_bottom_browser_toolbar - else -> R.layout.component_browser_top_toolbar + private val toolbarLayout = when (settings.toolbarPosition) { + ToolbarPosition.BOTTOM -> R.layout.component_bottom_browser_toolbar + ToolbarPosition.TOP -> R.layout.component_browser_top_toolbar } private val layout = LayoutInflater.from(container.context) @@ -144,7 +145,7 @@ class BrowserToolbarView( with(container.context) { val sessionManager = components.core.sessionManager - if (!shouldUseBottomToolbar) { + if (toolbarPosition == ToolbarPosition.TOP) { val offsetChangedListener = AppBarLayout.OnOffsetChangedListener { _: AppBarLayout?, verticalOffset: Int -> interactor.onScrolled(verticalOffset) @@ -167,10 +168,9 @@ class BrowserToolbarView( false } - display.progressGravity = if (shouldUseBottomToolbar) { - DisplayToolbar.Gravity.TOP - } else { - DisplayToolbar.Gravity.BOTTOM + display.progressGravity = when (toolbarPosition) { + ToolbarPosition.BOTTOM -> DisplayToolbar.Gravity.TOP + ToolbarPosition.TOP -> DisplayToolbar.Gravity.BOTTOM } val primaryTextColor = ContextCompat.getColor( @@ -207,7 +207,7 @@ class BrowserToolbarView( this, sessionManager, customTabSession?.id, - shouldReverseItems = !shouldUseBottomToolbar, + shouldReverseItems = toolbarPosition == ToolbarPosition.TOP, onItemTapped = { interactor.onBrowserToolbarMenuItemTapped(it) } @@ -216,7 +216,7 @@ class BrowserToolbarView( menuToolbar = DefaultToolbarMenu( context = this, hasAccountProblem = components.backgroundServices.accountManager.accountNeedsReauth(), - shouldReverseItems = !shouldUseBottomToolbar, + shouldReverseItems = toolbarPosition == ToolbarPosition.TOP, onItemTapped = { interactor.onBrowserToolbarMenuItemTapped(it) }, lifecycleOwner = lifecycleOwner, sessionManager = sessionManager, @@ -254,12 +254,15 @@ class BrowserToolbarView( } fun expand() { - if (settings.shouldUseBottomToolbar) { - (view.layoutParams as CoordinatorLayout.LayoutParams).apply { - (behavior as BrowserToolbarBottomBehavior).forceExpand(view) + when (settings.toolbarPosition) { + ToolbarPosition.BOTTOM -> { + (view.layoutParams as CoordinatorLayout.LayoutParams).apply { + (behavior as BrowserToolbarBottomBehavior).forceExpand(view) + } + } + ToolbarPosition.TOP -> { + layout.app_bar?.setExpanded(true) } - } else if (!settings.shouldUseBottomToolbar) { - layout.app_bar?.setExpanded(true) } } @@ -268,30 +271,27 @@ class BrowserToolbarView( * Note that the bottom toolbar has a feature flag for being dynamic, so it may not get flags set. */ fun setScrollFlags(shouldDisableScroll: Boolean = false) { - if (view.context.settings().shouldUseBottomToolbar) { - if (view.layoutParams is CoordinatorLayout.LayoutParams) { - (view.layoutParams as CoordinatorLayout.LayoutParams).apply { + when (settings.toolbarPosition) { + ToolbarPosition.BOTTOM -> { + (view.layoutParams as? CoordinatorLayout.LayoutParams)?.apply { behavior = BrowserToolbarBottomBehavior(view.context, null) } } - - return - } - - val params = view.layoutParams as AppBarLayout.LayoutParams - - params.scrollFlags = when (view.context.settings().shouldUseFixedTopToolbar || shouldDisableScroll) { - true -> { - // Force expand the toolbar so the user is not stuck with a hidden toolbar - expand() - 0 - } - false -> { - SCROLL_FLAG_SCROLL or SCROLL_FLAG_ENTER_ALWAYS or SCROLL_FLAG_SNAP or SCROLL_FLAG_EXIT_UNTIL_COLLAPSED + ToolbarPosition.TOP -> { + view.updateLayoutParams { + scrollFlags = if (settings.shouldUseFixedTopToolbar || shouldDisableScroll) { + // Force expand the toolbar so the user is not stuck with a hidden toolbar + expand() + 0 + } else { + SCROLL_FLAG_SCROLL or + SCROLL_FLAG_ENTER_ALWAYS or + SCROLL_FLAG_SNAP or + SCROLL_FLAG_EXIT_UNTIL_COLLAPSED + } + } } } - - view.layoutParams = params } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt index f8739c2ad..e282e23a2 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/TabCounterToolbarButton.kt @@ -113,10 +113,9 @@ class TabCounterToolbarButton( ) return BrowserMenuBuilder( - if (context.settings().shouldUseBottomToolbar) { - menuItems.reversed() - } else { - menuItems + when (context.settings().toolbarPosition) { + ToolbarPosition.BOTTOM -> menuItems.reversed() + ToolbarPosition.TOP -> menuItems } ).build(context) } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarPosition.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarPosition.kt new file mode 100644 index 000000000..b69b6fef8 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarPosition.kt @@ -0,0 +1,19 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components.toolbar + +import android.view.Gravity + +/** + * Fenix lets the browser toolbar be placed at either the top or the bottom of the screen. + * This enum represents the posible positions. + * + * @property androidGravity [Gravity] value corresponding to the position. + * Used to position related elements such as a CFR tooltip. + */ +enum class ToolbarPosition(val androidGravity: Int) { + BOTTOM(Gravity.BOTTOM), + TOP(Gravity.TOP) +} diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 0f3d75878..f147c9371 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -83,6 +83,7 @@ import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.tips.FenixTipManager import org.mozilla.fenix.components.tips.providers.MigrationTipProvider +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.metrics @@ -120,12 +121,9 @@ class HomeFragment : Fragment() { } private val snackbarAnchorView: View? - get() { - return if (requireContext().settings().shouldUseBottomToolbar) { - toolbarLayout - } else { - null - } + get() = when (requireContext().settings().toolbarPosition) { + ToolbarPosition.BOTTOM -> toolbarLayout + ToolbarPosition.TOP -> null } private val browsingModeManager get() = (activity as HomeActivity).browsingModeManager @@ -253,45 +251,45 @@ class HomeFragment : Fragment() { } private fun updateLayout(view: View) { - val shouldUseBottomToolbar = view.context.settings().shouldUseBottomToolbar - - if (!shouldUseBottomToolbar) { - view.toolbarLayout.layoutParams = CoordinatorLayout.LayoutParams( - ConstraintLayout.LayoutParams.MATCH_PARENT, - ConstraintLayout.LayoutParams.WRAP_CONTENT - ) - .apply { + when (view.context.settings().toolbarPosition) { + ToolbarPosition.TOP -> { + view.toolbarLayout.layoutParams = CoordinatorLayout.LayoutParams( + ConstraintLayout.LayoutParams.MATCH_PARENT, + ConstraintLayout.LayoutParams.WRAP_CONTENT + ).apply { gravity = Gravity.TOP } - ConstraintSet().apply { - clone(view.toolbarLayout) - clear(view.bottom_bar.id, BOTTOM) - clear(view.bottomBarShadow.id, BOTTOM) - connect(view.bottom_bar.id, TOP, PARENT_ID, TOP) - connect(view.bottomBarShadow.id, TOP, view.bottom_bar.id, BOTTOM) - connect(view.bottomBarShadow.id, BOTTOM, PARENT_ID, BOTTOM) - applyTo(view.toolbarLayout) + ConstraintSet().apply { + clone(view.toolbarLayout) + clear(view.bottom_bar.id, BOTTOM) + clear(view.bottomBarShadow.id, BOTTOM) + connect(view.bottom_bar.id, TOP, PARENT_ID, TOP) + connect(view.bottomBarShadow.id, TOP, view.bottom_bar.id, BOTTOM) + connect(view.bottomBarShadow.id, BOTTOM, PARENT_ID, BOTTOM) + applyTo(view.toolbarLayout) + } + + view.bottom_bar.background = resources.getDrawable( + ThemeManager.resolveAttribute(R.attr.bottomBarBackgroundTop, requireContext()), + null + ) + + view.homeAppBar.updateLayoutParams { + topMargin = HEADER_MARGIN.dpToPx(resources.displayMetrics) + } + + createNewAppBarListener(HEADER_MARGIN.dpToPx(resources.displayMetrics).toFloat()) + view.homeAppBar.addOnOffsetChangedListener( + homeAppBarOffSetListener + ) } - - view.bottom_bar.background = resources.getDrawable( - ThemeManager.resolveAttribute(R.attr.bottomBarBackgroundTop, requireContext()), - null - ) - - view.homeAppBar.updateLayoutParams { - topMargin = HEADER_MARGIN.dpToPx(resources.displayMetrics) + ToolbarPosition.BOTTOM -> { + createNewAppBarListener(0F) + view.homeAppBar.addOnOffsetChangedListener( + homeAppBarOffSetListener + ) } - - createNewAppBarListener(HEADER_MARGIN.dpToPx(resources.displayMetrics).toFloat()) - view.homeAppBar.addOnOffsetChangedListener( - homeAppBarOffSetListener - ) - } else { - createNewAppBarListener(0F) - view.homeAppBar.addOnOffsetChangedListener( - homeAppBarOffSetListener - ) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolder.kt index 87c3453ee..b484a8d3b 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolder.kt @@ -10,6 +10,7 @@ import kotlinx.android.synthetic.main.onboarding_toolbar_position_picker.view.* import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event.OnboardingToolbarPosition.Position +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.asActivity import org.mozilla.fenix.ext.components import org.mozilla.fenix.onboarding.OnboardingRadioButton @@ -29,10 +30,9 @@ class OnboardingToolbarPositionPickerViewHolder(view: View) : RecyclerView.ViewH radioBottomToolbar.addIllustration(view.toolbar_bottom_image) val settings = view.context.components.settings - radio = if (settings.shouldUseBottomToolbar) { - radioBottomToolbar - } else { - radioTopToolbar + radio = when (settings.toolbarPosition) { + ToolbarPosition.BOTTOM -> radioBottomToolbar + ToolbarPosition.TOP -> radioTopToolbar } radio.updateRadioValue(true) diff --git a/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt index d894e5514..3d586befd 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt @@ -12,6 +12,7 @@ import androidx.appcompat.app.AppCompatDelegate import androidx.preference.PreferenceFragmentCompat import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings @@ -122,8 +123,9 @@ class CustomizationFragment : PreferenceFragmentCompat() { )) } - topPreference.setCheckedWithoutClickListener(!requireContext().settings().shouldUseBottomToolbar) - bottomPreference.setCheckedWithoutClickListener(requireContext().settings().shouldUseBottomToolbar) + val toolbarPosition = requireContext().settings().toolbarPosition + topPreference.setCheckedWithoutClickListener(toolbarPosition == ToolbarPosition.TOP) + bottomPreference.setCheckedWithoutClickListener(toolbarPosition == ToolbarPosition.BOTTOM) addToRadioGroup(topPreference, bottomPreference) } diff --git a/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt b/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt index 91916c768..8ed15bf03 100644 --- a/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt +++ b/app/src/main/java/org/mozilla/fenix/trackingprotection/TrackingProtectionOverlay.kt @@ -13,7 +13,6 @@ import android.view.LayoutInflater import android.view.MotionEvent import android.view.View import android.widget.ImageView -import androidx.core.view.isGone import androidx.core.view.isVisible import androidx.core.view.marginTop import kotlinx.android.synthetic.main.tracking_protection_onboarding_popup.* @@ -22,6 +21,7 @@ import mozilla.components.browser.session.Session import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.increaseTapArea import org.mozilla.fenix.utils.Settings @@ -63,10 +63,10 @@ class TrackingProtectionOverlay( val layout = LayoutInflater.from(context) .inflate(R.layout.tracking_protection_onboarding_popup, null) - val isBottomToolbar = settings.shouldUseBottomToolbar + val toolbarPosition = settings.toolbarPosition - layout.drop_down_triangle.isGone = isBottomToolbar - layout.pop_up_triangle.isVisible = isBottomToolbar + layout.drop_down_triangle.isVisible = toolbarPosition == ToolbarPosition.TOP + layout.pop_up_triangle.isVisible = toolbarPosition == ToolbarPosition.BOTTOM layout.onboarding_message.text = context.getString( @@ -91,11 +91,7 @@ class TrackingProtectionOverlay( val xOffset = triangleMarginStartPx + triangleWidthPx / 2 - val gravity = if (isBottomToolbar) { - Gravity.START or Gravity.BOTTOM - } else { - Gravity.START or Gravity.TOP - } + val gravity = Gravity.START or toolbarPosition.androidGravity trackingOnboardingDialog.apply { setContentView(layout) diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index cb88d7579..e5e21dbdc 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -29,6 +29,7 @@ import org.mozilla.fenix.Config import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.metrics.MozillaProductDetector +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.settings.PhoneFeature @@ -463,6 +464,9 @@ class Settings(private val appContext: Context) : PreferencesHolder { default = !touchExplorationIsEnabled && !switchServiceIsEnabled ) + val toolbarPosition: ToolbarPosition + get() = if (shouldUseBottomToolbar) ToolbarPosition.BOTTOM else ToolbarPosition.TOP + /** * Check each active accessibility service to see if it can perform gestures, if any can, * then it is *likely* a switch service is enabled. We are assuming this to be the case based on #7486 diff --git a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolderTest.kt index 93636ec77..414871a69 100644 --- a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingToolbarPositionPickerViewHolderTest.kt @@ -15,6 +15,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.ext.components +import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings @@ -34,7 +35,7 @@ class OnboardingToolbarPositionPickerViewHolderTest { @Test fun `bottom illustration should select corresponding radio button`() { - every { settings.shouldUseBottomToolbar } returns false + every { settings.toolbarPosition } returns ToolbarPosition.TOP OnboardingToolbarPositionPickerViewHolder(view) assertTrue(view.toolbar_top_radio_button.isChecked) assertFalse(view.toolbar_bottom_radio_button.isChecked) @@ -46,7 +47,7 @@ class OnboardingToolbarPositionPickerViewHolderTest { @Test fun `top illustration should select corresponding radio button`() { - every { settings.shouldUseBottomToolbar } returns true + every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM OnboardingToolbarPositionPickerViewHolder(view) assertFalse(view.toolbar_top_radio_button.isChecked) assertTrue(view.toolbar_bottom_radio_button.isChecked)