Skip to content

Commit

Permalink
For mozilla-mobile#5694: Adds changing toolbar position functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
sblatz committed Nov 15, 2019
1 parent ccc996e commit 756578c
Show file tree
Hide file tree
Showing 18 changed files with 189 additions and 41 deletions.
2 changes: 1 addition & 1 deletion app/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ toolbar_settings:
bugs:
- https://github.com/mozilla-mobile/fenix/issue/6054
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/TODO
- https://github.com/mozilla-mobile/fenix/pull/6571
notification_emails:
- [email protected]
expires: "2020-03-01"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import androidx.lifecycle.lifecycleScope
import androidx.navigation.NavDirections
import androidx.navigation.fragment.findNavController
import com.google.android.material.snackbar.Snackbar
import kotlinx.android.synthetic.main.component_search.toolbar
import kotlinx.android.synthetic.main.fragment_browser.*
import kotlinx.android.synthetic.main.fragment_browser.view.*
import kotlinx.coroutines.Dispatchers.IO
Expand Down Expand Up @@ -197,6 +196,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs

browserToolbarView = BrowserToolbarView(
container = view.browserLayout,
shouldUseBottomToolbar = context.settings().shouldUseBottomToolbar,
interactor = browserInteractor,
customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) }
)
Expand All @@ -213,7 +213,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
sessionId = customTabSessionId,
stub = view.stubFindInPage,
engineView = view.engineView,
toolbar = toolbar
toolbar = browserToolbarView.view
),
owner = this,
view = view
Expand Down Expand Up @@ -350,14 +350,14 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
activity?.requestedOrientation =
ActivityInfo.SCREEN_ORIENTATION_USER_LANDSCAPE
activity?.enterToImmersiveMode()
toolbar.visibility = View.GONE
browserToolbarView.view.visibility = View.GONE
} else {
activity?.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_USER
activity?.exitImmersiveModeIfNeeded()
(activity as? HomeActivity)?.let { activity ->
activity.themeManager.applyStatusBarTheme(activity)
}
toolbar.visibility = View.VISIBLE
browserToolbarView.view.visibility = View.VISIBLE
}
updateLayoutMargins(inFullScreen)
},
Expand Down
14 changes: 14 additions & 0 deletions app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import android.widget.ImageView
import android.widget.PopupWindow
import android.widget.RadioButton
import androidx.appcompat.widget.AppCompatImageView
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.content.ContextCompat
import androidx.lifecycle.Observer
import androidx.transition.TransitionInflater
import com.google.android.material.snackbar.Snackbar
import kotlinx.android.synthetic.main.fragment_browser.*
import kotlinx.android.synthetic.main.fragment_browser.view.*
import kotlinx.android.synthetic.main.fragment_home.*
import kotlinx.android.synthetic.main.tracking_protection_onboarding_popup.view.*
Expand Down Expand Up @@ -128,6 +130,18 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler {
super.onStart()
subscribeToTabCollections()
getSessionById()?.register(toolbarSessionObserver, this, autoPause = true)

updateToolbar()
}

private fun updateToolbar() {
val browserEngine = swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams

browserEngine.bottomMargin = if (requireContext().settings().shouldUseBottomToolbar) {
requireContext().dimen(R.dimen.browser_toolbar_height)
} else {
0
}
}

private val toolbarSessionObserver = object : Session.Observer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package org.mozilla.fenix.browser
import android.animation.ValueAnimator
import android.content.Context
import android.util.AttributeSet
import android.util.Log
import android.view.Gravity
import android.view.View
import android.view.View.SCROLL_AXIS_VERTICAL
Expand Down Expand Up @@ -96,6 +97,8 @@ class BrowserToolbarTopBehavior(
override fun layoutDependsOn(parent: CoordinatorLayout, child: BrowserToolbar, dependency: View): Boolean {
if (dependency is Snackbar.SnackbarLayout) {
positionSnackbar(dependency)
} else {
Log.d("Sawyer", "depenedency: $dependency")
}

return super.layoutDependsOn(parent, child, dependency)
Expand All @@ -111,7 +114,7 @@ class BrowserToolbarTopBehavior(
val params = view.layoutParams as CoordinatorLayout.LayoutParams

// Position the snackbar below the toolbar so that it doesn't overlay the toolbar.
params.anchorId = R.id.quick_action_sheet
params.anchorId = R.id.toolbar
params.anchorGravity = Gravity.BOTTOM or Gravity.CENTER_HORIZONTAL
params.gravity = Gravity.BOTTOM or Gravity.CENTER_HORIZONTAL

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ private val Event.wrapper: EventWrapper<*>?
)
is Event.ViewLoginPassword -> EventWrapper<NoExtraKeys>(
{ Logins.viewPasswordLogin.record(it) }
)
is Event.ToolbarPositionChanged -> EventWrapper(
{ ToolbarSettings.changedPosition.record(it) },
{ ToolbarSettings.changedPositionKeys.valueOf(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import androidx.core.content.ContextCompat
import androidx.core.view.isVisible
import com.google.android.material.snackbar.Snackbar
import kotlinx.android.extensions.LayoutContainer
import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.copy
import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.paste
import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.paste_and_go
import kotlinx.android.synthetic.main.browser_toolbar_popup_window.view.*
import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider
import mozilla.components.browser.session.Session
import mozilla.components.browser.toolbar.BrowserToolbar
Expand All @@ -29,6 +27,7 @@ import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.customtabs.CustomTabToolbarMenu
import org.mozilla.fenix.ext.bookmarkStorage
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.theme.ThemeManager

interface BrowserToolbarViewInteractor {
Expand All @@ -41,19 +40,29 @@ interface BrowserToolbarViewInteractor {

class BrowserToolbarView(
private val container: ViewGroup,
private val shouldUseBottomToolbar: Boolean,
private val interactor: BrowserToolbarViewInteractor,
private val customTabSession: Session?
) : LayoutContainer {

override val containerView: View?
get() = container

private val urlBackground = LayoutInflater.from(container.context)
.inflate(R.layout.layout_url_background, container, false)

val view: BrowserToolbar = LayoutInflater.from(container.context)
.inflate(R.layout.component_search, container, true)
.findViewById(R.id.toolbar)
val view: BrowserToolbar = if (container.context.settings().shouldUseBottomToolbar) {
LayoutInflater.from(container.context)
.inflate(R.layout.component_bottom_browser_toolbar, container, true)
.findViewById(R.id.toolbar_bottom)
} else {
if (container.context.settings().shouldUseFixedToolbar) {
LayoutInflater.from(container.context)
.inflate(R.layout.component_browser_top_toolbar_fixed, container, true)
.findViewById(R.id.toolbar_top_fixed)
} else {
LayoutInflater.from(container.context)
.inflate(R.layout.component_browser_bottom_toolbar, container, true)
.findViewById(R.id.toolbar_top)
}
}

val toolbarIntegration: ToolbarIntegration

Expand Down Expand Up @@ -180,6 +189,7 @@ class BrowserToolbarView(
readerModeStateProvider = {
sessionManager.selectedSession?.readerMode ?: false
},
shouldReverseItems = !shouldUseBottomToolbar,
onItemTapped = { interactor.onBrowserToolbarMenuItemTapped(it) },
lifecycleOwner = container.context as AppCompatActivity,
sessionManager = sessionManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import kotlinx.coroutines.launch
import mozilla.components.browser.menu.BrowserMenuBuilder
import mozilla.components.browser.menu.item.BrowserMenuDivider
import mozilla.components.browser.menu.item.BrowserMenuHighlightableItem
import mozilla.components.browser.menu.item.BrowserMenuImageSwitch
import mozilla.components.browser.menu.item.BrowserMenuImageText
import mozilla.components.browser.menu.item.BrowserMenuItemToolbar
import mozilla.components.browser.menu.item.BrowserMenuImageSwitch
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.concept.storage.BookmarksStorage
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.ext.asActivity
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.theme.ThemeManager
Expand All @@ -31,6 +31,7 @@ class DefaultToolbarMenu(
private val context: Context,
private val hasAccountProblem: Boolean = false,
private val requestDesktopStateProvider: () -> Boolean = { false },
private val shouldReverseItems: Boolean,
private val onItemTapped: (ToolbarMenu.Item) -> Unit = {},
private val lifecycleOwner: LifecycleOwner,
private val bookmarksStorage: BookmarksStorage,
Expand Down Expand Up @@ -146,7 +147,7 @@ class DefaultToolbarMenu(
fun shouldShowReaderAppearance(): Boolean =
sessionManager.selectedSession?.readerMode ?: false

listOfNotNull(
val menuItems = listOfNotNull(
help,
settings,
library,
Expand All @@ -164,6 +165,8 @@ class DefaultToolbarMenu(
BrowserMenuDivider(),
menuToolbar
)

if (shouldReverseItems) { menuItems.reversed() } else { menuItems }
}

private val help = BrowserMenuImageText(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.mozilla.fenix.components.toolbar

import android.annotation.SuppressLint
import android.content.Context
import android.util.AttributeSet
import android.view.View
import androidx.coordinatorlayout.widget.CoordinatorLayout
import mozilla.components.concept.engine.EngineView
import mozilla.components.support.ktx.android.view.findViewInHierarchy

/**
* A [CoordinatorLayout.Behavior] implementation to be used with [EngineView] when placing a toolbar at the
* bottom of the screen.
*
* Using this behavior requires the toolbar to use the BrowserToolbarTopBehavior.
*
* This implementation will update the vertical clipping of the [EngineView] so that top-aligned web content will
* be drawn above the native toolbar.
*/
class EngineViewTopBehavior(
context: Context?,
attrs: AttributeSet?
) : CoordinatorLayout.Behavior<View>(context, attrs) {
@SuppressLint("LogUsage")
override fun layoutDependsOn(parent: CoordinatorLayout, child: View, dependency: View): Boolean {
// This package does not have access to "BrowserToolbar" ... so we are just checking the class name here since
// we actually do not need anything from that class - we only need to identify the instance.
// Right now we do not have a component that has access to (concept/browser-toolbar and concept-engine).
// Creating one just for this behavior is too excessive.
if (dependency::class.java.simpleName == "BrowserToolbar") {
return true
}

return super.layoutDependsOn(parent, child, dependency)
}

/**
* Apply vertical clipping to [EngineView]. This requires [EngineViewTopehavior] to be set
* in/on the [EngineView] or its parent. Must be a direct descending child of [CoordinatorLayout].
*/
override fun onDependentViewChanged(parent: CoordinatorLayout, child: View, dependency: View): Boolean {
val engineView = child.findViewInHierarchy { it is EngineView } as EngineView?
engineView?.setVerticalClipping(dependency.height - dependency.translationY.toInt())
return true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import android.view.Gravity
import android.view.View
import androidx.core.view.isGone
import androidx.navigation.fragment.navArgs
import kotlinx.android.synthetic.main.component_search.*
import kotlinx.android.synthetic.main.component_browser_bottom_toolbar.*
import kotlinx.android.synthetic.main.fragment_browser.view.*
import kotlinx.coroutines.ExperimentalCoroutinesApi
import mozilla.components.browser.session.Session
Expand Down Expand Up @@ -64,7 +64,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), BackHandler {
customTabsIntegration.set(
feature = CustomTabsIntegration(
sessionManager = requireComponents.core.sessionManager,
toolbar = toolbar,
toolbar = toolbar_top,
sessionId = customTabSessionId,
activity = activity,
engineLayout = view.swipeRefresh,
Expand All @@ -86,14 +86,14 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), BackHandler {
hideToolbarFeature.set(
feature = WebAppHideToolbarFeature(
requireComponents.core.sessionManager,
toolbar,
toolbar_top,
customTabSessionId,
trustedScopes
) { toolbarVisible ->
updateLayoutMargins(inFullScreen = !toolbarVisible)
},
owner = this,
view = toolbar
view = toolbar_top
)

if (manifest != null) {
Expand Down Expand Up @@ -187,7 +187,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), BackHandler {
}

override fun getEngineMargins(): Pair<Int, Int> {
val toolbarHidden = toolbar.isGone
val toolbarHidden = toolbar_top.isGone
return if (toolbarHidden) {
0 to 0
} else {
Expand Down
19 changes: 16 additions & 3 deletions app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import mozilla.components.feature.toolbar.ToolbarAutocompleteFeature
import mozilla.components.support.ktx.android.util.dpToPx
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.getColorFromAttr
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.search.SearchFragmentState

/**
Expand Down Expand Up @@ -58,9 +59,21 @@ class ToolbarView(
override val containerView: View?
get() = container

val view: BrowserToolbar = LayoutInflater.from(container.context)
.inflate(R.layout.component_search, container, true)
.findViewById(R.id.toolbar)
val view: BrowserToolbar = if (container.context.settings().shouldUseBottomToolbar) {
LayoutInflater.from(container.context)
.inflate(R.layout.component_bottom_browser_toolbar, container, true)
.findViewById(R.id.toolbar_bottom)
} else {
if (container.context.settings().shouldUseFixedToolbar) {
LayoutInflater.from(container.context)
.inflate(R.layout.component_browser_bottom_toolbar, container, true)
.findViewById(R.id.toolbar_top_fixed)
} else {
LayoutInflater.from(container.context)
.inflate(R.layout.component_browser_bottom_toolbar, container, true)
.findViewById(R.id.toolbar_top)
}
}

private var isInitialized = false

Expand Down
19 changes: 10 additions & 9 deletions app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,8 @@ class SettingsFragment : PreferenceFragmentCompat(), AccountObserver {
(activity as AppCompatActivity).supportActionBar?.show()

setSummaryAndTitleStrings()
setupPreferences()

updateAccountUIState(
context!!,
requireComponents.backgroundServices.accountManager.accountProfile()
)

updatePreferenceVisibilityForFeatureFlags()
}

private fun setSummaryAndTitleStrings() {
val trackingProtectionPreference =
findPreference<Preference>(getPreferenceKey(pref_key_tracking_protection_settings))
Expand Down Expand Up @@ -168,6 +160,15 @@ class SettingsFragment : PreferenceFragmentCompat(), AccountObserver {
isVisible =
!PrivateShortcutCreateManager.doesPrivateBrowsingPinnedShortcutExist(context)
}

setupPreferences()

updateAccountUIState(
context!!,
requireComponents.backgroundServices.accountManager.accountProfile()
)

updatePreferenceVisibilityForFeatureFlags()
}

private fun updatePreferenceVisibilityForFeatureFlags() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getPreferenceKey

class ToolbarSettingsFragment: PreferenceFragmentCompat() {
class ToolbarSettingsFragment : PreferenceFragmentCompat() {
private lateinit var topPreference: RadioButtonPreference
private lateinit var bottomPreference: RadioButtonPreference

Expand Down
Loading

0 comments on commit 756578c

Please sign in to comment.