Skip to content

Commit a711a07

Browse files
committed
Bug 1971553 - Remove leftover toolbar redesign layout code. r=android-reviewers,petru
Differential Revision: https://phabricator.services.mozilla.com/D253358
1 parent 82b31de commit a711a07

File tree

4 files changed

+9
-104
lines changed

4 files changed

+9
-104
lines changed

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

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
337337
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
338338
internal fun addLeadingAction(
339339
context: Context,
340-
showHomeButton: Boolean,
341340
showEraseButton: Boolean,
342341
) {
343342
if (leadingAction != null) return
@@ -352,7 +351,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
352351
iconTintColorResource = ThemeManager.resolveAttribute(R.attr.textPrimary, context),
353352
listener = browserToolbarInteractor::onEraseButtonClicked,
354353
)
355-
} else if (showHomeButton) {
354+
} else {
356355
BrowserToolbar.Button(
357356
imageDrawable = AppCompatResources.getDrawable(
358357
context,
@@ -362,23 +361,13 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
362361
iconTintColorResource = ThemeManager.resolveAttribute(R.attr.textPrimary, context),
363362
listener = browserToolbarInteractor::onHomeButtonClicked,
364363
)
365-
} else {
366-
null
367364
}
368365

369366
leadingAction?.let {
370367
(_browserToolbarView as? BrowserToolbarView)?.toolbar?.addNavigationAction(it)
371368
}
372369
}
373370

374-
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
375-
internal fun removeLeadingAction() {
376-
leadingAction?.let {
377-
(_browserToolbarView as? BrowserToolbarView)?.toolbar?.removeNavigationAction(it)
378-
}
379-
leadingAction = null
380-
}
381-
382371
/**
383372
* This code takes care of the [BrowserToolbar] leading and navigation actions.
384373
* The older design requires a HomeButton followed by navigation buttons for tablets.
@@ -394,7 +383,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
394383
feltPrivateBrowsingEnabled: Boolean,
395384
) {
396385
updateAddressBarLeadingAction(
397-
redesignEnabled = false,
398386
isLandscape = isLandscape,
399387
isPrivate = isPrivate,
400388
isTablet = isTablet,
@@ -407,37 +395,18 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
407395

408396
@VisibleForTesting
409397
internal fun updateAddressBarLeadingAction(
410-
redesignEnabled: Boolean,
411398
isLandscape: Boolean,
412399
isTablet: Boolean,
413400
isPrivate: Boolean,
414401
feltPrivateBrowsingEnabled: Boolean,
415402
context: Context,
416403
) {
417-
val showHomeButton = !redesignEnabled
418404
val showEraseButton = feltPrivateBrowsingEnabled && isPrivate && (isLandscape || isTablet)
419405

420-
if (showHomeButton || showEraseButton) {
421-
addLeadingAction(
422-
context = context,
423-
showHomeButton = showHomeButton,
424-
showEraseButton = showEraseButton,
425-
)
426-
} else {
427-
removeLeadingAction()
428-
}
429-
}
430-
431-
@VisibleForTesting
432-
internal fun updateAddressBarNavigationActions(
433-
context: Context,
434-
isWindowSizeSmall: Boolean,
435-
) {
436-
if (!isWindowSizeSmall) {
437-
addNavigationActions(context)
438-
} else {
439-
removeNavigationActions()
440-
}
406+
addLeadingAction(
407+
context = context,
408+
showEraseButton = showEraseButton,
409+
)
441410
}
442411

443412
override fun onUpdateToolbarForConfigurationChange(toolbar: FenixBrowserToolbarView) {

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabsIntegration.kt

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -147,19 +147,6 @@ class CustomTabsIntegration(
147147

148148
override fun onBackPressed() = feature.onBackPressed()
149149

150-
@VisibleForTesting
151-
internal fun updateAddressBarNavigationActions(
152-
context: Context,
153-
isWindowSizeSmall: Boolean,
154-
) {
155-
if (!isWindowSizeSmall) {
156-
addNavigationActions(context)
157-
browserToolbar.invalidateActions()
158-
} else {
159-
removeNavigationActions()
160-
}
161-
}
162-
163150
@VisibleForTesting
164151
internal fun addNavigationActions(context: Context) {
165152
val enableTint = feature.iconColor

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ class BrowserFragmentTest {
500500
feltPrivateBrowsingEnabled = false,
501501
)
502502

503-
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any(), any()) }
503+
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any()) }
504504
verify(exactly = 0) { browserFragment.addTabletActions(any()) }
505505
verify(exactly = 0) { browserFragment.addNavigationActions(any()) }
506506

@@ -520,7 +520,7 @@ class BrowserFragmentTest {
520520
feltPrivateBrowsingEnabled = false,
521521
)
522522

523-
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any(), any()) }
523+
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any()) }
524524
verify(exactly = 0) { browserFragment.addTabletActions(any()) }
525525
verify(exactly = 0) { browserFragment.addNavigationActions(any()) }
526526

@@ -542,7 +542,7 @@ class BrowserFragmentTest {
542542
)
543543

544544
verifyOrder {
545-
browserFragment.addLeadingAction(any(), any(), any())
545+
browserFragment.addLeadingAction(any(), any())
546546
browserFragment.addNavigationActions(any())
547547
}
548548

@@ -564,7 +564,6 @@ class BrowserFragmentTest {
564564
feltPrivateBrowsingEnabled = false,
565565
)
566566

567-
verify(exactly = 0) { browserFragment.removeLeadingAction() }
568567
verify(exactly = 0) { browserFragment.removeNavigationActions() }
569568

570569
unmockThemeManagerAndAppCompatResources()
@@ -585,7 +584,7 @@ class BrowserFragmentTest {
585584
feltPrivateBrowsingEnabled = false,
586585
)
587586

588-
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any(), any()) }
587+
verify(exactly = 1) { browserFragment.addLeadingAction(any(), any()) }
589588
verify(exactly = 1) { browserFragment.addNavigationActions(any()) }
590589

591590
unmockThemeManagerAndAppCompatResources()

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabsIntegrationTest.kt

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -164,56 +164,6 @@ class CustomTabsIntegrationTest {
164164
verify(exactly = 2) { toolbar.removeNavigationAction(any()) }
165165
}
166166

167-
@Test
168-
fun `GIVEN isLandscape is true WHEN updateAddressBarNavigationActions is called THEN navigation buttons are added`() {
169-
val toolbar: BrowserToolbar = mockk(relaxed = true)
170-
val integration = spyk(getIntegration(toolbar))
171-
172-
assertNull(integration.forwardAction)
173-
assertNull(integration.backAction)
174-
175-
integration.updateAddressBarNavigationActions(testContext, isWindowSizeSmall = false)
176-
177-
assertNotNull(integration.forwardAction)
178-
assertNotNull(integration.backAction)
179-
verify { integration.addNavigationActions(any()) }
180-
verify(exactly = 2) { toolbar.addNavigationAction(any()) }
181-
}
182-
183-
@Test
184-
fun `GIVEN isTablet is true WHEN updateAddressBarNavigationActions is called THEN navigation buttons are added`() {
185-
val toolbar: BrowserToolbar = mockk(relaxed = true)
186-
val integration = spyk(getIntegration(toolbar))
187-
188-
assertNull(integration.forwardAction)
189-
assertNull(integration.backAction)
190-
191-
integration.updateAddressBarNavigationActions(testContext, isWindowSizeSmall = false)
192-
193-
assertNotNull(integration.forwardAction)
194-
assertNotNull(integration.backAction)
195-
verify { integration.addNavigationActions(any()) }
196-
verify(exactly = 2) { toolbar.addNavigationAction(any()) }
197-
}
198-
199-
@Test
200-
fun `GIVEN isTablet and isLandscape are false WHEN updateAddressBarNavigationActions is called THEN navigation buttons are removed`() {
201-
val integration = spyk(getIntegration()).apply {
202-
forwardAction = mockk()
203-
backAction = mockk()
204-
}
205-
206-
assertNotNull(integration.forwardAction)
207-
assertNotNull(integration.backAction)
208-
209-
integration.updateAddressBarNavigationActions(testContext, isWindowSizeSmall = true)
210-
211-
assertNull(integration.forwardAction)
212-
assertNull(integration.backAction)
213-
verify { integration.removeNavigationActions() }
214-
verify(exactly = 2) { toolbar.removeNavigationAction(any()) }
215-
}
216-
217167
private fun getIntegration(
218168
toolbar: BrowserToolbar = this.toolbar,
219169
): CustomTabsIntegration {

0 commit comments

Comments
 (0)