Skip to content

Commit f66b2c4

Browse files
rocketsrogerrocketsroger
committed
Bug 1969041 - Hide navbar in landscape orientation. r=android-reviewers,petru
Differential Revision: https://phabricator.services.mozilla.com/D256780
1 parent 90e3e6c commit f66b2c4

File tree

9 files changed

+176
-116
lines changed

9 files changed

+176
-116
lines changed

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ abstract class BaseBrowserFragment :
13111311
)
13121312
}
13131313

1314-
browserNavigationBar = if (context?.settings()?.shouldUseSimpleToolbar == false) {
1314+
browserNavigationBar =
13151315
BrowserNavigationBar(
13161316
context = activity,
13171317
lifecycleOwner = this,
@@ -1323,9 +1323,6 @@ abstract class BaseBrowserFragment :
13231323
settings = activity.settings(),
13241324
customTabSession = customTabSessionId?.let { store.state.findCustomTab(it) },
13251325
)
1326-
} else {
1327-
null
1328-
}
13291326

13301327
return BrowserToolbarComposable(
13311328
activity = activity,

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserNavigationBar.kt‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,14 @@ class BrowserNavigationBar(
133133
private fun DefaultNavigationBarContent(showDivider: Boolean) {
134134
val uiState by store.observeAsState(initialValue = store.state) { it }
135135

136-
FirefoxTheme {
137-
NavigationBar(
138-
actions = uiState.displayState.navigationActions,
139-
shouldShowDivider = showDivider,
140-
onInteraction = { store.dispatch(it) },
141-
)
136+
if (uiState.displayState.navigationActions.isNotEmpty()) {
137+
FirefoxTheme {
138+
NavigationBar(
139+
actions = uiState.displayState.navigationActions,
140+
shouldShowDivider = showDivider,
141+
onInteraction = { store.dispatch(it) },
142+
)
143+
}
142144
}
143145
}
144146
}

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ import org.mozilla.fenix.components.appstate.AppAction.CurrentTabClosed
9494
import org.mozilla.fenix.components.appstate.AppAction.SnackbarAction.SnackbarDismissed
9595
import org.mozilla.fenix.components.appstate.AppAction.URLCopiedToClipboard
9696
import org.mozilla.fenix.components.appstate.AppAction.UpdateSearchBeingActiveState
97+
import org.mozilla.fenix.components.appstate.OrientationMode
9798
import org.mozilla.fenix.components.appstate.snackbar.SnackbarState
9899
import org.mozilla.fenix.components.menu.MenuAccessPoint
99100
import org.mozilla.fenix.components.metrics.MetricsUtils
@@ -665,11 +666,13 @@ class BrowserToolbarMiddleware(
665666

666667
private fun buildStartBrowserActions(): List<Action> {
667668
val environment = environment ?: return emptyList()
669+
val isLargeWindowOrLandscape = environment.context.isLargeWindow() ||
670+
appStore.state.orientation == OrientationMode.Landscape
668671

669672
return listOf(
670-
ToolbarActionConfig(ToolbarAction.Back) { environment.context.isLargeWindow() },
671-
ToolbarActionConfig(ToolbarAction.Forward) { environment.context.isLargeWindow() },
672-
ToolbarActionConfig(ToolbarAction.RefreshOrStop) { environment.context.isLargeWindow() },
673+
ToolbarActionConfig(ToolbarAction.Back) { isLargeWindowOrLandscape },
674+
ToolbarActionConfig(ToolbarAction.Forward) { isLargeWindowOrLandscape },
675+
ToolbarActionConfig(ToolbarAction.RefreshOrStop) { isLargeWindowOrLandscape },
673676
).filter { config ->
674677
config.isVisible()
675678
}.map { config ->
@@ -683,7 +686,8 @@ class BrowserToolbarMiddleware(
683686
browserScreenStore.state.readerModeStatus.isAvailable
684687
},
685688
ToolbarActionConfig(ToolbarAction.Translate) {
686-
browserScreenStore.state.pageTranslationStatus.isTranslationPossible
689+
browserScreenStore.state.pageTranslationStatus.isTranslationPossible &&
690+
!settings.shouldUseSimpleToolbar
687691
},
688692
).filter { config ->
689693
config.isVisible()
@@ -693,16 +697,17 @@ class BrowserToolbarMiddleware(
693697
}
694698

695699
private fun buildEndBrowserActions(): List<Action> {
696-
val environment = environment ?: return emptyList()
700+
val isSimpleOrLandscape = appStore.state.orientation == OrientationMode.Landscape ||
701+
settings.shouldUseSimpleToolbar
697702

698703
return listOf(
699704
ToolbarActionConfig(ToolbarAction.NewTab) {
700-
!settings.isTabStripEnabled && settings.shouldUseSimpleToolbar
705+
!settings.isTabStripEnabled && isSimpleOrLandscape
701706
},
702707
ToolbarActionConfig(ToolbarAction.TabCounter) {
703-
!settings.isTabStripEnabled && settings.shouldUseSimpleToolbar
708+
!settings.isTabStripEnabled && isSimpleOrLandscape
704709
},
705-
ToolbarActionConfig(ToolbarAction.Menu) { settings.shouldUseSimpleToolbar },
710+
ToolbarActionConfig(ToolbarAction.Menu) { isSimpleOrLandscape },
706711
).filter { config ->
707712
config.isVisible()
708713
}.map { config ->
@@ -711,13 +716,20 @@ class BrowserToolbarMiddleware(
711716
}
712717

713718
private fun buildNavigationActions(isBookmarked: Boolean): List<Action> {
719+
val isExpandedAndPortrait = !settings.shouldUseSimpleToolbar &&
720+
appStore.state.orientation == OrientationMode.Portrait
721+
714722
return listOf(
715-
ToolbarActionConfig(ToolbarAction.Bookmark) { !settings.shouldUseSimpleToolbar && !isBookmarked },
716-
ToolbarActionConfig(ToolbarAction.EditBookmark) { !settings.shouldUseSimpleToolbar && isBookmarked },
717-
ToolbarActionConfig(ToolbarAction.Share) { !settings.shouldUseSimpleToolbar },
718-
ToolbarActionConfig(ToolbarAction.NewTab) { !settings.shouldUseSimpleToolbar },
719-
ToolbarActionConfig(ToolbarAction.TabCounter) { !settings.shouldUseSimpleToolbar },
720-
ToolbarActionConfig(ToolbarAction.Menu) { !settings.shouldUseSimpleToolbar },
723+
ToolbarActionConfig(ToolbarAction.Bookmark) {
724+
isExpandedAndPortrait && !isBookmarked
725+
},
726+
ToolbarActionConfig(ToolbarAction.EditBookmark) {
727+
isExpandedAndPortrait && isBookmarked
728+
},
729+
ToolbarActionConfig(ToolbarAction.Share) { isExpandedAndPortrait },
730+
ToolbarActionConfig(ToolbarAction.NewTab) { isExpandedAndPortrait },
731+
ToolbarActionConfig(ToolbarAction.TabCounter) { isExpandedAndPortrait },
732+
ToolbarActionConfig(ToolbarAction.Menu) { isExpandedAndPortrait },
721733
).filter { config ->
722734
config.isVisible()
723735
}.map { config ->
@@ -799,6 +811,7 @@ class BrowserToolbarMiddleware(
799811
appStore.observeWhileActive {
800812
distinctUntilChangedBy { it.orientation }
801813
.collect {
814+
updateStartBrowserActions(store)
802815
updateEndBrowserActions(store)
803816
updateNavigationActions(store)
804817
}

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -317,16 +317,13 @@ class HomeFragment : Fragment() {
317317
}
318318
}
319319

320-
if (!requireContext().settings().shouldUseSimpleToolbar) {
321-
homeNavigationBar = HomeNavigationBar(
322-
context = requireContext(),
323-
lifecycleOwner = this,
324-
container = binding.navigationBarContainer,
325-
appStore = components.appStore,
326-
browserStore = store,
327-
settings = requireContext().settings(),
328-
)
329-
}
320+
homeNavigationBar = HomeNavigationBar(
321+
context = requireContext(),
322+
lifecycleOwner = this,
323+
container = binding.navigationBarContainer,
324+
appStore = components.appStore,
325+
browserStore = store,
326+
)
330327

331328
if (requireContext().settings().isExperimentationEnabled) {
332329
messagingFeatureHomescreen.set(

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddleware.kt‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Normal
5858
import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Private
5959
import org.mozilla.fenix.components.AppStore
6060
import org.mozilla.fenix.components.UseCases
61+
import org.mozilla.fenix.components.appstate.OrientationMode
6162
import org.mozilla.fenix.components.menu.MenuAccessPoint
6263
import org.mozilla.fenix.ext.nav
6364
import org.mozilla.fenix.ext.settings
@@ -74,7 +75,6 @@ import org.mozilla.fenix.search.ext.searchEngineShortcuts
7475
import org.mozilla.fenix.tabstray.DefaultTabManagementFeatureHelper
7576
import org.mozilla.fenix.tabstray.Page
7677
import org.mozilla.fenix.tabstray.TabManagementFeatureHelper
77-
import org.mozilla.fenix.utils.Settings
7878
import mozilla.components.lib.state.Action as MVIAction
7979
import mozilla.components.ui.icons.R as iconsR
8080

@@ -103,15 +103,13 @@ internal sealed class PageOriginInteractions : BrowserToolbarEvent {
103103
* @param browserStore [BrowserStore] to sync from.
104104
* @param clipboard [ClipboardHandler] to use for reading from device's clipboard.
105105
* @param useCases [UseCases] helping this integrate with other features of the applications.
106-
* @param settings [Settings] for accessing user preferences.
107106
* @param tabManagementFeatureHelper Feature flag helper for the tab management UI.
108107
*/
109108
class BrowserToolbarMiddleware(
110109
private val appStore: AppStore,
111110
private val browserStore: BrowserStore,
112111
private val clipboard: ClipboardHandler,
113112
private val useCases: UseCases,
114-
private val settings: Settings,
115113
private val tabManagementFeatureHelper: TabManagementFeatureHelper = DefaultTabManagementFeatureHelper,
116114
) : Middleware<BrowserToolbarState, BrowserToolbarAction> {
117115
@VisibleForTesting
@@ -320,15 +318,14 @@ class BrowserToolbarMiddleware(
320318

321319
private fun buildEndBrowserActions(): List<Action> {
322320
val environment = environment ?: return emptyList()
321+
val isSimpleOrLandScape = environment.context.settings().shouldUseSimpleToolbar ||
322+
appStore.state.orientation == OrientationMode.Landscape
323323

324324
return listOf(
325325
HomeToolbarActionConfig(HomeToolbarAction.TabCounter) {
326-
!settings.isTabStripEnabled &&
327-
environment.context.settings().shouldUseSimpleToolbar
328-
},
329-
HomeToolbarActionConfig(HomeToolbarAction.Menu) {
330-
environment.context.settings().shouldUseSimpleToolbar
326+
!environment.context.settings().isTabStripEnabled && isSimpleOrLandScape
331327
},
328+
HomeToolbarActionConfig(HomeToolbarAction.Menu) { isSimpleOrLandScape },
332329
).filter { config ->
333330
config.isVisible()
334331
}.map { config ->
@@ -344,18 +341,23 @@ class BrowserToolbarMiddleware(
344341
)
345342
}
346343

347-
private fun buildNavigationActions(): List<Action> =
348-
listOf(
349-
HomeToolbarActionConfig(HomeToolbarAction.FakeBookmark),
350-
HomeToolbarActionConfig(HomeToolbarAction.FakeShare),
351-
HomeToolbarActionConfig(HomeToolbarAction.NewTab),
352-
HomeToolbarActionConfig(HomeToolbarAction.TabCounter),
353-
HomeToolbarActionConfig(HomeToolbarAction.Menu),
344+
private fun buildNavigationActions(): List<Action> {
345+
val environment = environment ?: return emptyList()
346+
val isExpandedAndPortrait = !environment.context.settings().shouldUseSimpleToolbar &&
347+
appStore.state.orientation == OrientationMode.Portrait
348+
349+
return listOf(
350+
HomeToolbarActionConfig(HomeToolbarAction.FakeBookmark) { isExpandedAndPortrait },
351+
HomeToolbarActionConfig(HomeToolbarAction.FakeShare) { isExpandedAndPortrait },
352+
HomeToolbarActionConfig(HomeToolbarAction.NewTab) { isExpandedAndPortrait },
353+
HomeToolbarActionConfig(HomeToolbarAction.TabCounter) { isExpandedAndPortrait },
354+
HomeToolbarActionConfig(HomeToolbarAction.Menu) { isExpandedAndPortrait },
354355
).filter { config ->
355356
config.isVisible()
356357
}.map { config ->
357358
buildHomeAction(config.action)
358359
}
360+
}
359361

360362
private fun buildTabCounterMenu(): CombinedEventAndMenu? {
361363
val environment = environment ?: return null

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/HomeNavigationBar.kt‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,12 @@ package org.mozilla.fenix.home.toolbar
77
import android.app.ActionBar.LayoutParams
88
import android.content.Context
99
import android.view.ViewGroup
10-
import androidx.compose.foundation.layout.Spacer
11-
import androidx.compose.foundation.layout.height
1210
import androidx.compose.runtime.Composable
1311
import androidx.compose.runtime.SideEffect
1412
import androidx.compose.runtime.getValue
1513
import androidx.compose.runtime.mutableStateOf
1614
import androidx.compose.runtime.remember
17-
import androidx.compose.ui.Modifier
1815
import androidx.compose.ui.platform.ComposeView
19-
import androidx.compose.ui.unit.dp
2016
import androidx.fragment.app.Fragment
2117
import mozilla.components.browser.state.state.BrowserState
2218
import mozilla.components.browser.state.store.BrowserStore
@@ -28,7 +24,6 @@ import org.mozilla.fenix.components.AppStore
2824
import org.mozilla.fenix.components.StoreProvider
2925
import org.mozilla.fenix.ext.components
3026
import org.mozilla.fenix.theme.FirefoxTheme
31-
import org.mozilla.fenix.utils.Settings
3227

3328
/**
3429
* A wrapper over the [NavigationBar] composable that provides enhanced customization and
@@ -39,15 +34,13 @@ import org.mozilla.fenix.utils.Settings
3934
* @param container [ViewGroup] which will serve as parent of this View.
4035
* @param appStore [AppStore] to sync from.
4136
* @param browserStore [BrowserStore] used for observing the browsing details.
42-
* @param settings [Settings] object to get the toolbar position and other settings.
4337
*/
4438
class HomeNavigationBar(
4539
private val context: Context,
4640
private val lifecycleOwner: Fragment,
4741
private val container: ViewGroup,
4842
private val appStore: AppStore,
4943
private val browserStore: BrowserStore,
50-
private val settings: Settings,
5144
) : FenixHomeToolbar {
5245
val store = StoreProvider.get(lifecycleOwner) {
5346
BrowserToolbarStore(
@@ -58,37 +51,29 @@ class HomeNavigationBar(
5851
browserStore = browserStore,
5952
clipboard = context.components.clipboardHandler,
6053
useCases = context.components.useCases,
61-
settings = settings,
6254
),
6355
),
6456
)
6557
}
6658

6759
@Composable
6860
private fun DefaultNavigationBarContent(showDivider: Boolean) {
69-
if (settings.shouldUseSimpleToolbar) {
70-
return
71-
}
72-
7361
val uiState by store.observeAsState(initialValue = store.state) { it }
7462

75-
FirefoxTheme {
76-
NavigationBar(
77-
actions = uiState.displayState.navigationActions,
78-
shouldShowDivider = showDivider,
79-
onInteraction = { store.dispatch(it) },
80-
)
63+
if (uiState.displayState.navigationActions.isNotEmpty()) {
64+
FirefoxTheme {
65+
NavigationBar(
66+
actions = uiState.displayState.navigationActions,
67+
shouldShowDivider = showDivider,
68+
onInteraction = { store.dispatch(it) },
69+
)
70+
}
8171
}
8272
}
8373

8474
override val layout = ComposeView(context).apply {
8575
setContent {
86-
if (settings.shouldUseSimpleToolbar) {
87-
// empty spacer to be added
88-
Spacer(modifier = Modifier.height(0.dp))
89-
} else {
90-
DefaultNavigationBarContent(showDivider = true)
91-
}
76+
DefaultNavigationBarContent(showDivider = true)
9277
}
9378
}.apply {
9479
container.addView(

‎mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/HomeToolbarComposable.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,6 @@ internal class HomeToolbarComposable(
207207
browserStore = browserStore,
208208
clipboard = context.components.clipboardHandler,
209209
useCases = context.components.useCases,
210-
settings = settings,
211210
),
212211
BrowserToolbarSearchMiddleware(
213212
appStore = appStore,

0 commit comments

Comments
 (0)