From 130fecde97a2d42dd5ba0261e914695de4118fa3 Mon Sep 17 00:00:00 2001 From: sarah541 Date: Wed, 28 Feb 2024 17:43:44 -0500 Subject: [PATCH] Bug 1870938 - Add display/edit toolbar separator and update url bar background --- .../fenix/browser/BaseBrowserFragment.kt | 6 ++- .../mozilla/fenix/browser/BrowserFragment.kt | 43 ------------------- .../components/toolbar/BrowserToolbarView.kt | 18 +++++++- .../fenix/search/SearchDialogFragment.kt | 6 +++ .../fenix/search/toolbar/ToolbarView.kt | 14 +++++- .../org/mozilla/fenix/theme/FirefoxTheme.kt | 27 ++++++++++++ .../drawable/search_old_url_background.xml | 12 ++++++ .../res/drawable/search_url_background.xml | 8 ++-- app/src/main/res/values-night/colors.xml | 4 ++ app/src/main/res/values/attrs.xml | 3 ++ app/src/main/res/values/colors.xml | 6 +++ app/src/main/res/values/styles.xml | 7 ++- .../fenix/search/toolbar/ToolbarViewTest.kt | 2 + 13 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 app/src/main/res/drawable/search_old_url_background.xml 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 bb437233f..8ac5638f7 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -444,14 +444,16 @@ abstract class BaseBrowserFragment : lifecycleOwner = viewLifecycleOwner, ) + val browserToolbar = browserToolbarView.view + if (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) { + browserToolbar.showPageActionSeparator() val isToolbarAtBottom = context.components.settings.toolbarPosition == ToolbarPosition.BOTTOM // The toolbar view has already been added directly to the container. // We should remove it and add the view to the navigation bar container. // Should refactor this so there is no added view to remove to begin with: // https://bugzilla.mozilla.org/show_bug.cgi?id=1870976 - val browserToolbar = browserToolbarView.view if (isToolbarAtBottom) { binding.browserLayout.removeView(browserToolbar) } @@ -486,6 +488,8 @@ abstract class BaseBrowserFragment : owner = this, view = view, ) + } else { + browserToolbar.hidePageActionSeparator() } toolbarIntegration.set( diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index fb0e2d646..b6154c5ea 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -414,49 +414,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { } } - private fun initReloadAction(context: Context) { - if (!IncompleteRedesignToolbarFeature(context.settings()).isEnabled || refreshAction != null) { - return - } - - refreshAction = - BrowserToolbar.TwoStateButton( - primaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_arrow_clockwise_24, - )!!, - primaryContentDescription = context.getString(R.string.browser_menu_refresh), - primaryImageTintResource = ThemeManager.resolveAttribute(R.attr.textPrimary, context), - isInPrimaryState = { - getCurrentTab()?.content?.loading == false - }, - secondaryImage = AppCompatResources.getDrawable( - context, - R.drawable.mozac_ic_stop, - )!!, - secondaryContentDescription = context.getString(R.string.browser_menu_stop), - disableInSecondaryState = false, - longClickListener = { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Reload(bypassCache = true), - ) - }, - listener = { - if (getCurrentTab()?.content?.loading == true) { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped(ToolbarMenu.Item.Stop) - } else { - browserToolbarInteractor.onBrowserToolbarMenuItemTapped( - ToolbarMenu.Item.Reload(bypassCache = false), - ) - } - }, - ) - - refreshAction?.let { - browserToolbarView.view.addPageAction(it) - } - } - private fun initReviewQualityCheck(context: Context, view: View) { val reviewQualityCheck = BrowserToolbar.ToggleButton( 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 12f5dde98..d499ab51d 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 @@ -12,6 +12,7 @@ import android.view.View import android.view.ViewGroup import androidx.annotation.LayoutRes import androidx.annotation.VisibleForTesting +import androidx.appcompat.content.res.AppCompatResources import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.core.content.ContextCompat import androidx.lifecycle.LifecycleOwner @@ -80,13 +81,23 @@ class BrowserToolbarView( with(context) { val isPinningSupported = components.useCases.webAppUseCases.isPinningSupported() + val searchUrlBackground = if (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) { + R.drawable.search_url_background + } else { + R.drawable.search_old_url_background + } view.apply { setToolbarBehavior() elevation = resources.getDimension(R.dimen.browser_fragment_toolbar_elevation) if (!isCustomTabSession) { - display.setUrlBackground(getDrawable(R.drawable.search_url_background)) + display.setUrlBackground( + AppCompatResources.getDrawable( + context, + searchUrlBackground, + ), + ) } display.onUrlClicked = { @@ -111,6 +122,10 @@ class BrowserToolbarView( context, ThemeManager.resolveAttribute(R.attr.borderPrimary, context), ) + val pageActionSeparatorColor = ContextCompat.getColor( + context, + ThemeManager.resolveAttribute(R.attr.borderToolbarDivider, context), + ) display.urlFormatter = { url -> URLStringUtils.toDisplayUrl(url) } @@ -126,6 +141,7 @@ class BrowserToolbarView( context, R.color.fx_mobile_icon_color_information, ), + pageActionSeparator = pageActionSeparatorColor, ) display.hint = context.getString(R.string.search_hint) diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index aa07bb96e..f2e593e3f 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -83,6 +83,7 @@ import org.mozilla.fenix.components.Core.Companion.BOOKMARKS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.Core.Companion.HISTORY_SEARCH_ENGINE_ID import org.mozilla.fenix.components.Core.Companion.TABS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.databinding.FragmentSearchDialogBinding import org.mozilla.fenix.databinding.SearchSuggestionsHintBinding @@ -252,6 +253,11 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { binding.toolbar, fromHomeFragment, ).also { + if (!IncompleteRedesignToolbarFeature(requireContext().settings()).isEnabled) { + it.view.hidePageActionSeparator() + } else { + it.view.showPageActionSeparator() + } inlineAutocompleteEditText = it.view.findViewById(R.id.mozac_browser_toolbar_edit_url_view) inlineAutocompleteEditText.increaseTapArea(TAP_INCREASE_DPS_4) } diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt index 5c362119f..0b5bbc3a7 100644 --- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt @@ -18,6 +18,8 @@ import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.R import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.search.SearchEngineSource import org.mozilla.fenix.search.SearchFragmentState import org.mozilla.fenix.utils.Settings @@ -99,10 +101,20 @@ class ToolbarView( R.color.suggestion_highlight_color, ), clear = context.getColorFromAttr(R.attr.textPrimary), + pageActionSeparator = context.getColorFromAttr(R.attr.borderToolbarDivider), ) + val searchUrlBackground = if (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) { + R.drawable.search_url_background + } else { + R.drawable.search_old_url_background + } + edit.setUrlBackground( - AppCompatResources.getDrawable(context, R.drawable.search_url_background), + AppCompatResources.getDrawable( + context, + searchUrlBackground, + ), ) private = isPrivate diff --git a/app/src/main/java/org/mozilla/fenix/theme/FirefoxTheme.kt b/app/src/main/java/org/mozilla/fenix/theme/FirefoxTheme.kt index 904f26649..a46826c80 100644 --- a/app/src/main/java/org/mozilla/fenix/theme/FirefoxTheme.kt +++ b/app/src/main/java/org/mozilla/fenix/theme/FirefoxTheme.kt @@ -105,6 +105,7 @@ private val darkColorPalette = FirefoxColors( layerConfirmation = PhotonColors.Green80, layerError = PhotonColors.Pink80, layerInfo = PhotonColors.Blue50, + layerSearch = PhotonColors.DarkGrey80, actionPrimary = PhotonColors.Violet60, actionSecondary = PhotonColors.LightGrey30, actionTertiary = PhotonColors.DarkGrey10, @@ -161,6 +162,7 @@ private val darkColorPalette = FirefoxColors( borderAccent = PhotonColors.Violet40, borderDisabled = PhotonColors.LightGrey05A40, borderWarning = PhotonColors.Red40, + borderToolbarDivider = PhotonColors.DarkGrey60, ) private val lightColorPalette = FirefoxColors( @@ -180,6 +182,7 @@ private val lightColorPalette = FirefoxColors( layerConfirmation = PhotonColors.Green20, layerError = PhotonColors.Red10, layerInfo = PhotonColors.Blue50A44, + layerSearch = PhotonColors.LightGrey30, actionPrimary = PhotonColors.Ink20, actionSecondary = PhotonColors.LightGrey30, actionTertiary = PhotonColors.LightGrey40, @@ -236,12 +239,15 @@ private val lightColorPalette = FirefoxColors( borderAccent = PhotonColors.Ink20, borderDisabled = PhotonColors.DarkGrey90A40, borderWarning = PhotonColors.Red70, + borderToolbarDivider = PhotonColors.LightGrey10, ) private val privateColorPalette = darkColorPalette.copy( layer1 = PhotonColors.Ink50, layer2 = PhotonColors.Ink50, layer3 = PhotonColors.Ink90, + layerSearch = PhotonColors.Ink90, + borderToolbarDivider = PhotonColors.Violet80, ) /** @@ -266,6 +272,7 @@ class FirefoxColors( layerConfirmation: Color, layerError: Color, layerInfo: Color, + layerSearch: Color, actionPrimary: Color, actionSecondary: Color, actionTertiary: Color, @@ -322,6 +329,7 @@ class FirefoxColors( borderAccent: Color, borderDisabled: Color, borderWarning: Color, + borderToolbarDivider: Color, ) { // Layers @@ -388,6 +396,10 @@ class FirefoxColors( var layerInfo by mutableStateOf(layerInfo) private set + // Search + var layerSearch by mutableStateOf(layerSearch) + private set + // Actions // Primary button, Snackbar, Floating action button, Chip selected @@ -608,6 +620,14 @@ class FirefoxColors( var borderWarning by mutableStateOf(borderWarning) private set + // Toolbar divider + var borderToolbarDivider by mutableStateOf(borderToolbarDivider) + private set + + /** + * Updates the existing colors with the provided [FirefoxColors]. + */ + @Suppress("LongMethod") fun update(other: FirefoxColors) { layer1 = other.layer1 layer2 = other.layer2 @@ -625,6 +645,7 @@ class FirefoxColors( layerConfirmation = other.layerConfirmation layerError = other.layerError layerInfo = other.layerInfo + layerSearch = other.layerSearch actionPrimary = other.actionPrimary actionSecondary = other.actionSecondary actionTertiary = other.actionTertiary @@ -681,11 +702,13 @@ class FirefoxColors( borderAccent = other.borderAccent borderDisabled = other.borderDisabled borderWarning = other.borderWarning + borderToolbarDivider = other.borderToolbarDivider } /** * Return a copy of this [FirefoxColors] and optionally overriding any of the provided values. */ + @Suppress("LongMethod") fun copy( layer1: Color = this.layer1, layer2: Color = this.layer2, @@ -703,6 +726,7 @@ class FirefoxColors( layerConfirmation: Color = this.layerConfirmation, layerError: Color = this.layerError, layerInfo: Color = this.layerInfo, + layerSearch: Color = this.layerSearch, actionPrimary: Color = this.actionPrimary, actionSecondary: Color = this.actionSecondary, actionTertiary: Color = this.actionTertiary, @@ -759,6 +783,7 @@ class FirefoxColors( borderAccent: Color = this.borderAccent, borderDisabled: Color = this.borderDisabled, borderWarning: Color = this.borderWarning, + borderToolbarDivider: Color = this.borderToolbarDivider, ): FirefoxColors = FirefoxColors( layer1 = layer1, layer2 = layer2, @@ -776,6 +801,7 @@ class FirefoxColors( layerConfirmation = layerConfirmation, layerError = layerError, layerInfo = layerInfo, + layerSearch = layerSearch, actionPrimary = actionPrimary, actionSecondary = actionSecondary, actionTertiary = actionTertiary, @@ -832,6 +858,7 @@ class FirefoxColors( borderAccent = borderAccent, borderDisabled = borderDisabled, borderWarning = borderWarning, + borderToolbarDivider = borderToolbarDivider, ) } diff --git a/app/src/main/res/drawable/search_old_url_background.xml b/app/src/main/res/drawable/search_old_url_background.xml new file mode 100644 index 000000000..6c71fe948 --- /dev/null +++ b/app/src/main/res/drawable/search_old_url_background.xml @@ -0,0 +1,12 @@ + + + + + + + diff --git a/app/src/main/res/drawable/search_url_background.xml b/app/src/main/res/drawable/search_url_background.xml index 6c71fe948..c64970c69 100644 --- a/app/src/main/res/drawable/search_url_background.xml +++ b/app/src/main/res/drawable/search_url_background.xml @@ -3,10 +3,10 @@ - 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/. --> - + + android:bottomLeftRadius="8dp" + android:topLeftRadius="8dp" + android:topRightRadius="8dp"/> diff --git a/app/src/main/res/values-night/colors.xml b/app/src/main/res/values-night/colors.xml index 34e3e388a..f4eb10f27 100644 --- a/app/src/main/res/values-night/colors.xml +++ b/app/src/main/res/values-night/colors.xml @@ -31,6 +31,8 @@ @color/photonPink80 @color/photonBlue50A80 + + @color/photonDarkGrey80 @@ -143,6 +145,8 @@ @color/photonLightGrey05A40 @color/photonRed40 + + @color/photonDarkGrey60 @color/photonViolet50 diff --git a/app/src/main/res/values/attrs.xml b/app/src/main/res/values/attrs.xml index 8ceff53ac..a9b94385b 100644 --- a/app/src/main/res/values/attrs.xml +++ b/app/src/main/res/values/attrs.xml @@ -12,6 +12,7 @@ + @@ -47,6 +48,8 @@ + + diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 2629152f8..0625f4317 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -12,6 +12,7 @@ @color/photonWhite @color/photonLightGrey20 + @color/photonLightGrey30 @color/photonInk20 @@ -133,6 +134,8 @@ @color/photonLightGrey30 + + @color/photonLightGrey10 @color/photonDarkGrey05 @@ -153,6 +156,7 @@ @color/photonInk50 @color/photonInk90 + @color/photonInk90 @color/photonPurple70 @@ -274,6 +278,8 @@ @color/photonLightGrey05A40 @color/photonRed40 + + @color/photonViolet80 @color/photonInk80 diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 5f1092c36..cb306c5b5 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -50,6 +50,7 @@ @color/fx_mobile_layer_color_2 @color/fx_mobile_layer_color_3 + @color/fx_mobile_layer_color_search @color/fx_mobile_layer_color_accent_nonopaque @color/fx_mobile_layer_color_scrim @@ -85,7 +86,8 @@ @color/fx_mobile_border_color_primary - + + @color/fx_mobile_border_color_toolbar_divider @color/accent_high_contrast_normal_theme @@ -243,6 +245,7 @@ @color/fx_mobile_private_layer_color_2 @color/fx_mobile_private_layer_color_3 + @color/fx_mobile_private_layer_color_search @color/fx_mobile_private_layer_color_accent_nonopaque @color/fx_mobile_private_layer_color_scrim @@ -278,6 +281,8 @@ @color/fx_mobile_private_border_color_primary + + @color/fx_mobile_private_border_color_toolbar_divider @color/toggle_off_knob_dark_theme diff --git a/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt b/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt index 5c62516f8..5aa5baf8d 100644 --- a/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt @@ -147,6 +147,7 @@ class ToolbarViewTest { every { context.settings().shouldShowHistorySuggestions } returns true every { context.settings().shouldShowBookmarkSuggestions } returns true every { context.settings().isTabletAndTabStripEnabled } returns false + every { context.settings().enableIncompleteToolbarRedesign } returns false val view = buildToolbarView(false) mockkObject(FeatureFlags) @@ -163,6 +164,7 @@ class ToolbarViewTest { every { context.settings().shouldShowHistorySuggestions } returns true every { context.settings().shouldShowBookmarkSuggestions } returns true every { context.settings().isTabletAndTabStripEnabled } returns false + every { context.settings().enableIncompleteToolbarRedesign } returns false val view = buildToolbarView(false) mockkObject(FeatureFlags)