Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
For #8795: remove redundant ConstraintLayout around BrowserToolbar.
Browse files Browse the repository at this point in the history
This is functionally equivalent to the code before this patch but should
be slightly more performant in theory because ConstraintLayout is
expensive to inflate.

The elevation and layoutParams set dynamically appeared to have no effect
with the wrapping view but broke the view when used by itself so I had
to remove them. I also updated a few other unnecessary params.

Theoretically this may have some perf benefits but I didn't see anything
outside noise levels after I took the numbers (but I didn't try very
hard).
  • Loading branch information
mcomella committed Jun 26, 2020
1 parent 64947a8 commit 708ba6d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 41 deletions.
2 changes: 0 additions & 2 deletions app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,6 @@ class SearchFragment : Fragment(), UserInteractionHandler {

view.search_suggestions_onboarding.setOnInflateListener((stubListener))

view.toolbar_wrapper.clipToOutline = false

fill_link_from_clipboard.setOnClickListener {
(activity as HomeActivity)
.openToBrowserAndLoad(
Expand Down
10 changes: 0 additions & 10 deletions app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ import android.content.Context
import android.graphics.Bitmap
import android.graphics.drawable.BitmapDrawable
import androidx.appcompat.content.res.AppCompatResources
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.content.ContextCompat
import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider
import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.storage.HistoryStorage
import mozilla.components.feature.toolbar.ToolbarAutocompleteFeature
import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.ktx.android.util.dpToPx
import mozilla.components.support.ktx.android.view.hideKeyboard
import org.mozilla.fenix.R
import org.mozilla.fenix.search.SearchFragmentState
Expand Down Expand Up @@ -64,8 +62,6 @@ class ToolbarView(
view.apply {
editMode()

elevation = TOOLBAR_ELEVATION_IN_DP.dpToPx(resources.displayMetrics).toFloat()

setOnUrlCommitListener {
// We're hiding the keyboard as early as possible to prevent the engine view
// from resizing in case the BrowserFragment is being displayed before the
Expand All @@ -80,8 +76,6 @@ class ToolbarView(
context, ThemeManager.resolveAttribute(R.attr.foundation, context)
)

layoutParams.height = CoordinatorLayout.LayoutParams.MATCH_PARENT

edit.hint = context.getString(R.string.search_hint)

edit.colors = edit.colors.copy(
Expand Down Expand Up @@ -156,8 +150,4 @@ class ToolbarView(

view.edit.setIcon(icon, searchState.searchEngineSource.searchEngine.name)
}

companion object {
private const val TOOLBAR_ELEVATION_IN_DP = 16
}
}
44 changes: 15 additions & 29 deletions app/src/main/res/layout/fragment_search.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,24 @@
android:background="?foundation"
tools:context=".search.SearchFragment">

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/toolbar_wrapper"
<mozilla.components.browser.toolbar.BrowserToolbar
android:id="@+id/toolbar"
android:layout_width="0dp"
android:layout_height="@dimen/browser_toolbar_height"
android:layout_margin="0dp"
android:outlineProvider="paddedBounds"
android:background="@drawable/toolbar_background_top"
android:clickable="true"
android:focusable="true"
android:focusableInTouchMode="true"
app:layout_scrollFlags="scroll|enterAlways|snap|exitUntilCollapsed"
app:browserToolbarClearColor="?primaryText"
app:browserToolbarInsecureColor="?primaryText"
app:browserToolbarMenuColor="?primaryText"
app:browserToolbarProgressBarGravity="bottom"
app:browserToolbarSecureColor="?primaryText"
app:browserToolbarTrackingProtectionAndSecurityIndicatorSeparatorColor="?toolbarDivider"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent">

<mozilla.components.browser.toolbar.BrowserToolbar
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="@dimen/browser_toolbar_height"
android:layout_gravity="top"
android:background="@drawable/toolbar_background_top"
android:clickable="true"
android:focusable="true"
android:focusableInTouchMode="true"
app:layout_scrollFlags="scroll|enterAlways|snap|exitUntilCollapsed"
app:browserToolbarClearColor="?primaryText"
app:browserToolbarInsecureColor="?primaryText"
app:browserToolbarMenuColor="?primaryText"
app:browserToolbarProgressBarGravity="bottom"
app:browserToolbarSecureColor="?primaryText"
app:browserToolbarTrackingProtectionAndSecurityIndicatorSeparatorColor="?toolbarDivider"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"/>

</androidx.constraintlayout.widget.ConstraintLayout>
app:layout_constraintTop_toTopOf="parent"/>

<androidx.core.widget.NestedScrollView
android:layout_width="0dp"
Expand All @@ -51,7 +37,7 @@
app:layout_constraintBottom_toBottomOf="@id/search_divider"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/toolbar_wrapper">
app:layout_constraintTop_toBottomOf="@id/toolbar">

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/scrollable_area"
Expand Down

0 comments on commit 708ba6d

Please sign in to comment.