1
0
Fork 0

For #13156, #13280 - Use payloads to bind bookmark viewholders.

Change the DiffUtil callback for bookmarks to use the generated equals()
method instead of only checking the title and url fields. This prevents
the BookmarkNode in our state from getting out of sync with the
BookmarkNode the viewholder is bound to.
master
Kainalu Hagiwara 2020-08-04 16:26:03 -07:00 committed by Jeff Boek
parent 0aab7a806a
commit 6d8cfe1a50
6 changed files with 111 additions and 26 deletions

View File

@ -59,9 +59,20 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto
old[oldItemPosition].guid == new[newItemPosition].guid
override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean =
oldMode::class == newMode::class &&
old[oldItemPosition] in oldMode.selectedItems == new[newItemPosition] in newMode.selectedItems &&
old[oldItemPosition].title == new[newItemPosition].title &&
old[oldItemPosition].url == new[newItemPosition].url
old[oldItemPosition] == new[newItemPosition]
override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int): Any? {
val oldItem = old[oldItemPosition]
val newItem = new[newItemPosition]
return BookmarkPayload(
titleChanged = oldItem.title != newItem.title,
urlChanged = oldItem.url != newItem.url,
selectedChanged = oldItem in oldMode.selectedItems != newItem in newMode.selectedItems,
modeChanged = oldMode::class != newMode::class
)
}
override fun getOldListSize(): Int = old.size
override fun getNewListSize(): Int = new.size
@ -90,9 +101,36 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto
override fun getItemCount(): Int = tree.size
override fun onBindViewHolder(
holder: BookmarkNodeViewHolder,
position: Int,
payloads: MutableList<Any>
) {
if (payloads.isNotEmpty() && payloads[0] is BookmarkPayload) {
holder.bind(tree[position], mode, payloads[0] as BookmarkPayload)
} else {
super.onBindViewHolder(holder, position, payloads)
}
}
override fun onBindViewHolder(holder: BookmarkNodeViewHolder, position: Int) {
holder.bind(tree[position], mode)
}
}
/**
* A RecyclerView Adapter payload class that contains information about changes to a [BookmarkNode].
*
* @property titleChanged true if there has been a change to [BookmarkNode.title].
* @property urlChanged true if there has been a change to [BookmarkNode.url].
* @property selectedChanged true if there has been a change in the BookmarkNode's selected state.
* @property modeChanged true if there has been a change in the state's mode type.
*/
data class BookmarkPayload(
val titleChanged: Boolean,
val urlChanged: Boolean,
val selectedChanged: Boolean,
val modeChanged: Boolean
)
fun BookmarkNode.inRoots() = enumValues<BookmarkRoot>().any { it.id == guid }

View File

@ -139,7 +139,6 @@ class BookmarkView(
}
fun update(state: BookmarkFragmentState) {
val oldMode = mode
tree = state.tree
if (state.mode != mode) {
mode = state.mode
@ -149,9 +148,6 @@ class BookmarkView(
}
bookmarkAdapter.updateData(state.tree, mode)
if (state.mode != oldMode) {
bookmarkAdapter.notifyDataSetChanged()
}
when (mode) {
is BookmarkFragmentState.Mode.Normal -> {

View File

@ -14,6 +14,7 @@ import org.mozilla.fenix.ext.showAndEnable
import org.mozilla.fenix.library.LibrarySiteItemView
import org.mozilla.fenix.library.SelectionHolder
import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState
import org.mozilla.fenix.library.bookmarks.BookmarkPayload
import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor
import org.mozilla.fenix.library.bookmarks.inRoots
@ -28,28 +29,39 @@ class BookmarkFolderViewHolder(
override var item: BookmarkNode? = null
init {
containerView.displayAs(LibrarySiteItemView.ItemType.FOLDER)
}
override fun bind(
item: BookmarkNode,
mode: BookmarkFragmentState.Mode
) {
this.item = item
bind(item, mode, BookmarkPayload(true, true, true, true))
}
containerView.displayAs(LibrarySiteItemView.ItemType.FOLDER)
override fun bind(item: BookmarkNode, mode: BookmarkFragmentState.Mode, payload: BookmarkPayload) {
this.item = item
setSelectionListeners(item, selectionHolder)
if (!item.inRoots()) {
setupMenu(item)
if (mode is BookmarkFragmentState.Mode.Selecting) {
containerView.overflowView.hideAndDisable()
} else {
containerView.overflowView.showAndEnable()
if (payload.modeChanged) {
if (mode is BookmarkFragmentState.Mode.Selecting) {
containerView.overflowView.hideAndDisable()
} else {
containerView.overflowView.showAndEnable()
}
}
} else {
containerView.overflowView.visibility = View.GONE
}
containerView.changeSelected(item in selectionHolder.selectedItems)
if (payload.selectedChanged) {
containerView.changeSelected(item in selectionHolder.selectedItems)
}
containerView.iconView.setImageDrawable(
AppCompatResources.getDrawable(
containerView.context,
@ -63,6 +75,9 @@ class BookmarkFolderViewHolder(
)
}
)
containerView.titleView.text = item.title
if (payload.titleChanged) {
containerView.titleView.text = item.title
}
}
}

View File

@ -10,6 +10,7 @@ import org.mozilla.fenix.ext.showAndEnable
import org.mozilla.fenix.library.LibrarySiteItemView
import org.mozilla.fenix.library.SelectionHolder
import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState
import org.mozilla.fenix.library.bookmarks.BookmarkPayload
import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor
/**
@ -23,27 +24,46 @@ class BookmarkItemViewHolder(
override var item: BookmarkNode? = null
init {
containerView.displayAs(LibrarySiteItemView.ItemType.SITE)
}
override fun bind(
item: BookmarkNode,
mode: BookmarkFragmentState.Mode
) {
bind(item, mode, BookmarkPayload(true, true, true, true))
}
override fun bind(item: BookmarkNode, mode: BookmarkFragmentState.Mode, payload: BookmarkPayload) {
this.item = item
containerView.displayAs(LibrarySiteItemView.ItemType.SITE)
if (mode is BookmarkFragmentState.Mode.Selecting) {
containerView.overflowView.hideAndDisable()
} else {
containerView.overflowView.showAndEnable()
}
setupMenu(item)
containerView.titleView.text = if (item.title.isNullOrBlank()) item.url else item.title
containerView.urlView.text = item.url
if (payload.modeChanged) {
if (mode is BookmarkFragmentState.Mode.Selecting) {
containerView.overflowView.hideAndDisable()
} else {
containerView.overflowView.showAndEnable()
}
}
if (payload.selectedChanged) {
containerView.changeSelected(item in selectionHolder.selectedItems)
}
if (payload.titleChanged) {
containerView.titleView.text = if (item.title.isNullOrBlank()) item.url else item.title
} else if (payload.urlChanged && item.title.isNullOrBlank()) {
containerView.titleView.text = item.url
}
if (payload.urlChanged) {
containerView.urlView.text = item.url
setColorsAndIcons(item.url)
}
setSelectionListeners(item, selectionHolder)
containerView.changeSelected(item in selectionHolder.selectedItems)
setColorsAndIcons(item.url)
}
private fun setColorsAndIcons(url: String?) {

View File

@ -11,6 +11,7 @@ import org.mozilla.fenix.library.LibrarySiteItemView
import org.mozilla.fenix.library.SelectionHolder
import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState
import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu
import org.mozilla.fenix.library.bookmarks.BookmarkPayload
import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor
/**
@ -28,6 +29,12 @@ abstract class BookmarkNodeViewHolder(
mode: BookmarkFragmentState.Mode
)
abstract fun bind(
item: BookmarkNode,
mode: BookmarkFragmentState.Mode,
payload: BookmarkPayload
)
protected fun setSelectionListeners(item: BookmarkNode, selectionHolder: SelectionHolder<BookmarkNode>) {
containerView.setSelectionInteractor(item, selectionHolder, interactor)
}

View File

@ -7,6 +7,7 @@ package org.mozilla.fenix.library.bookmarks.viewholders
import mozilla.components.concept.storage.BookmarkNode
import org.mozilla.fenix.library.LibrarySiteItemView
import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState
import org.mozilla.fenix.library.bookmarks.BookmarkPayload
import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor
/**
@ -27,4 +28,12 @@ class BookmarkSeparatorViewHolder(
containerView.displayAs(LibrarySiteItemView.ItemType.SEPARATOR)
setupMenu(item)
}
override fun bind(
item: BookmarkNode,
mode: BookmarkFragmentState.Mode,
payload: BookmarkPayload
) {
bind(item, mode)
}
}