diff --git a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecorator.kt b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecorator.kt index 56e3950d9eb0..b2e6ad625dcc 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecorator.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecorator.kt @@ -74,7 +74,7 @@ sealed class RecentTabViewDecorator { val context = itemView.context itemView.background = - AppCompatResources.getDrawable(context, R.drawable.home_list_row_background) + AppCompatResources.getDrawable(context, R.drawable.card_list_row_background) return itemView } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 6443ed71fd36..4336027e9a83 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -21,8 +21,6 @@ import androidx.lifecycle.lifecycleScope import androidx.navigation.NavDirections import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs -import kotlinx.android.synthetic.main.component_bookmark.view.* -import kotlinx.android.synthetic.main.fragment_bookmark.view.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main @@ -44,6 +42,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.databinding.FragmentBookmarkBinding import org.mozilla.fenix.ext.bookmarkStorage import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.minus @@ -72,13 +71,16 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null private var pendingBookmarksToDelete: MutableSet = mutableSetOf() + private var _binding: FragmentBookmarkBinding? = null + private val binding get() = _binding!! + private val metrics get() = context?.components?.analytics?.metrics override val selectedItems get() = bookmarkStore.state.mode.selectedItems - override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { - val view = inflater.inflate(R.layout.fragment_bookmark, container, false) + override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View { + _binding = FragmentBookmarkBinding.inflate(inflater, container, false) bookmarkStore = StoreProvider.get(this) { BookmarkFragmentStore(BookmarkFragmentState(null)) @@ -103,8 +105,8 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan metrics = metrics!! ) - bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor, findNavController()) - bookmarkView.view.bookmark_folders_sign_in.visibility = View.GONE + bookmarkView = BookmarkView(binding.bookmarkLayout, bookmarkInteractor, findNavController()) + bookmarkView.binding.bookmarkFoldersSignIn.visibility = View.GONE viewLifecycleOwner.lifecycle.addObserver( BookmarkDeselectNavigationListener( @@ -114,7 +116,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan ) ) - return view + return binding.root } private fun showSnackBarWithText(text: String) { @@ -138,7 +140,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan // Don't want to pester user too much with it, and if there are lots of bookmarks present, // it'll just get visually lost. Inside of the "Desktop Bookmarks" node, it'll nicely stand-out, // since there are always only three other items in there. It's also the right place contextually. - bookmarkView.view.bookmark_folders_sign_in.isVisible = + bookmarkView.binding.bookmarkFoldersSignIn.isVisible = it.tree?.guid == BookmarkRoot.Root.id && accountManager.authenticatedAccount() == null } } @@ -361,6 +363,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan override fun onDestroyView() { super.onDestroyView() _bookmarkInteractor = null + _binding = null } private fun showRemoveFolderDialog(selected: Set) { diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt index 6c581a8f8cd5..7c7925079cbd 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt @@ -5,16 +5,15 @@ package org.mozilla.fenix.library.bookmarks import android.view.LayoutInflater -import android.view.View import android.view.ViewGroup import androidx.core.view.isVisible import androidx.navigation.NavController -import kotlinx.android.synthetic.main.component_bookmark.view.* import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.storage.BookmarkNode import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R +import org.mozilla.fenix.databinding.ComponentBookmarkBinding import org.mozilla.fenix.library.LibraryPageView import org.mozilla.fenix.selection.SelectionInteractor @@ -106,22 +105,23 @@ class BookmarkView( private val navController: NavController ) : LibraryPageView(container), UserInteractionHandler { - val view: View = LayoutInflater.from(container.context) - .inflate(R.layout.component_bookmark, container, true) + val binding = ComponentBookmarkBinding.inflate( + LayoutInflater.from(container.context), container, true + ) private var mode: BookmarkFragmentState.Mode = BookmarkFragmentState.Mode.Normal() private var tree: BookmarkNode? = null - private val bookmarkAdapter = BookmarkAdapter(view.bookmarks_empty_view, interactor) + private val bookmarkAdapter = BookmarkAdapter(binding.bookmarksEmptyView, interactor) init { - view.bookmark_list.apply { + binding.bookmarkList.apply { adapter = bookmarkAdapter } - view.bookmark_folders_sign_in.setOnClickListener { + binding.bookmarkFoldersSignIn.setOnClickListener { navController.navigate(NavGraphDirections.actionGlobalTurnOnSync()) } - view.swipe_refresh.setOnRefreshListener { + binding.swipeRefresh.setOnRefreshListener { interactor.onRequestSync() } } @@ -150,10 +150,10 @@ class BookmarkView( ) } } - view.bookmarks_progress_bar.isVisible = state.isLoading - view.swipe_refresh.isEnabled = + binding.bookmarksProgressBar.isVisible = state.isLoading + binding.swipeRefresh.isEnabled = state.mode is BookmarkFragmentState.Mode.Normal || state.mode is BookmarkFragmentState.Mode.Syncing - view.swipe_refresh.isRefreshing = state.mode is BookmarkFragmentState.Mode.Syncing + binding.swipeRefresh.isRefreshing = state.mode is BookmarkFragmentState.Mode.Syncing } override fun onBackPressed(): Boolean { diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt index 83a44e619252..1323be2bbc0b 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolder.kt @@ -12,6 +12,7 @@ import mozilla.components.concept.tabstray.Tab import org.mozilla.fenix.R import org.mozilla.fenix.databinding.InactiveFooterItemBinding import org.mozilla.fenix.databinding.InactiveRecentlyClosedItemBinding +import org.mozilla.fenix.databinding.InactiveHeaderItemBinding import org.mozilla.fenix.databinding.InactiveTabListItemBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.loadIntoView @@ -22,9 +23,32 @@ import org.mozilla.fenix.tabstray.browser.AutoCloseInterval.OneMonth import org.mozilla.fenix.tabstray.browser.AutoCloseInterval.OneWeek sealed class InactiveTabViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) { - class HeaderHolder(itemView: View) : InactiveTabViewHolder(itemView) { + + class HeaderHolder( + itemView: View, + interactor: InactiveTabsInteractor + ) : InactiveTabViewHolder(itemView) { + + private val binding = InactiveHeaderItemBinding.bind(itemView) + + init { + itemView.apply { + isActivated = InactiveTabsState.isExpanded + + setOnClickListener { + val newState = !it.isActivated + + interactor.onHeaderClicked(newState) + + it.isActivated = newState + binding.chevron.rotation = ROTATION_DEGREE + } + } + } + companion object { const val LAYOUT_ID = R.layout.inactive_header_item + private const val ROTATION_DEGREE = 180F } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt index 60e0d015ecac..165f6e3332e3 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsAdapter.kt @@ -39,12 +39,14 @@ class InactiveTabsAdapter( delegate: Observable = ObserverRegistry() ) : Adapter(DiffCallback), TabsTray, Observable by delegate { + internal lateinit var inactiveTabsInteractor: InactiveTabsInteractor + override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): InactiveTabViewHolder { val view = LayoutInflater.from(parent.context) .inflate(viewType, parent, false) return when (viewType) { - HeaderHolder.LAYOUT_ID -> HeaderHolder(view) + HeaderHolder.LAYOUT_ID -> HeaderHolder(view, inactiveTabsInteractor) TabViewHolder.LAYOUT_ID -> TabViewHolder(view, browserTrayInteractor) FooterHolder.LAYOUT_ID -> FooterHolder(view) RecentlyClosedHolder.LAYOUT_ID -> RecentlyClosedHolder(view, browserTrayInteractor) @@ -81,12 +83,18 @@ class InactiveTabsAdapter( } override fun updateTabs(tabs: Tabs) { + // Early return with an empty list to remove the header/footer items. if (tabs.list.isEmpty()) { - // Early return with an empty list to remove the header/footer items. submitList(emptyList()) return } + // If we have items, but we should be in a collapsed state. + if (!InactiveTabsState.isExpanded) { + submitList(listOf(Item.Header)) + return + } + val items = tabs.list.map { Item.Tab(it) } val footer = Item.Footer(context.autoCloseInterval) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt new file mode 100644 index 000000000000..16f9e94b657e --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsController.kt @@ -0,0 +1,28 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray.browser + +import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.tabstray.TabsTray +import mozilla.components.feature.tabs.ext.toTabs + +class InactiveTabsController( + private val browserStore: BrowserStore, + private val tabFilter: (TabSessionState) -> Boolean, + private val tray: TabsTray +) { + /** + * Updates the inactive card to be expanded to display all the tabs, or collapsed with only + * the title showing. + */ + fun updateCardExpansion(isExpanded: Boolean) { + InactiveTabsState.isExpanded = isExpanded + + val tabs = browserStore.state.toTabs { tabFilter.invoke(it) } + + tray.updateTabs(tabs) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsInteractor.kt new file mode 100644 index 000000000000..08d01debb270 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsInteractor.kt @@ -0,0 +1,26 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray.browser + +interface InactiveTabsInteractor { + fun onHeaderClicked(activated: Boolean) +} + +class DefaultInactiveTabsInteractor( + private val controller: InactiveTabsController +) : InactiveTabsInteractor { + override fun onHeaderClicked(activated: Boolean) { + controller.updateCardExpansion(activated) + } +} + +/** + * An experimental state holder for [InactiveTabsAdapter] that lives at the application lifetime. + * + * TODO This should be replaced with the AppStore. + */ +object InactiveTabsState { + var isExpanded = true +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt index 21504c8a7bd1..9e12a46a424c 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.tabstray.browser import android.content.Context import android.util.AttributeSet import androidx.recyclerview.widget.ConcatAdapter +import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.FeatureFlags @@ -53,20 +54,30 @@ class NormalBrowserTrayList @JvmOverloads constructor( ) } + /** + * NB: The setup for this feature is a bit complicated without a better dependency injection + * solution to scope it down to just this view. + */ private val inactiveFeature by lazy { - val tabsAdapter = concatAdapter.inactiveTabsAdapter + val store = context.components.core.store + val tabFilter: (TabSessionState) -> Boolean = filter@{ + if (!FeatureFlags.inactiveTabs) { + return@filter false + } + it.isNormalTabInactive(maxActiveTime) + } + val tabsAdapter = concatAdapter.inactiveTabsAdapter.apply { + inactiveTabsInteractor = DefaultInactiveTabsInteractor( + InactiveTabsController(store, tabFilter, this) + ) + } TabsFeature( tabsAdapter, - context.components.core.store, + store, selectTabUseCase, removeTabUseCase, - { state -> - if (!FeatureFlags.inactiveTabs) { - return@TabsFeature false - } - state.isNormalTabInactive(maxActiveTime) - }, + tabFilter, {} ) } diff --git a/app/src/main/res/drawable/home_list_row_background.xml b/app/src/main/res/drawable/card_list_row_background.xml similarity index 100% rename from app/src/main/res/drawable/home_list_row_background.xml rename to app/src/main/res/drawable/card_list_row_background.xml diff --git a/app/src/main/res/layout/collection_home_list_row.xml b/app/src/main/res/layout/collection_home_list_row.xml index 63e5afbbd15c..22957cc81f39 100644 --- a/app/src/main/res/layout/collection_home_list_row.xml +++ b/app/src/main/res/layout/collection_home_list_row.xml @@ -8,7 +8,7 @@ android:layout_width="match_parent" android:layout_height="48dp" android:layout_marginTop="12dp" - android:background="@drawable/home_list_row_background" + android:background="@drawable/card_list_row_background" android:clickable="true" android:clipToPadding="false" android:elevation="@dimen/home_item_elevation" diff --git a/app/src/main/res/layout/inactive_header_item.xml b/app/src/main/res/layout/inactive_header_item.xml index 1bfee4139d05..2af0c5d37aad 100644 --- a/app/src/main/res/layout/inactive_header_item.xml +++ b/app/src/main/res/layout/inactive_header_item.xml @@ -9,7 +9,7 @@ android:layout_marginStart="16dp" android:layout_marginTop="12dp" android:layout_marginEnd="16dp" - android:background="@drawable/rounded_top_corners" + android:background="@drawable/card_list_row_background" android:clickable="false" android:clipToPadding="false" android:focusable="true" @@ -31,4 +31,16 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" tools:text="Inactive tabs" /> + + + \ No newline at end of file diff --git a/app/src/test/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecoratorTest.kt b/app/src/test/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecoratorTest.kt index d4f7251524f4..ee359f6aa0b1 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecoratorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recenttabs/view/RecentTabViewDecoratorTest.kt @@ -63,7 +63,7 @@ class RecentTabViewDecoratorTest { RecentTabViewDecorator.SingleTabDecoration(view) verify { view.background = drawable } - assertEquals(R.drawable.home_list_row_background, drawableResCaptor.captured) + assertEquals(R.drawable.card_list_row_background, drawableResCaptor.captured) } finally { unmockkStatic(AppCompatResources::class) } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/DefaultInactiveTabsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/DefaultInactiveTabsInteractorTest.kt new file mode 100644 index 000000000000..36c27520b867 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/DefaultInactiveTabsInteractorTest.kt @@ -0,0 +1,22 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray.browser + +import io.mockk.mockk +import io.mockk.verify +import org.junit.Test + +class DefaultInactiveTabsInteractorTest { + + @Test + fun `WHEN onHeaderClicked THEN updateCardExpansion`() { + val controller: InactiveTabsController = mockk(relaxed = true) + val interactor = DefaultInactiveTabsInteractor(controller) + + interactor.onHeaderClicked(true) + + verify { controller.updateCardExpansion(true) } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolderTest.kt new file mode 100644 index 000000000000..d33dc9626d14 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabViewHolderTest.kt @@ -0,0 +1,33 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray.browser + +import android.view.LayoutInflater +import io.mockk.mockk +import io.mockk.verify +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.tabstray.browser.InactiveTabViewHolder.HeaderHolder + +@RunWith(FenixRobolectricTestRunner::class) +class InactiveTabViewHolderTest { + @Test + fun `HeaderHolder - WHEN clicked THEN notify the interactor`() { + val view = LayoutInflater.from(testContext).inflate(HeaderHolder.LAYOUT_ID, null) + val interactor: InactiveTabsInteractor = mockk(relaxed = true) + val viewHolder = HeaderHolder(view, interactor) + + val initialActivatedState = view.isActivated + + viewHolder.itemView.performClick() + + verify { interactor.onHeaderClicked(any()) } + + assertEquals(!initialActivatedState, view.isActivated) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt new file mode 100644 index 000000000000..207f619fa40c --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.tabstray.browser + +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.tabstray.Tabs +import mozilla.components.concept.tabstray.TabsTray +import org.junit.Assert.assertEquals +import mozilla.components.browser.state.state.createTab as createTabState +import org.junit.Test + +class InactiveTabsControllerTest { + @Test + fun `WHEN expanded THEN notify filtered card`() { + val filter: (TabSessionState) -> Boolean = { !it.content.private } + val store = BrowserStore( + BrowserState( + tabs = listOf( + createTabState("https://mozilla.org", id = "1"), + createTabState("https://firefox.com", id = "2"), + createTabState("https://getpocket.com", id = "3", private = true) + ) + ) + ) + val tray: TabsTray = mockk(relaxed = true) + val tabsSlot = slot() + val controller = InactiveTabsController(store, filter, tray) + + controller.updateCardExpansion(true) + + verify { tray.updateTabs(capture(tabsSlot)) } + + assertEquals(2, tabsSlot.captured.list.size) + assertEquals("1", tabsSlot.captured.list.first().id) + } +}