commit dd5917dee54219b497c5f7d5c766b7f6a7ab5faa
parent 92cf2b2fec555934fa986c5dd426edd51a460fe4
Author: rmalicdem <rmalicdem@mozilla.com>
Date: Wed, 1 Oct 2025 17:18:59 +0000
Bug 1988235 - Tighten the layout for address bar and navigation bar when shown together r=android-reviewers,petru
Differential Revision: https://phabricator.services.mozilla.com/D266446
Diffstat:
7 files changed, 114 insertions(+), 23 deletions(-)
diff --git a/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/BrowserDisplayToolbar.kt b/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/BrowserDisplayToolbar.kt
@@ -124,7 +124,10 @@ fun BrowserDisplayToolbar(
true -> TOOLBAR_PADDING_DP.dp
false -> NO_TOOLBAR_PADDING_DP.dp
},
- bottom = TOOLBAR_PADDING_DP.dp,
+ bottom = when (gravity) {
+ Top -> TOOLBAR_PADDING_DP
+ Bottom -> if (browserActionsEnd.isEmpty()) NO_TOOLBAR_PADDING_DP else TOOLBAR_PADDING_DP
+ }.dp,
)
.height(48.dp)
.background(
diff --git a/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/NavigationBar.kt b/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/NavigationBar.kt
@@ -44,7 +44,7 @@ fun NavigationBar(
) {
Box(
modifier = Modifier
- .height(60.dp)
+ .height(if (toolbarGravity == Top) 60.dp else 48.dp)
.background(color = AcornTheme.colors.layer1)
.pointerInput(Unit) {
awaitPointerEventScope {
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/ext/Fragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/ext/Fragment.kt
@@ -229,8 +229,8 @@ fun Fragment.isLargeScreenSize(): Boolean {
return requireContext().isLargeScreenSize()
}
-private const val TALL_SCREEN_HEIGHT_DP = 480
-private const val WIDE_SCREEN_WIDTH_DP = 600
+internal const val TALL_SCREEN_HEIGHT_DP = 480
+internal const val WIDE_SCREEN_WIDTH_DP = 600
/**
* Helper function to determine whether the app's current window height
@@ -273,6 +273,7 @@ fun Fragment.getBottomToolbarHeight(includeNavBarIfEnabled: Boolean = true): Int
val isMicrosurveyEnabled = settings.shouldShowMicrosurveyPrompt
val isToolbarAtBottom = settings.toolbarPosition == ToolbarPosition.BOTTOM
+ val isNavBarEnabled = settings.shouldUseExpandedToolbar && isTallWindow() && !isWideWindow()
val microsurveyHeight = if (isMicrosurveyEnabled) {
pixelSizeFor(R.dimen.browser_microsurvey_height)
@@ -287,8 +288,14 @@ fun Fragment.getBottomToolbarHeight(includeNavBarIfEnabled: Boolean = true): Int
}
val navBarHeight =
- if (includeNavBarIfEnabled && settings.shouldUseExpandedToolbar && isTallWindow()) {
- pixelSizeFor(R.dimen.browser_navbar_height)
+ if (includeNavBarIfEnabled && isNavBarEnabled) {
+ pixelSizeFor(
+ if (settings.shouldUseComposableToolbar && isToolbarAtBottom) {
+ R.dimen.browser_navbar_height_small
+ } else {
+ R.dimen.browser_navbar_height
+ },
+ )
} else {
0
}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
@@ -46,6 +46,8 @@ import org.mozilla.fenix.components.settings.featureFlagPreference
import org.mozilla.fenix.components.settings.lazyFeatureFlagPreference
import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.debugsettings.addresses.SharedPrefsAddressesDebugLocalesRepository
+import org.mozilla.fenix.ext.TALL_SCREEN_HEIGHT_DP
+import org.mozilla.fenix.ext.WIDE_SCREEN_WIDTH_DP
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getPreferenceKey
import org.mozilla.fenix.ext.pixelSizeFor
@@ -2455,7 +2457,17 @@ class Settings(private val appContext: Context) : PreferencesHolder {
val browserToolbarHeight: Int
get() = appContext.pixelSizeFor(
when (shouldUseComposableToolbar) {
- true -> R.dimen.composable_browser_toolbar_height
+ true -> {
+ val isTallWindow = appContext.resources.configuration.screenHeightDp > TALL_SCREEN_HEIGHT_DP
+ val isWideWindow = appContext.resources.configuration.screenWidthDp > WIDE_SCREEN_WIDTH_DP
+ when (
+ toolbarPosition == ToolbarPosition.BOTTOM && shouldUseExpandedToolbar &&
+ isTallWindow && !isWideWindow
+ ) {
+ true -> R.dimen.composable_browser_toolbar_height_small
+ false -> R.dimen.composable_browser_toolbar_height
+ }
+ }
false -> R.dimen.browser_toolbar_height
},
)
diff --git a/mobile/android/fenix/app/src/main/res/values/dimens.xml b/mobile/android/fenix/app/src/main/res/values/dimens.xml
@@ -66,8 +66,10 @@
<!-- Browser Toolbar -->
<dimen name="composable_browser_toolbar_height">64dp</dimen>
+ <dimen name="composable_browser_toolbar_height_small">56dp</dimen>
<dimen name="browser_toolbar_height">56dp</dimen>
<dimen name="browser_navbar_height">60dp</dimen>
+ <dimen name="browser_navbar_height_small">48dp</dimen>
<dimen name="tab_strip_height">56dp</dimen>
<dimen name="browser_microsurvey_height">131dp</dimen>
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/ext/FragmentTest.kt
@@ -19,6 +19,7 @@ import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import junit.framework.TestCase.assertEquals
+import mozilla.components.support.test.robolectric.testContext
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
@@ -38,7 +39,7 @@ class FragmentTest {
private val navController: NavController = mockk(relaxed = true)
private val mockOptions: NavOptions = mockk(relaxed = true)
private val navControllerProvider: NavControllerProvider = mockk(relaxed = true)
- private val settings: Settings = mockk(relaxed = true)
+ private val settings: Settings = spyk(Settings(testContext))
private lateinit var mockContext: Context
private lateinit var fragment: Fragment
@@ -54,10 +55,10 @@ class FragmentTest {
every { navControllerProvider.getNavController(fragment) } returns navController
every { navController.currentDestination } returns mockDestination
every { mockDestination.id } returns mockId
- every { settings.browserToolbarHeight } returns 64
every { mockContext.resources.getDimensionPixelSize(R.dimen.tab_strip_height) } returns 56
every { mockContext.resources.getDimensionPixelSize(R.dimen.browser_microsurvey_height) } returns 131
every { mockContext.resources.getDimensionPixelSize(R.dimen.browser_navbar_height) } returns 60
+ every { mockContext.resources.getDimensionPixelSize(R.dimen.browser_navbar_height_small) } returns 48
}
@Test
@@ -82,45 +83,50 @@ class FragmentTest {
@Test
fun `GIVEN the top addressbar and tabstrip are shown WHEN getTopToolbarHeight THEN return their combined height`() {
+ every { settings.shouldUseComposableToolbar } returns false
every { settings.isTabStripEnabled } returns true
every { settings.toolbarPosition } returns ToolbarPosition.TOP
val topToolbarHeight = fragment.getTopToolbarHeight()
- assertEquals(120, topToolbarHeight)
+ assertEquals(112, topToolbarHeight)
}
@Test
fun `GIVEN the top addressbar and tabstrip are shown WHEN getTopToolbarHeight with tabstrip excluded THEN return the addressbar height`() {
+ every { settings.shouldUseComposableToolbar } returns false
every { settings.isTabStripEnabled } returns true
every { settings.toolbarPosition } returns ToolbarPosition.TOP
val topToolbarHeight = fragment.getTopToolbarHeight(includeTabStripIfAvailable = false)
- assertEquals(64, topToolbarHeight)
+ assertEquals(56, topToolbarHeight)
}
@Test
fun `GIVEN only the top addressbar shown WHEN getTopToolbarHeight with tabstrip included THEN return the addressbar height`() {
+ every { settings.shouldUseComposableToolbar } returns false
every { settings.isTabStripEnabled } returns false
every { settings.toolbarPosition } returns ToolbarPosition.TOP
val topToolbarHeight = fragment.getTopToolbarHeight(includeTabStripIfAvailable = true)
- assertEquals(64, topToolbarHeight)
+ assertEquals(56, topToolbarHeight)
}
@Test
fun `GIVEN only the top addressbar shown WHEN getTopToolbarHeight THEN return the addressbar height`() {
+ every { settings.shouldUseComposableToolbar } returns false
every { settings.toolbarPosition } returns ToolbarPosition.TOP
val topToolbarHeight = fragment.getTopToolbarHeight()
- assertEquals(64, topToolbarHeight)
+ assertEquals(56, topToolbarHeight)
}
@Test
fun `GIVEN the top addressbar or tabstrip not shown WHEN getTopToolbarHeight THEN return 0`() {
+ every { settings.shouldUseComposableToolbar } returns false
every { settings.isTabStripEnabled } returns false
every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM
@@ -137,7 +143,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(195, bottomToolbarHeight)
+ assertEquals(187, bottomToolbarHeight)
}
@Test
@@ -169,7 +175,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(64, bottomToolbarHeight)
+ assertEquals(56, bottomToolbarHeight)
}
@Test
@@ -198,11 +204,16 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(255, bottomToolbarHeight)
+ assertEquals(247, bottomToolbarHeight)
}
@Test
- fun `GIVEN the address bar, navigation bar and the microsurvey are shown at bottom WHEN getBottomToolbarHeight with excluded navigation bar THEN returns the combined height minus navigation bar`() {
+ @Config(qualifiers = "h481dp") // navbar is only shown on screens taller than 480dp
+ fun `GIVEN the composable toolbar, navigation bar and the microsurvey are shown at bottom WHEN getBottomToolbarHeight with excluded navigation bar THEN returns the combined height minus navigation bar`() {
+ val configuration = Configuration().apply {
+ screenHeightDp = 481
+ }
+ every { mockContext.resources.configuration } returns configuration
every { settings.shouldUseComposableToolbar } returns true
every { settings.shouldShowMicrosurveyPrompt } returns true
every { settings.shouldUseExpandedToolbar } returns true
@@ -210,7 +221,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight(includeNavBarIfEnabled = false)
- assertEquals(195, bottomToolbarHeight)
+ assertEquals(187, bottomToolbarHeight)
}
@Test
@@ -227,7 +238,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(255, bottomToolbarHeight)
+ assertEquals(235, bottomToolbarHeight)
}
@Test
@@ -271,7 +282,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(124, bottomToolbarHeight)
+ assertEquals(116, bottomToolbarHeight)
}
@Test
@@ -283,7 +294,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight(includeNavBarIfEnabled = false)
- assertEquals(64, bottomToolbarHeight)
+ assertEquals(56, bottomToolbarHeight)
}
@Test
@@ -300,7 +311,7 @@ class FragmentTest {
val bottomToolbarHeight = fragment.getBottomToolbarHeight()
- assertEquals(124, bottomToolbarHeight)
+ assertEquals(104, bottomToolbarHeight)
}
@Test
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt
@@ -4,6 +4,7 @@
package org.mozilla.fenix.utils
+import android.content.res.Configuration
import androidx.core.content.edit
import io.mockk.every
import io.mockk.spyk
@@ -24,6 +25,7 @@ import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
+import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.nimbus.FakeNimbusEventStore
import org.mozilla.fenix.settings.PhoneFeature
import org.mozilla.fenix.settings.deletebrowsingdata.DeleteBrowsingDataOnQuitType
@@ -1041,9 +1043,63 @@ class SettingsTest {
}
@Test
- fun `GIVEN composable toolbar is enabled WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
+ fun `GIVEN top composable toolbar is enabled WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
val settings = spyk(settings)
every { settings.shouldUseComposableToolbar } returns true
+ every { settings.toolbarPosition } returns ToolbarPosition.TOP
+
+ assertEquals(64, settings.browserToolbarHeight)
+ }
+
+ @Test
+ fun `GIVEN bottom composable toolbar is enabled and navigation bar is disabled WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
+ val settings = spyk(settings)
+ every { settings.shouldUseComposableToolbar } returns true
+ every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM
+ every { settings.shouldUseExpandedToolbar } returns false
+
+ assertEquals(64, settings.browserToolbarHeight)
+ }
+
+ @Test fun `GIVEN bottom composable toolbar is enabled and navigation bar is enabled WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
+ testContext.resources.configuration.apply {
+ screenHeightDp = 481
+ screenWidthDp = 599
+ }
+ val settings = spyk(settings)
+ every { settings.shouldUseComposableToolbar } returns true
+ every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM
+ every { settings.shouldUseExpandedToolbar } returns true
+
+ assertEquals(56, settings.browserToolbarHeight)
+ }
+
+ @Test
+ fun `GIVEN bottom composable toolbar is enabled and navigation bar is enabled but window is not tall enough WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
+ testContext.resources.configuration.apply {
+ screenHeightDp = 479
+ screenWidthDp = 599
+ }
+ val settings = spyk(settings)
+
+ every { settings.shouldUseComposableToolbar } returns true
+ every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM
+ every { settings.shouldUseExpandedToolbar } returns true
+
+ assertEquals(64, settings.browserToolbarHeight)
+ }
+
+ @Test
+ fun `GIVEN bottom composable toolbar is enabled and navigation bar is enabled but window is too wide WHEN querying the toolbar height THEN get the height of the composable toolbar`() {
+ testContext.resources.configuration.apply {
+ screenHeightDp = 481
+ screenWidthDp = 601
+ }
+ val settings = spyk(settings)
+
+ every { settings.shouldUseComposableToolbar } returns true
+ every { settings.toolbarPosition } returns ToolbarPosition.BOTTOM
+ every { settings.shouldUseExpandedToolbar } returns true
assertEquals(64, settings.browserToolbarHeight)
}
