From d15b8381a6874061fed25fc5a07dcb23e54e1d85 Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Tue, 21 Jul 2020 03:13:09 -0400 Subject: [PATCH] For #10925: Fix breaking APIs in tabs tray --- .../java/org/mozilla/fenix/HomeActivity.kt | 16 ------ .../mozilla/fenix/tabtray/FenixTabsAdapter.kt | 2 +- .../fenix/tabtray/TabTrayDialogFragment.kt | 7 ++- .../org/mozilla/fenix/tabtray/TabTrayView.kt | 56 ++++++++++--------- .../fenix/tabtray/TabTrayViewHolder.kt | 8 ++- .../main/res/layout/component_tabstray.xml | 10 +--- .../fenix/tabtray/TabTrayViewHolderTest.kt | 6 +- 7 files changed, 50 insertions(+), 55 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 254898460..ed1a811c9 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -28,7 +28,6 @@ import androidx.navigation.NavDirections import androidx.navigation.fragment.NavHostFragment import androidx.navigation.ui.AppBarConfiguration import androidx.navigation.ui.NavigationUI -import androidx.recyclerview.widget.LinearLayoutManager import kotlinx.android.synthetic.main.activity_home.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -41,10 +40,7 @@ import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.WebExtensionState import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.browser.tabstray.BrowserTabsTray -import mozilla.components.browser.thumbnails.loader.ThumbnailLoader import mozilla.components.concept.engine.EngineView -import mozilla.components.concept.tabstray.TabsTray import mozilla.components.feature.contextmenu.DefaultSelectionActionDelegate import mozilla.components.feature.search.BrowserStoreSearchAdapter import mozilla.components.feature.search.SearchAdapter @@ -97,7 +93,6 @@ import org.mozilla.fenix.settings.search.AddSearchEngineFragmentDirections import org.mozilla.fenix.settings.search.EditCustomSearchEngineFragmentDirections import org.mozilla.fenix.share.AddNewDeviceFragmentDirections import org.mozilla.fenix.sync.SyncedTabsFragmentDirections -import org.mozilla.fenix.tabtray.FenixTabsAdapter import org.mozilla.fenix.tabtray.TabTrayDialogFragment import org.mozilla.fenix.theme.DefaultThemeManager import org.mozilla.fenix.theme.ThemeManager @@ -315,17 +310,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { actionSorter = ::actionSorter ) }.asView() - TabsTray::class.java.name -> { - val layout = LinearLayoutManager(context).apply { - reverseLayout = true - stackFromEnd = true - } - - val thumbnailLoader = ThumbnailLoader(components.core.thumbnailStorage) - val adapter = FenixTabsAdapter(context, thumbnailLoader) - - BrowserTabsTray(context, attrs, 0, adapter, layout) - } else -> super.onCreateView(parent, name, context, attrs) } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/FenixTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabtray/FenixTabsAdapter.kt index 1610ffd91..267dc4e70 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/FenixTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/FenixTabsAdapter.kt @@ -16,7 +16,7 @@ class FenixTabsAdapter( context: Context, imageLoader: ImageLoader ) : TabsAdapter( - viewHolderProvider = { parentView, _ -> + viewHolderProvider = { parentView -> TabTrayViewHolder( LayoutInflater.from(context).inflate( R.layout.tab_tray_item, diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt index c04818f50..e621d9a81 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt @@ -23,6 +23,7 @@ import kotlinx.android.synthetic.main.fragment_tab_tray_dialog.view.* import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.browser.session.Session import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.thumbnails.loader.ThumbnailLoader import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.feature.tabs.tabstray.TabsFeature @@ -127,8 +128,12 @@ class TabTrayDialogFragment : AppCompatDialogFragment() { super.onViewCreated(view, savedInstanceState) val isPrivate = (activity as HomeActivity).browsingModeManager.mode.isPrivate + val thumbnailLoader = ThumbnailLoader(requireContext().components.core.thumbnailStorage) + val adapter = FenixTabsAdapter(requireContext(), thumbnailLoader) + _tabTrayView = TabTrayView( view.tabLayout, + adapter, interactor = TabTrayFragmentInteractor( DefaultTabTrayController( activity = (activity as HomeActivity), @@ -152,7 +157,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment() { tabsFeature.set( TabsFeature( - tabTrayView.view.tabsTray, + adapter, view.context.components.core.store, selectTabUseCase, removeTabUseCase, diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index a5e87a297..c0c3acecb 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -13,6 +13,7 @@ import androidx.cardview.widget.CardView import androidx.core.content.ContextCompat import androidx.core.view.isVisible import androidx.lifecycle.LifecycleCoroutineScope +import androidx.recyclerview.widget.LinearLayoutManager import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.tabs.TabLayout import kotlinx.android.extensions.LayoutContainer @@ -29,7 +30,6 @@ import mozilla.components.browser.menu.item.SimpleBrowserMenuItem import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState -import mozilla.components.browser.tabstray.BrowserTabsTray import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components @@ -41,6 +41,7 @@ import org.mozilla.fenix.ext.settings @Suppress("LongParameterList", "TooManyFunctions", "LargeClass") class TabTrayView( private val container: ViewGroup, + private val tabsAdapter: FenixTabsAdapter, private val interactor: TabTrayInteractor, isPrivate: Boolean, startingInLandscape: Boolean, @@ -118,27 +119,32 @@ class TabTrayView( setTopOffset(startingInLandscape) - (view.tabsTray as? BrowserTabsTray)?.also { tray -> - TabsTouchHelper(tray.tabsAdapter).attachToRecyclerView(tray) - (tray.tabsAdapter as? FenixTabsAdapter)?.also { adapter -> - adapter.onTabsUpdated = { - if (hasAccessibilityEnabled) { - adapter.notifyDataSetChanged() - } - if (!hasLoaded) { - hasLoaded = true - scrollToTab(view.context.components.core.store.state.selectedTabId) - if (view.context.settings().accessibilityServicesEnabled) { - lifecycleScope.launch { - delay(SELECTION_DELAY.toLong()) - lifecycleScope.launch(Main) { - tray.layoutManager?.findViewByPosition(selectedBrowserTabIndex) - ?.requestFocus() - tray.layoutManager?.findViewByPosition(selectedBrowserTabIndex) - ?.sendAccessibilityEvent( - AccessibilityEvent.TYPE_VIEW_FOCUSED - ) - } + view.tabsTray.apply { + layoutManager = LinearLayoutManager(container.context).apply { + reverseLayout = true + stackFromEnd = true + } + adapter = tabsAdapter + + TabsTouchHelper(tabsAdapter).attachToRecyclerView(this) + + tabsAdapter.onTabsUpdated = { + if (hasAccessibilityEnabled) { + tabsAdapter.notifyDataSetChanged() + } + if (!hasLoaded) { + hasLoaded = true + scrollToTab(view.context.components.core.store.state.selectedTabId) + if (view.context.settings().accessibilityServicesEnabled) { + lifecycleScope.launch { + delay(SELECTION_DELAY.toLong()) + lifecycleScope.launch(Main) { + layoutManager?.findViewByPosition(selectedBrowserTabIndex) + ?.requestFocus() + layoutManager?.findViewByPosition(selectedBrowserTabIndex) + ?.sendAccessibilityEvent( + AccessibilityEvent.TYPE_VIEW_FOCUSED + ) } } } @@ -241,7 +247,7 @@ class TabTrayView( } } - view.tabsTray.asView().visibility = if (hasNoTabs) { + view.tabsTray.visibility = if (hasNoTabs) { View.INVISIBLE } else { View.VISIBLE @@ -288,7 +294,7 @@ class TabTrayView( } fun scrollToTab(sessionId: String?) { - (view.tabsTray as? BrowserTabsTray)?.also { tray -> + view.tabsTray.apply { val tabs = if (isPrivateModeSelected) { view.context.components.core.store.state.privateTabs } else { @@ -298,7 +304,7 @@ class TabTrayView( val selectedBrowserTabIndex = tabs .indexOfFirst { it.id == sessionId } - tray.layoutManager?.scrollToPosition(selectedBrowserTabIndex) + layoutManager?.scrollToPosition(selectedBrowserTabIndex) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt index d9fce4c4c..61989e696 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayViewHolder.kt @@ -16,6 +16,7 @@ import androidx.core.content.ContextCompat import mozilla.components.browser.state.state.MediaState import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.tabstray.TabViewHolder +import mozilla.components.browser.tabstray.TabsTrayStyling import mozilla.components.browser.tabstray.thumbnail.TabThumbnailView import mozilla.components.browser.toolbar.MAX_URI_LENGTH import mozilla.components.concept.tabstray.Tab @@ -64,7 +65,12 @@ class TabTrayViewHolder( /** * Displays the data of the given session and notifies the given observable about events. */ - override fun bind(tab: Tab, isSelected: Boolean, observable: Observable) { + override fun bind( + tab: Tab, + isSelected: Boolean, + styling: TabsTrayStyling, + observable: Observable + ) { // This is a hack to workaround a bug in a-c. // https://github.com/mozilla-mobile/android-components/issues/7186 val isSelected2 = tab.id == getSelectedTabId() diff --git a/app/src/main/res/layout/component_tabstray.xml b/app/src/main/res/layout/component_tabstray.xml index 51a1edc62..70e3a49ce 100644 --- a/app/src/main/res/layout/component_tabstray.xml +++ b/app/src/main/res/layout/component_tabstray.xml @@ -105,7 +105,7 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/tab_layout" /> - + app:layout_constraintTop_toBottomOf="@+id/divider" /> diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt index 57aa76948..480213250 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/TabTrayViewHolderTest.kt @@ -59,7 +59,7 @@ class TabTrayViewHolderTest { id = "123", url = extremelyLongUrl ) - tabViewHolder.bind(tab, false, mockk()) + tabViewHolder.bind(tab, false, mockk(), mockk()) assertEquals("m".repeat(MAX_URI_LENGTH), tabViewHolder.urlView?.text) verify { imageLoader.loadIntoView(any(), ImageLoadRequest("123", 92)) } @@ -82,7 +82,7 @@ class TabTrayViewHolderTest { ) ) ) - tabViewHolder.bind(tab, false, mockk()) + tabViewHolder.bind(tab, false, mockk(), mockk()) assertEquals("Play", playPauseButtonView.contentDescription) } @@ -104,7 +104,7 @@ class TabTrayViewHolderTest { ) ) ) - tabViewHolder.bind(tab, false, mockk()) + tabViewHolder.bind(tab, false, mockk(), mockk()) assertEquals("Pause", playPauseButtonView.contentDescription) }