From b3a3c94169f2058c83371f5b8e28030005d9a6af Mon Sep 17 00:00:00 2001 From: Jeff Boek Date: Wed, 15 May 2019 17:06:49 -0700 Subject: [PATCH] Small refactor before we add onboarding cards (#2541) * For #2390 - Cleans up the toAdapterList method before we add onboarding * For #2514 - Hide tabs menu when no tabs are open --- .../sessioncontrol/SessionControlAdapter.kt | 21 +++--- .../sessioncontrol/SessionControlComponent.kt | 16 +++-- .../sessioncontrol/SessionControlUIView.kt | 70 +++++++++---------- .../viewholders/TabHeaderViewHolder.kt | 41 +++++------ .../viewholders/TabInCollectionViewHolder.kt | 1 - app/src/main/res/layout/tab_header.xml | 45 ++++++------ 6 files changed, 100 insertions(+), 94 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt index 9276245df..bb841e71f 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt @@ -22,7 +22,7 @@ import org.mozilla.fenix.home.sessioncontrol.viewholders.TabInCollectionViewHold import java.lang.IllegalStateException sealed class AdapterItem { - object TabHeader : AdapterItem() + data class TabHeader(val isPrivate: Boolean, val hasTabs: Boolean) : AdapterItem() object NoTabMessage : AdapterItem() data class TabItem(val tab: Tab) : AdapterItem() object PrivateBrowsingDescription : AdapterItem() @@ -35,7 +35,7 @@ sealed class AdapterItem { val viewType: Int get() = when (this) { - TabHeader -> TabHeaderViewHolder.LAYOUT_ID + is TabHeader -> TabHeaderViewHolder.LAYOUT_ID NoTabMessage -> NoTabMessageViewHolder.LAYOUT_ID is TabItem -> TabViewHolder.LAYOUT_ID SaveTabGroup -> SaveTabGroupViewHolder.LAYOUT_ID @@ -52,7 +52,7 @@ class SessionControlAdapter( private val actionEmitter: Observer ) : RecyclerView.Adapter() { - var items: List = listOf() + private var items: List = listOf() private lateinit var job: Job fun reloadData(items: List) { @@ -69,15 +69,10 @@ class SessionControlAdapter( NoTabMessageViewHolder.LAYOUT_ID -> NoTabMessageViewHolder(view) TabViewHolder.LAYOUT_ID -> TabViewHolder(view, actionEmitter, job) SaveTabGroupViewHolder.LAYOUT_ID -> SaveTabGroupViewHolder(view, actionEmitter) - PrivateBrowsingDescriptionViewHolder.LAYOUT_ID -> PrivateBrowsingDescriptionViewHolder( - view, - actionEmitter - ) + PrivateBrowsingDescriptionViewHolder.LAYOUT_ID -> PrivateBrowsingDescriptionViewHolder(view, actionEmitter) DeleteTabsViewHolder.LAYOUT_ID -> DeleteTabsViewHolder(view, actionEmitter) CollectionHeaderViewHolder.LAYOUT_ID -> CollectionHeaderViewHolder(view) - NoCollectionMessageViewHolder.LAYOUT_ID -> NoCollectionMessageViewHolder( - view - ) + NoCollectionMessageViewHolder.LAYOUT_ID -> NoCollectionMessageViewHolder(view) CollectionViewHolder.LAYOUT_ID -> CollectionViewHolder(view, actionEmitter, job) TabInCollectionViewHolder.LAYOUT_ID -> TabInCollectionViewHolder(view, actionEmitter, job) else -> throw IllegalStateException() @@ -100,6 +95,10 @@ class SessionControlAdapter( override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) { when (holder) { + is TabHeaderViewHolder -> { + val tabHeader = items[position] as AdapterItem.TabHeader + holder.bind(tabHeader.isPrivate, tabHeader.hasTabs) + } is TabViewHolder -> holder.bindSession( (items[position] as AdapterItem.TabItem).tab ) @@ -107,7 +106,7 @@ class SessionControlAdapter( (items[position] as AdapterItem.CollectionItem).collection ) is TabInCollectionViewHolder -> { - val item = (items[position] as AdapterItem.TabInCollectionItem) + val item = items[position] as AdapterItem.TabInCollectionItem holder.bindSession(item.collection, item.tab, item.isLastTab) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt index bc90274e1..df768de92 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlComponent.kt @@ -58,9 +58,16 @@ data class TabCollection( var expanded: Boolean = false ) : Parcelable +sealed class FirefoxAcocuntsState { + object SignedOut : FirefoxAcocuntsState() + data class AutoSignedIn(val email: String) : FirefoxAcocuntsState() + data class SignedIn(val email: String) : FirefoxAcocuntsState() +} + sealed class Mode { object Normal : Mode() object Private : Mode() + data class Onboarding(val firefoxAccountsState: FirefoxAcocuntsState) : Mode() } data class SessionControlState( @@ -110,12 +117,9 @@ sealed class SessionControlChange : Change { data class CollectionsChange(val collections: List) : SessionControlChange() } -class SessionControlViewModel(initialState: SessionControlState) : - UIComponentViewModelBase( - initialState, - reducer - ) { - +class SessionControlViewModel( + initialState: SessionControlState +) : UIComponentViewModelBase(initialState, reducer) { companion object { val reducer: (SessionControlState, SessionControlChange) -> SessionControlState = { state, change -> when (change) { diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt index 67583a332..f6700d1cf 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlUIView.kt @@ -16,60 +16,60 @@ import org.mozilla.fenix.mvi.UIView import androidx.recyclerview.widget.ItemTouchHelper import org.mozilla.fenix.BuildConfig -// Convert HomeState into a data structure HomeAdapter understands -@SuppressWarnings("ComplexMethod", "NestedBlockDepth") -private fun SessionControlState.toAdapterList(): List { +private fun normalModeAdapterItems(tabs: List, collections: List): List { val items = mutableListOf() - items.add(AdapterItem.TabHeader) + items.add(AdapterItem.TabHeader(false, tabs.isNotEmpty())) - // Populate tabs if (tabs.isNotEmpty()) { - tabs.reversed().map(AdapterItem::TabItem).forEach { items.add(it) } - if (mode == Mode.Private) { - items.add(AdapterItem.DeleteTabs) - } else if (BuildConfig.COLLECTIONS_ENABLED) { + items.addAll(tabs.reversed().map(AdapterItem::TabItem)) + if (BuildConfig.COLLECTIONS_ENABLED) { items.add(AdapterItem.SaveTabGroup) } } else { - val item = if (mode == Mode.Private) AdapterItem.PrivateBrowsingDescription - else AdapterItem.NoTabMessage - - items.add(item) + items.add(AdapterItem.NoTabMessage) } - // Populate collections - if (mode == Mode.Normal) { - items.add(AdapterItem.CollectionHeader) - if (collections.isNotEmpty()) { + items.add(AdapterItem.CollectionHeader) + if (collections.isNotEmpty()) { - // If the collection is expanded, we want to add all of its tabs beneath it in the adapter - collections.reversed().map(AdapterItem::CollectionItem).forEach { - if (it.collection.expanded) { - items.add(it) - addCollectionTabItems(it.collection, it.collection.tabs, items) - } else { - items.add(it) - } + // If the collection is expanded, we want to add all of its tabs beneath it in the adapter + collections.reversed().map(AdapterItem::CollectionItem).forEach { + items.add(it) + if (it.collection.expanded) { + items.addAll(collectionTabItems(it.collection)) } - } else { - items.add(AdapterItem.NoCollectionMessage) } + } else { + items.add(AdapterItem.NoCollectionMessage) } return items } -private fun addCollectionTabItems( - collection: TabCollection, - tabs: MutableList, - itemList: MutableList -) { - for (tabIndex in 0 until tabs.size) { - itemList.add(AdapterItem.TabInCollectionItem - (collection, collection.tabs[tabIndex], tabIndex == collection.tabs.size - 1)) +private fun privateModeAdapterItems(tabs: List): List { + val items = mutableListOf() + items.add(AdapterItem.TabHeader(true, tabs.isNotEmpty())) + + if (tabs.isNotEmpty()) { + items.addAll(tabs.reversed().map(AdapterItem::TabItem)) + items.add(AdapterItem.DeleteTabs) + } else { + items.add(AdapterItem.PrivateBrowsingDescription) } + + return items } +private fun SessionControlState.toAdapterList(): List = when (mode) { + is Mode.Normal -> normalModeAdapterItems(tabs, collections) + is Mode.Private -> privateModeAdapterItems(tabs) + is Mode.Onboarding -> listOf() +} + +private fun collectionTabItems(collection: TabCollection) = collection.tabs.mapIndexed { index, tab -> + AdapterItem.TabInCollectionItem(collection, tab, index == collection.tabs.lastIndex) + } + class SessionControlUIView( container: ViewGroup, actionEmitter: Observer, diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt index c3d8d22ab..543717339 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabHeaderViewHolder.kt @@ -19,14 +19,14 @@ import org.mozilla.fenix.home.sessioncontrol.TabAction import org.mozilla.fenix.home.sessioncontrol.onNext class TabHeaderViewHolder( - view: View, + private val view: View, private val actionEmitter: Observer ) : RecyclerView.ViewHolder(view) { private var isPrivate = false private var tabsMenu: TabHeaderMenu init { - tabsMenu = TabHeaderMenu(view.context) { + tabsMenu = TabHeaderMenu(view.context, isPrivate) { when (it) { is TabHeaderMenu.Item.Share -> actionEmitter.onNext(TabAction.ShareTabs) is TabHeaderMenu.Item.CloseAll -> actionEmitter.onNext(TabAction.CloseAll(isPrivate)) @@ -37,11 +37,8 @@ class TabHeaderViewHolder( ) } } - view.apply { - val headerTextResourceId = - if (isPrivate) R.string.tabs_header_private_title else R.string.tab_header_label - header_text.text = context.getString(headerTextResourceId) + view.apply { add_tab_button.run { setOnClickListener { actionEmitter.onNext(TabAction.Add) @@ -58,8 +55,19 @@ class TabHeaderViewHolder( } } + fun bind(isPrivate: Boolean, hasTabs: Boolean) { + this.isPrivate = isPrivate + tabsMenu.isPrivate = isPrivate + + val headerTextResourceId = + if (isPrivate) R.string.tabs_header_private_title else R.string.tab_header_label + view.header_text.text = view.context.getString(headerTextResourceId) + view.tabs_overflow_button.visibility = if (hasTabs) View.VISIBLE else View.GONE + } + class TabHeaderMenu( private val context: Context, + var isPrivate: Boolean, private val onItemTapped: (Item) -> Unit = {} ) { sealed class Item { @@ -81,20 +89,13 @@ class TabHeaderViewHolder( context.getString(R.string.tabs_menu_share_tabs) ) { onItemTapped.invoke(Item.Share) - } - ).let { - val list = it.toMutableList() - if (BuildConfig.COLLECTIONS_ENABLED) { - list.add( - SimpleBrowserMenuItem( - context.getString(R.string.tabs_menu_save_to_collection) - ) { - onItemTapped.invoke(Item.SaveToCollection) - } - ) - } - list - } + }, + SimpleBrowserMenuItem( + context.getString(R.string.tabs_menu_save_to_collection) + ) { + onItemTapped.invoke(Item.SaveToCollection) + }.apply { visible = { !isPrivate && BuildConfig.COLLECTIONS_ENABLED } } + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt index 8a9e56c6d..728686b60 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt @@ -47,7 +47,6 @@ class TabInCollectionViewHolder( var isLastTab = false init { - collection_tab_icon.clipToOutline = true collection_tab_icon.outlineProvider = object : ViewOutlineProvider() { override fun getOutline(view: View?, outline: Outline?) { diff --git a/app/src/main/res/layout/tab_header.xml b/app/src/main/res/layout/tab_header.xml index 79d91a67e..ec98bc93b 100644 --- a/app/src/main/res/layout/tab_header.xml +++ b/app/src/main/res/layout/tab_header.xml @@ -21,27 +21,30 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" /> - - - + app:layout_constraintTop_toTopOf="parent" + app:layout_constraintEnd_toEndOf="parent"> + + + + \ No newline at end of file