From dc26272381dfa98cb7c67e43f2c92a3579664c8b Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 20 May 2021 17:18:52 +0300 Subject: [PATCH] For #19475 - Cleanup - respect naming scheme in TabsTrayController In our current MVI implementation the View Interactors are first called in response to a direct user action and contain methods following the "onXXHappened" naming scheme and then delegate other Interactors / Controllers for specific actions. Controllers contain the business logic for actually updating the app's state and offer methods following the "handleXXAction" naming scheme. --- .../org/mozilla/fenix/tabstray/TabLayoutMediator.kt | 2 +- .../org/mozilla/fenix/tabstray/TabsTrayController.kt | 9 +++------ .../org/mozilla/fenix/tabstray/TabsTrayFragment.kt | 8 ++------ .../org/mozilla/fenix/tabstray/TabsTrayInteractor.kt | 4 ++-- .../fenix/tabstray/browser/BrowserTrayInteractor.kt | 5 ++--- .../mozilla/fenix/tabstray/browser/BrowserTrayList.kt | 2 +- .../mozilla/fenix/tabstray/TabLayoutObserverTest.kt | 10 +++++----- 7 files changed, 16 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt index 8e27666f79..4bfeabe2bc 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabLayoutMediator.kt @@ -78,7 +78,7 @@ internal class TabLayoutObserver( true } - interactor.setCurrentTrayPosition(tab.position, animate) + interactor.onTrayPositionSelected(tab.position, animate) Do exhaustive when (Page.positionToPage(tab.position)) { Page.NormalTabs -> metrics.track(Event.TabsTrayNormalModeTapped) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt index 0999ad47e5..0a4d00fd61 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -18,23 +18,20 @@ import org.mozilla.fenix.tabtray.TabTrayDialogFragmentDirections interface TabsTrayController { /** - * Called when user clicks the new tab button. + * Called to open a new tab. */ - fun onNewTabTapped(isPrivate: Boolean) + fun handleOpeningNewTab(isPrivate: Boolean) } class DefaultTabsTrayController( - private val store: TabsTrayStore, private val browsingModeManager: BrowsingModeManager, private val navController: NavController, private val profiler: Profiler?, private val navigationInteractor: NavigationInteractor, private val metrics: MetricController, - private val ioScope: CoroutineScope, - private val accountManager: FxaAccountManager ) : TabsTrayController { - override fun onNewTabTapped(isPrivate: Boolean) { + override fun handleOpeningNewTab(isPrivate: Boolean) { val startTime = profiler?.getProfilerTime() browsingModeManager.mode = BrowsingMode.fromBoolean(isPrivate) navController.navigateBlockingForAsyncNavGraph( diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt index 8fba5dda25..90756e972e 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -29,7 +29,6 @@ import kotlinx.android.synthetic.main.tabstray_multiselect_items.* import kotlinx.android.synthetic.main.tabstray_multiselect_items.view.* import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.plus import mozilla.components.browser.state.selector.findTab import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.selector.normalTabs @@ -124,14 +123,11 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { ) tabsTrayController = DefaultTabsTrayController( - store = tabsTrayStore, browsingModeManager = activity.browsingModeManager, navController = findNavController(), navigationInteractor = navigationInteractor, profiler = requireComponents.core.engine.profiler, - accountManager = requireComponents.backgroundServices.accountManager, metrics = requireComponents.analytics.metrics, - ioScope = lifecycleScope + Dispatchers.IO ) browserTrayInteractor = DefaultBrowserTrayInteractor( @@ -262,13 +258,13 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { ) } - override fun setCurrentTrayPosition(position: Int, smoothScroll: Boolean) { + override fun onTrayPositionSelected(position: Int, smoothScroll: Boolean) { tabsTray.setCurrentItem(position, smoothScroll) tab_layout.getTabAt(position)?.select() tabsTrayStore.dispatch(TabsTrayAction.PageSelected(Page.positionToPage(position))) } - override fun navigateToBrowser() { + override fun onBrowserTabSelected() { dismissTabsTray() val navController = findNavController() diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt index 5c3a7371a5..b898e67659 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInteractor.kt @@ -13,12 +13,12 @@ interface TabsTrayInteractor { * @param position The position on the tray to focus. * @param smoothScroll If true, animate the scrolling from the current tab to [position]. */ - fun setCurrentTrayPosition(position: Int, smoothScroll: Boolean) + fun onTrayPositionSelected(position: Int, smoothScroll: Boolean) /** * Dismisses the tabs tray and navigates to the browser. */ - fun navigateToBrowser() + fun onBrowserTabSelected() /** * Invoked when a tab is removed from the tabs tray with the given [tabId]. diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt index 32154234a6..ec03088f2b 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayInteractor.kt @@ -57,7 +57,7 @@ class DefaultBrowserTrayInteractor( private val selectTabWrapper by lazy { SelectTabUseCaseWrapper(metrics, selectTab) { - trayInteractor.navigateToBrowser() + trayInteractor.onBrowserTabSelected() } } @@ -73,7 +73,6 @@ class DefaultBrowserTrayInteractor( */ override fun open(item: Tab) { selectTabWrapper.invoke(item.id) - trayInteractor.navigateToBrowser() } /** @@ -133,6 +132,6 @@ class DefaultBrowserTrayInteractor( * See [BrowserTrayInteractor.onFabClicked] */ override fun onFabClicked(isPrivate: Boolean) { - controller.onNewTabTapped(isPrivate) + controller.handleOpeningNewTab(isPrivate) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt index 0e30f0bc3a..4d561ed76c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/BrowserTrayList.kt @@ -36,7 +36,7 @@ class BrowserTrayList @JvmOverloads constructor( context.components.analytics.metrics, context.components.useCases.tabsUseCases.selectTab ) { - interactor.navigateToBrowser() + interactor.onBrowserTabSelected() } val removeTabUseCase = RemoveTabUseCaseWrapper( diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt index e9c2f2922b..115c8e2f31 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabLayoutObserverTest.kt @@ -37,7 +37,7 @@ class TabLayoutObserverTest { store.waitUntilIdle() - verify { interactor.setCurrentTrayPosition(1, false) } + verify { interactor.onTrayPositionSelected(1, false) } verify { metrics.track(Event.TabsTrayPrivateModeTapped) } every { tab.position } returns 0 @@ -46,7 +46,7 @@ class TabLayoutObserverTest { store.waitUntilIdle() - verify { interactor.setCurrentTrayPosition(0, true) } + verify { interactor.onTrayPositionSelected(0, true) } verify { metrics.track(Event.TabsTrayNormalModeTapped) } every { tab.position } returns 2 @@ -55,7 +55,7 @@ class TabLayoutObserverTest { store.waitUntilIdle() - verify { interactor.setCurrentTrayPosition(2, true) } + verify { interactor.onTrayPositionSelected(2, true) } verify { metrics.track(Event.TabsTraySyncedModeTapped) } } @@ -67,12 +67,12 @@ class TabLayoutObserverTest { observer.onTabSelected(tab) - verify { interactor.setCurrentTrayPosition(1, false) } + verify { interactor.onTrayPositionSelected(1, false) } verify { metrics.track(Event.TabsTrayPrivateModeTapped) } observer.onTabSelected(tab) - verify { interactor.setCurrentTrayPosition(1, true) } + verify { interactor.onTrayPositionSelected(1, true) } verify { metrics.track(Event.TabsTrayPrivateModeTapped) } } }