Bug 1895748 - Remove toolbar layout changes not needed for nav bar release. r=android-reviewers,harrisono,skhan,tchoh,mavduevskiy

Differential Revision: https://phabricator.services.mozilla.com/D210099
This commit is contained in:
Roger Yang 2024-05-29 20:23:16 +00:00
parent 9fd989ed37
commit 19d6f9b5d1
30 changed files with 21 additions and 442 deletions

View file

@ -76,7 +76,6 @@ class BrowserToolbar @JvmOverloads constructor(
@VisibleForTesting @VisibleForTesting
internal var searchTerms: String = "" internal var searchTerms: String = ""
private var urlCommitListener: ((String) -> Boolean)? = null private var urlCommitListener: ((String) -> Boolean)? = null
var isNavBarEnabled: Boolean = false
/** /**
* Toolbar in "display mode". * Toolbar in "display mode".
@ -357,22 +356,6 @@ class BrowserToolbar @JvmOverloads constructor(
display.setHorizontalPadding(horizontalPadding) display.setHorizontalPadding(horizontalPadding)
} }
/**
* Hides the page action separator in display/edit mode.
*/
override fun hidePageActionSeparator() {
display.hidePageActionSeparator()
edit.hidePageActionSeparator()
}
/**
* Shows the page action separator in display/edit mode.
*/
override fun showPageActionSeparator() {
display.showPageActionSeparator()
edit.showPageActionSeparator()
}
/** /**
* Switches to URL editing mode. * Switches to URL editing mode.
* *

View file

@ -95,7 +95,6 @@ class DisplayToolbar internal constructor(
* @property text Text color of the URL. * @property text Text color of the URL.
* @property trackingProtection Color tint for the tracking protection icons. * @property trackingProtection Color tint for the tracking protection icons.
* @property separator Color tint for the separator shown between indicators. * @property separator Color tint for the separator shown between indicators.
* @property pageActionSeparator Color tint of separator dividing url and page actions.
* @property highlight Color tint for the highlight icon. * @property highlight Color tint for the highlight icon.
* *
* Set/Get the site security icon colours. It uses a pair of color integers which represent the * Set/Get the site security icon colours. It uses a pair of color integers which represent the
@ -111,7 +110,6 @@ class DisplayToolbar internal constructor(
@ColorInt val text: Int, @ColorInt val text: Int,
@ColorInt val trackingProtection: Int?, @ColorInt val trackingProtection: Int?,
@ColorInt val separator: Int, @ColorInt val separator: Int,
@ColorInt val pageActionSeparator: Int,
@ColorInt val highlight: Int?, @ColorInt val highlight: Int?,
) )
@ -150,7 +148,6 @@ class DisplayToolbar internal constructor(
navigationActions = rootView.findViewById(R.id.mozac_browser_toolbar_navigation_actions), navigationActions = rootView.findViewById(R.id.mozac_browser_toolbar_navigation_actions),
background = rootView.findViewById(R.id.mozac_browser_toolbar_background), background = rootView.findViewById(R.id.mozac_browser_toolbar_background),
separator = rootView.findViewById(R.id.mozac_browser_toolbar_separator), separator = rootView.findViewById(R.id.mozac_browser_toolbar_separator),
pageActionSeparator = rootView.findViewById(R.id.mozac_browser_toolbar_action_separator),
emptyIndicator = rootView.findViewById(R.id.mozac_browser_toolbar_empty_indicator), emptyIndicator = rootView.findViewById(R.id.mozac_browser_toolbar_empty_indicator),
menu = MenuButton(rootView.findViewById(R.id.mozac_browser_toolbar_menu)), menu = MenuButton(rootView.findViewById(R.id.mozac_browser_toolbar_menu)),
securityIndicator = rootView.findViewById(R.id.mozac_browser_toolbar_security_indicator), securityIndicator = rootView.findViewById(R.id.mozac_browser_toolbar_security_indicator),
@ -177,7 +174,6 @@ class DisplayToolbar internal constructor(
text = views.origin.textColor, text = views.origin.textColor,
trackingProtection = null, trackingProtection = null,
separator = ContextCompat.getColor(context, photonColors.photonGrey80), separator = ContextCompat.getColor(context, photonColors.photonGrey80),
pageActionSeparator = ContextCompat.getColor(context, photonColors.photonGrey80),
highlight = null, highlight = null,
) )
set(value) { set(value) {
@ -190,7 +186,6 @@ class DisplayToolbar internal constructor(
views.origin.titleColor = value.title views.origin.titleColor = value.title
views.origin.textColor = value.text views.origin.textColor = value.text
views.separator.setColorFilter(value.separator) views.separator.setColorFilter(value.separator)
views.pageActionSeparator.setBackgroundColor(value.pageActionSeparator)
if (value.trackingProtection != null) { if (value.trackingProtection != null) {
views.trackingProtectionIndicator.setTint(value.trackingProtection) views.trackingProtectionIndicator.setTint(value.trackingProtection)
@ -679,20 +674,6 @@ class DisplayToolbar internal constructor(
background.layoutParams = this background.layoutParams = this
} }
} }
/**
* Hides the page action separator in display mode.
*/
fun hidePageActionSeparator() {
views.pageActionSeparator.isVisible = false
}
/**
* Shows the page action separator in display mode.
*/
internal fun showPageActionSeparator() {
views.pageActionSeparator.isVisible = true
}
} }
/** /**
@ -705,7 +686,6 @@ internal class DisplayToolbarViews(
val navigationActions: ActionContainer, val navigationActions: ActionContainer,
val background: ImageView, val background: ImageView,
val separator: ImageView, val separator: ImageView,
val pageActionSeparator: View,
val emptyIndicator: ImageView, val emptyIndicator: ImageView,
val menu: MenuButton, val menu: MenuButton,
val securityIndicator: SiteSecurityIconView, val securityIndicator: SiteSecurityIconView,

View file

@ -71,17 +71,14 @@ class EditToolbar internal constructor(
* @property text Text color of the URL. * @property text Text color of the URL.
* @property suggestionBackground The background color used for autocomplete suggestions. * @property suggestionBackground The background color used for autocomplete suggestions.
* @property suggestionForeground The foreground color used for autocomplete suggestions. * @property suggestionForeground The foreground color used for autocomplete suggestions.
* @property pageActionSeparator Color tint of separator dividing page actions.
*/ */
data class Colors( data class Colors(
@ColorInt val clear: Int, @ColorInt val clear: Int,
@ColorInt val erase: Int,
@ColorInt val icon: Int?, @ColorInt val icon: Int?,
@ColorInt val hint: Int, @ColorInt val hint: Int,
@ColorInt val text: Int, @ColorInt val text: Int,
@ColorInt val suggestionBackground: Int, @ColorInt val suggestionBackground: Int,
@ColorInt val suggestionForeground: Int?, @ColorInt val suggestionForeground: Int?,
@ColorInt val pageActionSeparator: Int,
) )
private val autocompleteDispatcher = SupervisorJob() + private val autocompleteDispatcher = SupervisorJob() +
@ -104,11 +101,6 @@ class EditToolbar internal constructor(
onClear() onClear()
} }
}, },
erase = rootView.findViewById<ImageView>(R.id.mozac_browser_toolbar_erase_view).apply {
setOnClickListener {
onClear()
}
},
url = rootView.findViewById<InlineAutocompleteEditText>( url = rootView.findViewById<InlineAutocompleteEditText>(
R.id.mozac_browser_toolbar_edit_url_view, R.id.mozac_browser_toolbar_edit_url_view,
).apply { ).apply {
@ -136,7 +128,6 @@ class EditToolbar internal constructor(
false false
} }
}, },
pageActionSeparator = rootView.findViewById(R.id.mozac_browser_action_separator),
) )
/** /**
@ -144,21 +135,17 @@ class EditToolbar internal constructor(
*/ */
var colors: Colors = Colors( var colors: Colors = Colors(
clear = ContextCompat.getColor(context, colorsR.color.photonWhite), clear = ContextCompat.getColor(context, colorsR.color.photonWhite),
erase = ContextCompat.getColor(context, colorsR.color.photonWhite),
icon = null, icon = null,
hint = views.url.currentHintTextColor, hint = views.url.currentHintTextColor,
text = views.url.currentTextColor, text = views.url.currentTextColor,
suggestionBackground = views.url.autoCompleteBackgroundColor, suggestionBackground = views.url.autoCompleteBackgroundColor,
suggestionForeground = views.url.autoCompleteForegroundColor, suggestionForeground = views.url.autoCompleteForegroundColor,
pageActionSeparator = ContextCompat.getColor(context, colorsR.color.photonGrey80),
) )
set(value) { set(value) {
field = value field = value
views.clear.setColorFilter(value.clear) views.clear.setColorFilter(value.clear)
views.erase.setColorFilter(value.erase)
if (value.icon != null) { if (value.icon != null) {
views.icon.setColorFilter(value.icon) views.icon.setColorFilter(value.icon)
} }
@ -167,7 +154,6 @@ class EditToolbar internal constructor(
views.url.setTextColor(value.text) views.url.setTextColor(value.text)
views.url.autoCompleteBackgroundColor = value.suggestionBackground views.url.autoCompleteBackgroundColor = value.suggestionBackground
views.url.autoCompleteForegroundColor = value.suggestionForeground views.url.autoCompleteForegroundColor = value.suggestionForeground
views.pageActionSeparator.setBackgroundColor(value.pageActionSeparator)
} }
/** /**
@ -296,8 +282,7 @@ class EditToolbar internal constructor(
} else { } else {
views.url.setText(url, shouldAutoComplete) views.url.setText(url, shouldAutoComplete)
} }
views.clear.isVisible = url.isNotBlank() && !toolbar.isNavBarEnabled views.clear.isVisible = url.isNotBlank()
views.erase.isVisible = url.isNotBlank() && toolbar.isNavBarEnabled
if (shouldHighlight) { if (shouldHighlight) {
views.url.setSelection(views.url.text.length - url.length, views.url.text.length) views.url.setSelection(views.url.text.length - url.length, views.url.text.length)
@ -363,8 +348,7 @@ class EditToolbar internal constructor(
} }
private fun onTextChanged(text: String) { private fun onTextChanged(text: String) {
views.clear.isVisible = text.isNotBlank() && !toolbar.isNavBarEnabled views.clear.isVisible = text.isNotBlank()
views.erase.isVisible = text.isNotBlank() && toolbar.isNavBarEnabled
views.editActionsEnd.autoHideAction(text.isEmpty()) views.editActionsEnd.autoHideAction(text.isEmpty())
/* /*
@ -383,33 +367,16 @@ class EditToolbar internal constructor(
} }
editListener?.onTextChanged(text) editListener?.onTextChanged(text)
} }
/**
* Hides the page action separator in edit mode.
*/
fun hidePageActionSeparator() {
views.pageActionSeparator.isVisible = false
}
/**
* Shows the page action separator in edit mode.
*/
fun showPageActionSeparator() {
views.pageActionSeparator.isVisible = true
}
} }
/** /**
* Internal holder for view references. * Internal holder for view references.
*/ */
@Suppress("LongParameterList")
internal class EditToolbarViews( internal class EditToolbarViews(
val background: ImageView, val background: ImageView,
val icon: ImageView, val icon: ImageView,
val editActionsStart: ActionContainer, val editActionsStart: ActionContainer,
val editActionsEnd: ActionContainer, val editActionsEnd: ActionContainer,
val clear: ImageView, val clear: ImageView,
val erase: ImageView,
val url: InlineAutocompleteEditText, val url: InlineAutocompleteEditText,
val pageActionSeparator: View,
) )

View file

@ -106,25 +106,12 @@
android:layout_height="40dp" android:layout_height="40dp"
android:layout_marginTop="8dp" android:layout_marginTop="8dp"
android:paddingEnd="@dimen/mozac_browser_toolbar_origin_padding_end" android:paddingEnd="@dimen/mozac_browser_toolbar_origin_padding_end"
app:layout_constraintEnd_toStartOf="@+id/mozac_browser_toolbar_action_separator" app:layout_constraintEnd_toStartOf="@+id/mozac_browser_toolbar_page_actions"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_toolbar_security_indicator" app:layout_constraintStart_toEndOf="@+id/mozac_browser_toolbar_security_indicator"
app:layout_constraintTop_toTopOf="parent" app:layout_constraintTop_toTopOf="parent"
app:layout_goneMarginStart="8dp" app:layout_goneMarginStart="8dp"
app:layout_goneMarginTop="8dp" /> app:layout_goneMarginTop="8dp" />
<View
android:id="@+id/mozac_browser_toolbar_action_separator"
android:layout_width="@dimen/mozac_browser_toolbar_page_action_separator_width"
android:layout_height="40dp"
android:layout_marginTop="8dp"
android:layout_marginStart="8dp"
android:visibility="gone"
android:importantForAccessibility="no"
android:scaleType="center"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_toolbar_origin_view"
app:layout_constraintEnd_toStartOf="@id/mozac_browser_toolbar_page_actions"
app:layout_constraintTop_toTopOf="parent" />
<!-- Page actions --> <!-- Page actions -->
<mozilla.components.browser.toolbar.internal.ActionContainer <mozilla.components.browser.toolbar.internal.ActionContainer

View file

@ -61,31 +61,6 @@
android:textSize="15sp" android:textSize="15sp"
app:layout_goneMarginStart="8dp" app:layout_goneMarginStart="8dp"
app:layout_constraintStart_toEndOf="@id/mozac_browser_toolbar_edit_actions_start" app:layout_constraintStart_toEndOf="@id/mozac_browser_toolbar_edit_actions_start"
app:layout_constraintEnd_toStartOf="@id/mozac_browser_toolbar_erase_view"
app:layout_constraintTop_toTopOf="parent" />
<ImageView
android:id="@+id/mozac_browser_toolbar_erase_view"
android:layout_width="44dp"
android:layout_height="44dp"
android:contentDescription="@string/mozac_clear_button_description"
android:scaleType="center"
app:srcCompat="@drawable/mozac_ic_cross_circle_fill_20"
android:visibility="gone"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_toolbar_edit_url_view"
app:layout_constraintEnd_toStartOf="@id/mozac_browser_action_separator"
app:layout_constraintTop_toTopOf="parent" />
<View
android:id="@+id/mozac_browser_action_separator"
android:layout_width="@dimen/mozac_browser_toolbar_page_action_separator_width"
android:layout_height="40dp"
android:layout_marginTop="8dp"
android:visibility="gone"
android:importantForAccessibility="no"
android:scaleType="center"
app:layout_constraintStart_toEndOf="@+id/mozac_browser_toolbar_erase_view"
app:layout_constraintEnd_toStartOf="@id/mozac_browser_toolbar_edit_actions_end" app:layout_constraintEnd_toStartOf="@id/mozac_browser_toolbar_edit_actions_end"
app:layout_constraintTop_toTopOf="parent" /> app:layout_constraintTop_toTopOf="parent" />

View file

@ -9,7 +9,6 @@
<dimen name="mozac_browser_toolbar_progress_bar_height">3dp</dimen> <dimen name="mozac_browser_toolbar_progress_bar_height">3dp</dimen>
<dimen name="mozac_browser_toolbar_icons_separator_height">24dp</dimen> <dimen name="mozac_browser_toolbar_icons_separator_height">24dp</dimen>
<dimen name="mozac_browser_toolbar_icons_separator_width">1dp</dimen> <dimen name="mozac_browser_toolbar_icons_separator_width">1dp</dimen>
<dimen name="mozac_browser_toolbar_page_action_separator_width">4dp</dimen>
<dimen name="mozac_browser_toolbar_url_fading_edge_size">24dp</dimen> <dimen name="mozac_browser_toolbar_url_fading_edge_size">24dp</dimen>
<dimen name="mozac_browser_toolbar_icon_padding">12dp</dimen> <dimen name="mozac_browser_toolbar_icon_padding">12dp</dimen>
<dimen name="mozac_browser_toolbar_icon_size">24dp</dimen> <dimen name="mozac_browser_toolbar_icon_size">24dp</dimen>

View file

@ -566,36 +566,6 @@ class BrowserToolbarTest {
verify(display).showMenuButton() verify(display).showMenuButton()
} }
@Test
fun `WHEN showPageActionSeparator is sent to BrowserToolbar THEN it will be forwarded to the DisplayToolbar and EditToolbar`() {
val toolbar = BrowserToolbar(testContext)
val display: DisplayToolbar = mock()
val edit: EditToolbar = mock()
toolbar.display = display
toolbar.edit = edit
toolbar.showPageActionSeparator()
verify(display).showPageActionSeparator()
verify(edit).showPageActionSeparator()
}
@Test
fun `WHEN hidePageActionSeparator is sent to BrowserToolbar THEN it will be forwarded to the DisplayToolbar and EditToolbar`() {
val toolbar = BrowserToolbar(testContext)
val display: DisplayToolbar = mock()
val edit: EditToolbar = mock()
toolbar.display = display
toolbar.edit = edit
toolbar.hidePageActionSeparator()
verify(display).hidePageActionSeparator()
verify(edit).hidePageActionSeparator()
}
@Test @Test
fun `WHEN setDisplayHorizontalPadding is sent to BrowserToolbar THEN it will be forwarded to the DisplayToolbar`() { fun `WHEN setDisplayHorizontalPadding is sent to BrowserToolbar THEN it will be forwarded to the DisplayToolbar`() {
val toolbar = BrowserToolbar(testContext) val toolbar = BrowserToolbar(testContext)

View file

@ -357,22 +357,6 @@ class BrowserToolbar @JvmOverloads constructor(
display.setHorizontalPadding(horizontalPadding) display.setHorizontalPadding(horizontalPadding)
} }
/**
* Hides the page action separator in display/edit mode.
*/
override fun hidePageActionSeparator() {
display.hidePageActionSeparator()
edit.hidePageActionSeparator()
}
/**
* Shows the page action separator in display/edit mode.
*/
override fun showPageActionSeparator() {
display.showPageActionSeparator()
edit.showPageActionSeparator()
}
/** /**
* Switches to URL editing mode. * Switches to URL editing mode.
* *

View file

@ -188,16 +188,6 @@ interface Toolbar : ScrollableToolbar {
*/ */
fun setDisplayHorizontalPadding(horizontalPadding: Int) fun setDisplayHorizontalPadding(horizontalPadding: Int)
/**
* Hides the page action separator in display mode.
*/
fun hidePageActionSeparator()
/**
* Shows the page action separator in display mode.
*/
fun showPageActionSeparator()
/** /**
* Casts this toolbar to an Android View object. * Casts this toolbar to an Android View object.
*/ */

View file

@ -132,7 +132,6 @@ class CustomTabsToolbarFeature(
internal fun init(config: CustomTabConfig) { internal fun init(config: CustomTabConfig) {
// Don't allow clickable toolbar so a custom tab can't switch to edit mode. // Don't allow clickable toolbar so a custom tab can't switch to edit mode.
toolbar.display.onUrlClicked = { false } toolbar.display.onUrlClicked = { false }
toolbar.display.hidePageActionSeparator()
// Use the intent provided color scheme or fallback to the app night mode preference. // Use the intent provided color scheme or fallback to the app night mode preference.
val nightMode = config.colorScheme?.toNightMode() ?: appNightMode val nightMode = config.colorScheme?.toNightMode() ?: appNightMode

View file

@ -91,8 +91,6 @@ class CustomTabSessionTitleObserverTest {
override fun hideMenuButton() = Unit override fun hideMenuButton() = Unit
override fun showMenuButton() = Unit override fun showMenuButton() = Unit
override fun setDisplayHorizontalPadding(horizontalPadding: Int) = Unit override fun setDisplayHorizontalPadding(horizontalPadding: Int) = Unit
override fun hidePageActionSeparator() = Unit
override fun showPageActionSeparator() = Unit
override fun setOnEditListener(listener: Toolbar.OnEditListener) = Unit override fun setOnEditListener(listener: Toolbar.OnEditListener) = Unit
override fun displayMode() = Unit override fun displayMode() = Unit
override fun editMode(cursorPlacement: Toolbar.CursorPlacement) = Unit override fun editMode(cursorPlacement: Toolbar.CursorPlacement) = Unit

View file

@ -127,14 +127,6 @@ class ToolbarAutocompleteFeatureTest {
fail() fail()
} }
override fun hidePageActionSeparator() {
fail()
}
override fun showPageActionSeparator() {
fail()
}
override fun invalidateActions() { override fun invalidateActions() {
fail() fail()
} }

View file

@ -110,14 +110,6 @@ class ToolbarInteractorTest {
fail() fail()
} }
override fun hidePageActionSeparator() {
fail()
}
override fun showPageActionSeparator() {
fail()
}
override fun invalidateActions() { override fun invalidateActions() {
fail() fail()
} }

View file

@ -45,7 +45,7 @@ events:
source: source:
description: | description: |
The view the user was on when they initiated the search (For example: The view the user was on when they initiated the search (For example:
`HOME`, `HOME_QR`, `HOME_VOICE` or `BROWSER`) `Home` or `Browser`)
type: string type: string
bugs: bugs:
- https://github.com/mozilla-mobile/fenix/issues/959 - https://github.com/mozilla-mobile/fenix/issues/959

View file

@ -512,8 +512,6 @@ abstract class BaseBrowserFragment :
context = context, context = context,
activity = activity, activity = activity,
) )
} else {
browserToolbarView.view.hidePageActionSeparator()
} }
if (!shouldAddNavigationBar && shouldShowMicrosurveyPrompt()) { if (!shouldAddNavigationBar && shouldShowMicrosurveyPrompt()) {
@ -1303,7 +1301,6 @@ abstract class BaseBrowserFragment :
context: Context, context: Context,
activity: HomeActivity, activity: HomeActivity,
) { ) {
browserToolbar.showPageActionSeparator()
val isToolbarAtBottom = isToolbarAtBottom(context) val isToolbarAtBottom = isToolbarAtBottom(context)
// The toolbar view has already been added directly to the container. // The toolbar view has already been added directly to the container.

View file

@ -13,7 +13,6 @@ import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import androidx.annotation.LayoutRes import androidx.annotation.LayoutRes
import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting
import androidx.appcompat.content.res.AppCompatResources
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.ViewCompositionStrategy import androidx.compose.ui.platform.ViewCompositionStrategy
@ -103,28 +102,15 @@ class BrowserToolbarView(
true true
} }
view.isNavBarEnabled = isNavBarEnabled
with(context) { with(context) {
val isPinningSupported = components.useCases.webAppUseCases.isPinningSupported() val isPinningSupported = components.useCases.webAppUseCases.isPinningSupported()
val searchUrlBackground = if (isNavBarEnabled) {
R.drawable.search_url_background
} else {
R.drawable.search_old_url_background
}
layout.elevation = resources.getDimension(R.dimen.browser_fragment_toolbar_elevation) layout.elevation = resources.getDimension(R.dimen.browser_fragment_toolbar_elevation)
view.apply { view.apply {
setToolbarBehavior() setToolbarBehavior()
if (!isCustomTabSession) { if (!isCustomTabSession) {
display.setUrlBackground( display.setUrlBackground(getDrawable(R.drawable.search_url_background))
AppCompatResources.getDrawable(
context,
searchUrlBackground,
),
)
} }
display.onUrlClicked = { display.onUrlClicked = {
@ -149,10 +135,6 @@ class BrowserToolbarView(
context, context,
ThemeManager.resolveAttribute(R.attr.borderPrimary, context), ThemeManager.resolveAttribute(R.attr.borderPrimary, context),
) )
val pageActionSeparatorColor = ContextCompat.getColor(
context,
ThemeManager.resolveAttribute(R.attr.borderToolbarDivider, context),
)
display.urlFormatter = { url -> display.urlFormatter = { url ->
if (isNavBarEnabled) { if (isNavBarEnabled) {
@ -174,7 +156,6 @@ class BrowserToolbarView(
context, context,
R.color.fx_mobile_icon_color_information, R.color.fx_mobile_icon_color_information,
), ),
pageActionSeparator = pageActionSeparatorColor,
) )
display.hint = context.getString(R.string.search_hint) display.hint = context.getString(R.string.search_hint)

View file

@ -466,7 +466,6 @@ class HomeFragment : Fragment() {
binding = binding, binding = binding,
context = requireContext(), context = requireContext(),
interactor = sessionControlInteractor, interactor = sessionControlInteractor,
searchEngine = components.core.store.state.search.selectedOrDefaultSearchEngine,
) )
val shouldAddNavigationBar = shouldAddNavigationBar(requireContext()) val shouldAddNavigationBar = shouldAddNavigationBar(requireContext())

View file

@ -5,8 +5,6 @@
package org.mozilla.fenix.home package org.mozilla.fenix.home
import android.content.Context import android.content.Context
import android.content.Intent
import android.speech.RecognizerIntent
import android.view.Gravity import android.view.Gravity
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
@ -16,7 +14,6 @@ import androidx.constraintlayout.widget.ConstraintSet
import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.view.isVisible import androidx.core.view.isVisible
import androidx.core.view.updateLayoutParams import androidx.core.view.updateLayoutParams
import mozilla.components.browser.state.search.SearchEngine
import mozilla.components.support.ktx.android.content.res.resolveAttribute import mozilla.components.support.ktx.android.content.res.resolveAttribute
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.browser.tabstrip.isTabStripEnabled import org.mozilla.fenix.browser.tabstrip.isTabStripEnabled
@ -25,7 +22,6 @@ import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.databinding.FragmentHomeBinding import org.mozilla.fenix.databinding.FragmentHomeBinding
import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.home.toolbar.ToolbarInteractor import org.mozilla.fenix.home.toolbar.ToolbarInteractor
import org.mozilla.fenix.search.ExtraAction
import org.mozilla.fenix.utils.ToolbarPopupWindow import org.mozilla.fenix.utils.ToolbarPopupWindow
import java.lang.ref.WeakReference import java.lang.ref.WeakReference
@ -36,7 +32,6 @@ class ToolbarView(
private val binding: FragmentHomeBinding, private val binding: FragmentHomeBinding,
private val context: Context, private val context: Context,
private val interactor: ToolbarInteractor, private val interactor: ToolbarInteractor,
private val searchEngine: SearchEngine?,
) { ) {
init { init {
updateLayout(binding.root) updateLayout(binding.root)
@ -53,14 +48,6 @@ class ToolbarView(
interactor.onNavigateSearch() interactor.onNavigateSearch()
} }
binding.qrActionImage.setOnClickListener {
interactor.onNavigateSearch(ExtraAction.QR_READER)
}
binding.microphoneActionImage.setOnClickListener {
interactor.onNavigateSearch(ExtraAction.VOICE_SEARCH)
}
binding.toolbarWrapper.setOnLongClickListener { binding.toolbarWrapper.setOnLongClickListener {
ToolbarPopupWindow.show( ToolbarPopupWindow.show(
WeakReference(it), WeakReference(it),
@ -72,31 +59,10 @@ class ToolbarView(
} }
} }
@Suppress("LongMethod")
private fun updateLayout(view: View) { private fun updateLayout(view: View) {
val speechIntent = Intent(RecognizerIntent.ACTION_RECOGNIZE_SPEECH) val redesignEnabled = IncompleteRedesignToolbarFeature(context.settings()).isEnabled
binding.menuButton.isVisible = !redesignEnabled
when (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) { binding.tabButton.isVisible = !redesignEnabled
true -> {
binding.menuButton.isVisible = false
binding.tabButton.isVisible = false
binding.qrActionImage.isVisible =
searchEngine?.isGeneral == true || searchEngine?.type == SearchEngine.Type.CUSTOM
binding.microphoneActionImage.isVisible =
speechIntent.resolveActivity(context.packageManager) != null &&
context.settings().shouldShowVoiceSearch
binding.browserActionSeparator.isVisible =
binding.qrActionImage.isVisible || binding.microphoneActionImage.isVisible
}
false -> {
binding.menuButton.isVisible = true
binding.tabButton.isVisible = true
binding.browserActionSeparator.isVisible = false
binding.qrActionImage.isVisible = false
binding.microphoneActionImage.isVisible = false
}
}
when (context.settings().toolbarPosition) { when (context.settings().toolbarPosition) {
ToolbarPosition.TOP -> { ToolbarPosition.TOP -> {
@ -161,13 +127,5 @@ class ToolbarView(
ToolbarPosition.BOTTOM -> {} ToolbarPosition.BOTTOM -> {}
} }
binding.toolbarWrapper.updateLayoutParams<ViewGroup.MarginLayoutParams> {
rightMargin = if (IncompleteRedesignToolbarFeature(context.settings()).isEnabled) {
context.resources.getDimensionPixelSize(R.dimen.home_fragment_toolbar_margin)
} else {
0
}
}
} }
} }

View file

@ -31,7 +31,6 @@ import org.mozilla.fenix.home.recentvisits.controller.RecentVisitsController
import org.mozilla.fenix.home.recentvisits.interactor.RecentVisitsInteractor import org.mozilla.fenix.home.recentvisits.interactor.RecentVisitsInteractor
import org.mozilla.fenix.home.toolbar.ToolbarController import org.mozilla.fenix.home.toolbar.ToolbarController
import org.mozilla.fenix.home.toolbar.ToolbarInteractor import org.mozilla.fenix.home.toolbar.ToolbarInteractor
import org.mozilla.fenix.search.ExtraAction
import org.mozilla.fenix.search.toolbar.SearchSelectorController import org.mozilla.fenix.search.toolbar.SearchSelectorController
import org.mozilla.fenix.search.toolbar.SearchSelectorInteractor import org.mozilla.fenix.search.toolbar.SearchSelectorInteractor
import org.mozilla.fenix.search.toolbar.SearchSelectorMenu import org.mozilla.fenix.search.toolbar.SearchSelectorMenu
@ -334,8 +333,8 @@ class SessionControlInteractor(
toolbarController.handlePaste(clipboardText) toolbarController.handlePaste(clipboardText)
} }
override fun onNavigateSearch(extraAction: ExtraAction) { override fun onNavigateSearch() {
toolbarController.handleNavigateSearch(extraAction) toolbarController.handleNavigateSearch()
} }
override fun onRemoveCollectionsPlaceholder() { override fun onRemoveCollectionsPlaceholder() {

View file

@ -15,7 +15,6 @@ import org.mozilla.fenix.NavGraphDirections
import org.mozilla.fenix.browser.BrowserAnimator import org.mozilla.fenix.browser.BrowserAnimator
import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.components.metrics.MetricsUtils
import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.search.ExtraAction
/** /**
* An interface that handles the view manipulation of the home screen toolbar. * An interface that handles the view manipulation of the home screen toolbar.
@ -34,7 +33,7 @@ interface ToolbarController {
/** /**
* @see [ToolbarInteractor.onNavigateSearch] * @see [ToolbarInteractor.onNavigateSearch]
*/ */
fun handleNavigateSearch(extraAction: ExtraAction = ExtraAction.NONE) fun handleNavigateSearch()
} }
/** /**
@ -74,11 +73,10 @@ class DefaultToolbarController(
navController.nav(navController.currentDestination?.id, directions) navController.nav(navController.currentDestination?.id, directions)
} }
override fun handleNavigateSearch(extraAction: ExtraAction) { override fun handleNavigateSearch() {
val directions = val directions =
NavGraphDirections.actionGlobalSearchDialog( NavGraphDirections.actionGlobalSearchDialog(
sessionId = null, sessionId = null,
extraAction = extraAction,
) )
navController.nav( navController.nav(
@ -87,13 +85,6 @@ class DefaultToolbarController(
BrowserAnimator.getToolbarNavOptions(activity), BrowserAnimator.getToolbarNavOptions(activity),
) )
when (extraAction) { Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME"))
ExtraAction.QR_READER ->
Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME_QR"))
ExtraAction.VOICE_SEARCH ->
Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME_VOICE"))
ExtraAction.NONE ->
Events.searchBarTapped.record(Events.SearchBarTappedExtra("HOME"))
}
} }
} }

View file

@ -4,8 +4,6 @@
package org.mozilla.fenix.home.toolbar package org.mozilla.fenix.home.toolbar
import org.mozilla.fenix.search.ExtraAction
/** /**
* Interface for toolbar related actions. * Interface for toolbar related actions.
*/ */
@ -26,8 +24,6 @@ interface ToolbarInteractor {
/** /**
* Navigates to the search dialog. * Navigates to the search dialog.
*
* @param extraAction specify [ExtraAction] that needs to be performed in the search dialog.
*/ */
fun onNavigateSearch(extraAction: ExtraAction = ExtraAction.NONE) fun onNavigateSearch()
} }

View file

@ -83,7 +83,6 @@ 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.HISTORY_SEARCH_ENGINE_ID
import org.mozilla.fenix.components.Core.Companion.TABS_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.appstate.AppAction
import org.mozilla.fenix.components.toolbar.IncompleteRedesignToolbarFeature
import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.databinding.FragmentSearchDialogBinding import org.mozilla.fenix.databinding.FragmentSearchDialogBinding
import org.mozilla.fenix.databinding.SearchSuggestionsHintBinding import org.mozilla.fenix.databinding.SearchSuggestionsHintBinding
@ -105,13 +104,6 @@ import org.mozilla.fenix.settings.SupportUtils
typealias SearchDialogFragmentStore = SearchFragmentStore typealias SearchDialogFragmentStore = SearchFragmentStore
/**
* Possible extra actions when opening search dialog fragment.
*/
enum class ExtraAction {
QR_READER, VOICE_SEARCH, NONE,
}
@SuppressWarnings("LargeClass", "TooManyFunctions") @SuppressWarnings("LargeClass", "TooManyFunctions")
class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
private var _binding: FragmentSearchDialogBinding? = null private var _binding: FragmentSearchDialogBinding? = null
@ -260,12 +252,6 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
binding.toolbar, binding.toolbar,
fromHomeFragment, fromHomeFragment,
).also { ).also {
if (!IncompleteRedesignToolbarFeature(requireContext().settings()).isEnabled) {
it.view.hidePageActionSeparator()
} else {
it.view.showPageActionSeparator()
it.view.isNavBarEnabled = true
}
inlineAutocompleteEditText = it.view.findViewById(R.id.mozac_browser_toolbar_edit_url_view) inlineAutocompleteEditText = it.view.findViewById(R.id.mozac_browser_toolbar_edit_url_view)
inlineAutocompleteEditText.increaseTapArea(TAP_INCREASE_DPS_4) inlineAutocompleteEditText.increaseTapArea(TAP_INCREASE_DPS_4)
} }
@ -482,13 +468,6 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
updateQrButton(it) updateQrButton(it)
updateVoiceSearchButton() updateVoiceSearchButton()
} }
val args by navArgs<SearchDialogFragmentArgs>()
when (args.extraAction) {
ExtraAction.QR_READER -> launchQr()
ExtraAction.VOICE_SEARCH -> launchVoiceSearch()
ExtraAction.NONE -> {}
}
} }
/** /**

View file

@ -19,8 +19,6 @@ import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.browser.tabstrip.isTabStripEnabled import org.mozilla.fenix.browser.tabstrip.isTabStripEnabled
import org.mozilla.fenix.components.Components 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.SearchEngineSource
import org.mozilla.fenix.search.SearchFragmentState import org.mozilla.fenix.search.SearchFragmentState
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
@ -102,21 +100,10 @@ class ToolbarView(
R.color.suggestion_highlight_color, R.color.suggestion_highlight_color,
), ),
clear = context.getColorFromAttr(R.attr.textPrimary), clear = context.getColorFromAttr(R.attr.textPrimary),
erase = 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( edit.setUrlBackground(
AppCompatResources.getDrawable( AppCompatResources.getDrawable(context, R.drawable.search_url_background),
context,
searchUrlBackground,
),
) )
private = isPrivate private = isPrivate

View file

@ -1,12 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- 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/. -->
<shape xmlns:android="http://schemas.android.com/apk/res/android">
<solid android:color="?layer3"/>
<corners android:bottomRightRadius="8dp"
android:bottomLeftRadius="8dp"
android:topLeftRadius="8dp"
android:topRightRadius="8dp"/>
</shape>

View file

@ -3,10 +3,10 @@
- License, v. 2.0. If a copy of the MPL was not distributed with this - 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/. --> - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<shape xmlns:android="http://schemas.android.com/apk/res/android"> <shape xmlns:android="http://schemas.android.com/apk/res/android">
<solid android:color="?layerSearch"/> <solid android:color="?layer3"/>
<corners android:bottomRightRadius="8dp" <corners android:bottomRightRadius="8dp"
android:bottomLeftRadius="8dp" android:bottomLeftRadius="8dp"
android:topLeftRadius="8dp" android:topLeftRadius="8dp"
android:topRightRadius="8dp"/> android:topRightRadius="8dp"/>
</shape> </shape>

View file

@ -151,7 +151,7 @@
<TextView <TextView
android:id="@+id/toolbar" android:id="@+id/toolbar"
android:layout_width="0dp" android:layout_width="wrap_content"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_gravity="start|center_vertical" android:layout_gravity="start|center_vertical"
android:clickable="false" android:clickable="false"
@ -161,52 +161,7 @@
android:lines="1" android:lines="1"
android:text="@string/search_hint" android:text="@string/search_hint"
android:textColor="?attr/textPrimary" android:textColor="?attr/textPrimary"
android:textSize="15sp" android:textSize="15sp" />
android:layout_weight="1"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintStart_toEndOf="@id/search_selector_button"
app:layout_constraintEnd_toStartOf="@id/browser_action_separator" />
<View
android:id="@+id/browser_action_separator"
android:layout_width="@dimen/mozac_browser_toolbar_page_action_separator_width"
android:layout_height="match_parent"
android:layout_marginStart="8dp"
android:importantForAccessibility="no"
android:scaleType="center"
android:background="?homeBackground"
android:gravity="end"
android:visibility="gone"
app:layout_constraintStart_toEndOf="@id/toolbar"
app:layout_constraintEnd_toStartOf="@id/microphone_action_image"
app:layout_constraintTop_toTopOf="parent" />
<ImageView
android:id="@+id/microphone_action_image"
app:srcCompat="@drawable/ic_microphone"
android:paddingHorizontal="8dp"
android:paddingVertical="8dp"
android:layout_width="40dp"
android:layout_height="match_parent"
android:gravity="end"
android:visibility="gone"
app:layout_constraintStart_toEndOf="@id/browser_action_separator"
app:layout_constraintEnd_toEndOf="@id/qr_action_image"
app:layout_constraintTop_toTopOf="parent" />
<ImageView
android:id="@+id/qr_action_image"
app:srcCompat="@drawable/ic_qr"
android:paddingHorizontal="8dp"
android:paddingVertical="8dp"
android:layout_width="40dp"
android:layout_height="match_parent"
android:gravity="end"
android:visibility="gone"
app:layout_constraintStart_toEndOf="@id/microphone_action_image"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent" />
</LinearLayout> </LinearLayout>
<androidx.constraintlayout.widget.Barrier <androidx.constraintlayout.widget.Barrier

View file

@ -272,10 +272,6 @@
android:defaultValue="@null" android:defaultValue="@null"
app:argType="string" app:argType="string"
app:nullable="true" /> app:nullable="true" />
<argument
android:name="extra_action"
android:defaultValue="NONE"
app:argType="org.mozilla.fenix.search.ExtraAction" />
</dialog> </dialog>
<fragment <fragment

View file

@ -79,7 +79,6 @@
<!-- Home Fragment --> <!-- Home Fragment -->
<dimen name="home_fragment_top_toolbar_header_margin">60dp</dimen> <dimen name="home_fragment_top_toolbar_header_margin">60dp</dimen>
<dimen name="home_fragment_toolbar_margin">4dp</dimen>
<dimen name="home_item_horizontal_margin">16dp</dimen> <dimen name="home_item_horizontal_margin">16dp</dimen>
<dimen name="home_item_vertical_margin">8dp</dimen> <dimen name="home_item_vertical_margin">8dp</dimen>
<dimen name="wordmark_text_height">18dp</dimen> <dimen name="wordmark_text_height">18dp</dimen>

View file

@ -27,7 +27,6 @@ import org.mozilla.fenix.home.recentvisits.controller.RecentVisitsController
import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController
import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor
import org.mozilla.fenix.home.toolbar.ToolbarController import org.mozilla.fenix.home.toolbar.ToolbarController
import org.mozilla.fenix.search.ExtraAction
import org.mozilla.fenix.search.toolbar.SearchSelectorController import org.mozilla.fenix.search.toolbar.SearchSelectorController
class SessionControlInteractorTest { class SessionControlInteractorTest {
@ -148,18 +147,6 @@ class SessionControlInteractorTest {
verify { toolbarController.handleNavigateSearch() } verify { toolbarController.handleNavigateSearch() }
} }
@Test
fun onNavigateSearchWithQr() {
interactor.onNavigateSearch(ExtraAction.QR_READER)
verify { toolbarController.handleNavigateSearch(ExtraAction.QR_READER) }
}
@Test
fun onNavigateSearchWithVoice() {
interactor.onNavigateSearch(ExtraAction.VOICE_SEARCH)
verify { toolbarController.handleNavigateSearch(ExtraAction.VOICE_SEARCH) }
}
@Test @Test
fun onRemoveCollectionsPlaceholder() { fun onRemoveCollectionsPlaceholder() {
interactor.onRemoveCollectionsPlaceholder() interactor.onRemoveCollectionsPlaceholder()

View file

@ -29,7 +29,6 @@ import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.search.ExtraAction
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
@RunWith(FenixRobolectricTestRunner::class) // For gleanTestRule @RunWith(FenixRobolectricTestRunner::class) // For gleanTestRule
@ -134,54 +133,6 @@ class DefaultToolbarControllerTest {
} }
} }
@Test
@Suppress("Deprecation")
fun `WHEN the toolbar QR button is tapped THEN navigate to the search dialog with QR reader activated`() {
assertNull(Events.searchBarTapped.testGetValue())
createController().handleNavigateSearch(ExtraAction.QR_READER)
assertNotNull(Events.searchBarTapped.testGetValue())
val recordedEvents = Events.searchBarTapped.testGetValue()!!
assertEquals(1, recordedEvents.size)
assertEquals("HOME_QR", recordedEvents.single().extra?.getValue("source"))
verify {
navController.navigate(
match<NavDirections> {
it.actionId == R.id.action_global_search_dialog &&
it.arguments.get("extra_action") as ExtraAction == ExtraAction.QR_READER
},
any<NavOptions>(),
)
}
}
@Test
@Suppress("Deprecation")
fun `WHEN the toolbar VOICE button is tapped THEN navigate to the search dialog with voice search activated`() {
assertNull(Events.searchBarTapped.testGetValue())
createController().handleNavigateSearch(ExtraAction.VOICE_SEARCH)
assertNotNull(Events.searchBarTapped.testGetValue())
val recordedEvents = Events.searchBarTapped.testGetValue()!!
assertEquals(1, recordedEvents.size)
assertEquals("HOME_VOICE", recordedEvents.single().extra?.getValue("source"))
verify {
navController.navigate(
match<NavDirections> {
it.actionId == R.id.action_global_search_dialog &&
it.arguments.get("extra_action") as ExtraAction == ExtraAction.VOICE_SEARCH
},
any<NavOptions>(),
)
}
}
private fun createController() = DefaultToolbarController( private fun createController() = DefaultToolbarController(
activity = activity, activity = activity,
store = store, store = store,