From 0327b1146b42572c800c97daea0c1fddedf43b14 Mon Sep 17 00:00:00 2001 From: Emily Kager Date: Fri, 31 May 2019 10:59:51 -0700 Subject: [PATCH] For #2977 - Update add new collections flow (#2991) * For #2977 - Update add new collections flow * Rename shared elements to be more general * Make tab list not clickable in other modes * For #2577 - Stop Flickering in List * Add extensions function for next step with collections list size --- .../mozilla/fenix/browser/BrowserFragment.kt | 4 +- .../CollectionCreationTabListAdapter.kt | 61 +++++++++----- .../collections/CollectionCreationUIView.kt | 53 +++++++++--- .../collections/CreateCollectionFragment.kt | 35 +++++--- .../collections/CreateCollectionViewModel.kt | 3 + .../collections/SaveCollectionListAdapter.kt | 41 ++++++++++ .../res/layout/collection_home_list_row.xml | 15 ++-- .../res/layout/collection_tab_list_row.xml | 1 - .../main/res/layout/collections_list_item.xml | 81 +++++++++++++------ .../layout/component_collection_creation.xml | 40 ++------- ...nt_collection_creation_name_collection.xml | 36 ++------- ..._collection_creation_select_collection.xml | 65 +++++---------- 12 files changed, 248 insertions(+), 187 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 0e73966a7..e9ee1bf9d 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -62,6 +62,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ThemeManager import org.mozilla.fenix.collections.CreateCollectionViewModel import org.mozilla.fenix.collections.SaveCollectionStep +import org.mozilla.fenix.collections.getStepForCollectionsSize import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FindInPageIntegration import org.mozilla.fenix.components.metrics.Event @@ -698,8 +699,9 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope { viewModel?.tabs = listOf(tabs) val selectedSet = mutableSetOf(tabs) viewModel?.selectedTabs = selectedSet - viewModel?.saveCollectionStep = SaveCollectionStep.SelectCollection viewModel?.tabCollections = requireComponents.core.tabCollectionStorage.cachedTabCollections.reversed() + viewModel?.saveCollectionStep = + viewModel?.tabCollections?.getStepForCollectionsSize() ?: SaveCollectionStep.SelectCollection viewModel?.snackbarAnchorView = nestedScrollQuickAction view?.let { val directions = BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment() diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt index 51be1c9b5..effc4f8f9 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt @@ -7,7 +7,6 @@ package org.mozilla.fenix.collections import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.CompoundButton import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer @@ -25,9 +24,8 @@ import kotlin.coroutines.CoroutineContext class CollectionCreationTabListAdapter( val actionEmitter: Observer ) : RecyclerView.Adapter() { - private var tabs: List = listOf() - private var selectedTabs: Set = setOf() + private var selectedTabs: MutableSet = mutableSetOf() private lateinit var job: Job private var hideCheckboxes = false @@ -35,13 +33,40 @@ class CollectionCreationTabListAdapter( val view = LayoutInflater.from(parent.context).inflate(TabViewHolder.LAYOUT_ID, parent, false) - return TabViewHolder(view, actionEmitter, job) + return TabViewHolder(view, job) + } + + override fun onBindViewHolder(holder: TabViewHolder, position: Int, payloads: MutableList) { + if (payloads.isEmpty()) { + super.onBindViewHolder(holder, position, payloads) + } else { + when (payloads[0]) { + is CheckChanged -> { + val checkChanged = payloads[0] as CheckChanged + if (checkChanged.shouldBeChecked) { + holder.view.tab_selected_checkbox.isChecked = true + } else if (checkChanged.shouldBeUnchecked) { + holder.view.tab_selected_checkbox.isChecked = false + } + } + } + } } override fun onBindViewHolder(holder: TabViewHolder, position: Int) { val tab = tabs[position] val isSelected = selectedTabs.contains(tab) holder.bind(tab, isSelected, hideCheckboxes) + holder.view.tab_selected_checkbox.setOnCheckedChangeListener { _, isChecked -> + val action = if (isChecked) { + selectedTabs.add(tab) + CollectionCreationAction.AddTabToSelection(tab) + } else { + selectedTabs.remove(tab) + CollectionCreationAction.RemoveTabFromSelection(tab) + } + actionEmitter.onNext(action) + } } override fun getItemCount(): Int = tabs.size @@ -69,7 +94,7 @@ class CollectionCreationTabListAdapter( ) this.tabs = tabs - this.selectedTabs = selectedTabs + this.selectedTabs = selectedTabs.toMutableSet() this.hideCheckboxes = hideCheckboxes diffUtil.dispatchUpdatesTo(this) @@ -88,20 +113,27 @@ private class TabDiffUtil( old[oldItemPosition].sessionId == new[newItemPosition].sessionId override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { - val isSameTab = old[oldItemPosition].url == new[newItemPosition].url - val sameSelectedState = - oldSelected.contains(old[oldItemPosition]) == newSelected.contains(new[newItemPosition]) + val isSameTab = old[oldItemPosition].sessionId == new[newItemPosition].sessionId + val sameSelectedState = oldSelected.contains(old[oldItemPosition]) == newSelected.contains(new[newItemPosition]) val isSameHideCheckboxes = oldHideCheckboxes == newHideCheckboxes return isSameTab && sameSelectedState && isSameHideCheckboxes } + override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int): Any? { + val shouldBeChecked = newSelected.contains(new[newItemPosition]) && !oldSelected.contains(old[oldItemPosition]) + val shouldBeUnchecked = + !newSelected.contains(new[newItemPosition]) && oldSelected.contains(old[oldItemPosition]) + return CheckChanged(shouldBeChecked, shouldBeUnchecked) + } + override fun getOldListSize(): Int = old.size override fun getNewListSize(): Int = new.size } +data class CheckChanged(val shouldBeChecked: Boolean, val shouldBeUnchecked: Boolean) + class TabViewHolder( val view: View, - actionEmitter: Observer, val job: Job ) : RecyclerView.ViewHolder(view), CoroutineScope { @@ -111,14 +143,6 @@ class TabViewHolder( private var tab: Tab? = null private val checkbox = view.tab_selected_checkbox!! - private val checkboxListener = CompoundButton.OnCheckedChangeListener { _, isChecked -> - tab?.apply { - val action = if (isChecked) CollectionCreationAction.AddTabToSelection(this) - else CollectionCreationAction.RemoveTabFromSelection(this) - - actionEmitter.onNext(action) - } - } init { view.collection_item_tab.setOnClickListener { @@ -128,16 +152,13 @@ class TabViewHolder( fun bind(tab: Tab, isSelected: Boolean, shouldHideCheckBox: Boolean) { this.tab = tab - view.hostname.text = tab.hostname view.tab_title.text = tab.title checkbox.visibility = if (shouldHideCheckBox) View.INVISIBLE else View.VISIBLE view.isClickable = !shouldHideCheckBox - checkbox.setOnCheckedChangeListener(null) if (checkbox.isChecked != isSelected) { checkbox.isChecked = isSelected } - checkbox.setOnCheckedChangeListener(checkboxListener) launch(Dispatchers.IO) { val bitmap = view.favicon_image.context.components.core.icons diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationUIView.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationUIView.kt index b2e2a77af..fa9434dec 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationUIView.kt @@ -11,6 +11,7 @@ import android.view.View import android.view.ViewGroup import android.view.inputmethod.EditorInfo import androidx.constraintlayout.widget.ConstraintSet +import androidx.core.content.ContextCompat import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import androidx.transition.AutoTransition @@ -67,15 +68,12 @@ class CollectionCreationUIView( R.layout.component_collection_creation_name_collection ) - view.close_icon.apply { + view.bottom_bar_icon_button.apply { increaseTapArea(increaseButtonByDps) - setOnClickListener { - actionEmitter.onNext(CollectionCreationAction.Close) - } } view.name_collection_edittext.setOnEditorActionListener { v, actionId, _ -> - if (actionId == EditorInfo.IME_ACTION_DONE && !v.text.toString().isEmpty()) { + if (actionId == EditorInfo.IME_ACTION_DONE && v.text.toString().isNotEmpty()) { when (step) { is SaveCollectionStep.NameCollection -> { actionEmitter.onNext( @@ -95,10 +93,6 @@ class CollectionCreationUIView( false } - view.add_collection_button.setOnClickListener { - actionEmitter.onNext(CollectionCreationAction.AddNewCollection) - } - view.tab_list.run { adapter = collectionCreationTabListAdapter layoutManager = LinearLayoutManager(container.context, RecyclerView.VERTICAL, true) @@ -118,6 +112,8 @@ class CollectionCreationUIView( when (it.saveCollectionStep) { is SaveCollectionStep.SelectTabs -> { + view.tab_list.isClickable = true + back_button.setOnClickListener { actionEmitter.onNext(CollectionCreationAction.BackPressed(SaveCollectionStep.SelectTabs)) } @@ -134,6 +130,18 @@ class CollectionCreationUIView( actionEmitter.onNext(CollectionCreationAction.SelectAllTapped) } } + + view.bottom_button_bar_layout.setOnClickListener(null) + view.bottom_button_bar_layout.isClickable = false + + val drawable = view.context.getDrawable(R.drawable.ic_close) + drawable?.setTint(ContextCompat.getColor(view.context, R.color.photonWhite)) + view.bottom_bar_icon_button.setImageDrawable(drawable) + + view.bottom_bar_icon_button.setOnClickListener { + actionEmitter.onNext(CollectionCreationAction.Close) + } + TransitionManager.beginDelayedTransition( view.collection_constraint_layout, transition @@ -156,7 +164,7 @@ class CollectionCreationUIView( ) } - view.select_tabs_layout_text.text = selectTabsText + view.bottom_bar_text.text = selectTabsText save_button.setOnClickListener { _ -> if (selectedCollection != null) { @@ -178,8 +186,22 @@ class CollectionCreationUIView( } } is SaveCollectionStep.SelectCollection -> { - // Only show selected tabs and hide checkboxes - collectionCreationTabListAdapter.updateData(it.selectedTabs.toList(), setOf(), true) + view.tab_list.isClickable = false + + save_button.visibility = View.GONE + + view.bottom_bar_text.text = + view.context.getString(R.string.create_collection_add_new_collection) + + val drawable = view.context.getDrawable(R.drawable.ic_new) + drawable?.setTint(ContextCompat.getColor(view.context, R.color.photonWhite)) + view.bottom_bar_icon_button.setImageDrawable(drawable) + view.bottom_bar_icon_button.setOnClickListener(null) + + view.bottom_button_bar_layout.isClickable = true + view.bottom_button_bar_layout.setOnClickListener { + actionEmitter.onNext(CollectionCreationAction.AddNewCollection) + } back_button.setOnClickListener { actionEmitter.onNext(CollectionCreationAction.BackPressed(SaveCollectionStep.SelectCollection)) @@ -194,6 +216,9 @@ class CollectionCreationUIView( view.context.getString(R.string.create_collection_select_collection) } is SaveCollectionStep.NameCollection -> { + view.tab_list.isClickable = false + + collectionCreationTabListAdapter.updateData(it.selectedTabs.toList(), it.selectedTabs, true) back_button.setOnClickListener { name_collection_edittext.hideKeyboard() val handler = Handler() @@ -231,6 +256,8 @@ class CollectionCreationUIView( view.context.getString(R.string.create_collection_name_collection) } is SaveCollectionStep.RenameCollection -> { + view.tab_list.isClickable = false + it.selectedTabCollection?.let { tabCollection -> tabCollection.tabs.map { tab -> Tab( @@ -240,7 +267,7 @@ class CollectionCreationUIView( tab.title ) }.let { tabs -> - collectionCreationTabListAdapter.updateData(tabs, setOf(), true) + collectionCreationTabListAdapter.updateData(tabs, tabs.toSet(), true) } } diff --git a/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionFragment.kt b/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionFragment.kt index aa6f8d240..741ec35d8 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionFragment.kt @@ -62,13 +62,15 @@ class CreateCollectionFragment : DialogFragment(), CoroutineScope { this, CollectionCreationViewModel::class.java ) { - CollectionCreationViewModel(CollectionCreationState( - viewModel.tabs, - viewModel.selectedTabs, - viewModel.saveCollectionStep, - viewModel.tabCollections, - viewModel.selectedTabCollection - )) + CollectionCreationViewModel( + CollectionCreationState( + viewModel.tabs, + viewModel.selectedTabs, + viewModel.saveCollectionStep, + viewModel.tabCollections, + viewModel.selectedTabCollection + ) + ) } ) return view @@ -100,7 +102,11 @@ class CreateCollectionFragment : DialogFragment(), CoroutineScope { is CollectionCreationAction.Close -> dismiss() is CollectionCreationAction.SaveTabsToCollection -> { getManagedEmitter() - .onNext(CollectionCreationChange.StepChanged(SaveCollectionStep.SelectCollection)) + .onNext( + CollectionCreationChange.StepChanged( + viewModel.tabCollections.getStepForCollectionsSize() + ) + ) } is CollectionCreationAction.AddTabToSelection -> { getManagedEmitter() @@ -189,11 +195,16 @@ class CreateCollectionFragment : DialogFragment(), CoroutineScope { CollectionCreationChange.StepChanged(SaveCollectionStep.SelectTabs) ) SaveCollectionStep.NameCollection -> { - getManagedEmitter().onNext( - CollectionCreationChange.StepChanged(SaveCollectionStep.SelectCollection) - ) + getManagedEmitter() + .onNext( + CollectionCreationChange.StepChanged( + viewModel.tabCollections.getStepForCollectionsSize() + ) + ) + } + SaveCollectionStep.RenameCollection -> { + dismiss() } - SaveCollectionStep.RenameCollection -> { dismiss() } } } } diff --git a/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionViewModel.kt b/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionViewModel.kt index 0c8d1b8dd..b90e26b21 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionViewModel.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CreateCollectionViewModel.kt @@ -17,3 +17,6 @@ class CreateCollectionViewModel : ViewModel() { var selectedTabCollection: TabCollection? = null var snackbarAnchorView: View? = null } + +fun List.getStepForCollectionsSize(): SaveCollectionStep = + if (isEmpty()) SaveCollectionStep.NameCollection else SaveCollectionStep.SelectCollection diff --git a/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt b/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt index bf1e7bd7f..cf4a69404 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt @@ -7,13 +7,17 @@ package org.mozilla.fenix.collections import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.core.content.ContextCompat import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer import kotlinx.android.synthetic.main.collections_list_item.view.* +import kotlinx.android.synthetic.main.collections_list_item.view.collection_description +import kotlinx.android.synthetic.main.collections_list_item.view.collection_icon import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import org.mozilla.fenix.R +import org.mozilla.fenix.ext.urlToTrimmedHost import org.mozilla.fenix.home.sessioncontrol.Tab import org.mozilla.fenix.home.sessioncontrol.TabCollection import kotlin.coroutines.CoroutineContext @@ -78,9 +82,46 @@ class CollectionViewHolder( fun bind(collection: TabCollection) { this.collection = collection view.collection_item.text = collection.title + + val hostNameList = collection.tabs.map { it.url.urlToTrimmedHost().capitalize() } + + var tabsDisplayed = 0 + val tabTitlesList = hostNameList.joinToString(", ") { + if (it.length > maxTitleLength) { + it.substring( + 0, + maxTitleLength + ) + "..." + } else { + tabsDisplayed += 1 + it + } + } + view.collection_description.text = tabTitlesList + + view.collection_icon.setColorFilter( + ContextCompat.getColor( + view.context, + getIconColor(collection.id) + ), + android.graphics.PorterDuff.Mode.SRC_IN + ) + } + + @Suppress("ComplexMethod", "MagicNumber") + private fun getIconColor(id: Long): Int { + return when ((id % 4).toInt()) { + 0 -> R.color.collection_icon_color_violet + 1 -> R.color.collection_icon_color_blue + 2 -> R.color.collection_icon_color_pink + 3 -> R.color.collection_icon_color_green + 4 -> R.color.collection_icon_color_yellow + else -> R.color.white_color + } } companion object { const val LAYOUT_ID = R.layout.collections_list_item + const val maxTitleLength = 20 } } 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 398a3cc1d..09425fb25 100644 --- a/app/src/main/res/layout/collection_home_list_row.xml +++ b/app/src/main/res/layout/collection_home_list_row.xml @@ -2,9 +2,9 @@ - + tools:text="@tools:sample/lorem/random" /> @@ -76,7 +76,8 @@ app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toStartOf="@id/collection_share_button" app:layout_constraintStart_toStartOf="@id/collection_title" - app:layout_constraintTop_toBottomOf="@id/collection_share_button" /> + app:layout_constraintTop_toBottomOf="@id/collection_share_button" + tools:text="@tools:sample/lorem/random" /> diff --git a/app/src/main/res/layout/collections_list_item.xml b/app/src/main/res/layout/collections_list_item.xml index 68c0d03b0..1de229d27 100644 --- a/app/src/main/res/layout/collections_list_item.xml +++ b/app/src/main/res/layout/collections_list_item.xml @@ -1,35 +1,66 @@ - + + android:layout_marginTop="8dp" + android:layout_marginBottom="8dp" + android:clickable="true" + android:clipToPadding="false" + android:focusable="true" + android:foreground="?android:attr/selectableItemBackground" + app:cardBackgroundColor="?above" + app:cardCornerRadius="@dimen/tab_corner_radius" + app:cardElevation="5dp"> - + android:layout_margin="16dp"> - - + + + + + + + diff --git a/app/src/main/res/layout/component_collection_creation.xml b/app/src/main/res/layout/component_collection_creation.xml index 77de408a0..6b188945a 100644 --- a/app/src/main/res/layout/component_collection_creation.xml +++ b/app/src/main/res/layout/component_collection_creation.xml @@ -44,38 +44,11 @@ android:layout_width="match_parent" android:layout_height="wrap_content" android:visibility="gone" - app:layout_constraintBottom_toTopOf="@id/add_collection_button" + app:layout_constraintBottom_toTopOf="@id/bottom_button_bar_layout" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/back_button"/> - - - - @@ -123,10 +96,11 @@ app:layout_constraintTop_toTopOf="parent" />