ListAdapter: Item click listener not re-binded after submitList

66 views Asked by At

I have a problem with ListAdapter despite that it is probably working as intended is becoming a bug on my end.

Let say I have this data class

data class AssetMinDataDomain(
    val id: String = "",
    val metricsMinDomain: MetricsMinDomain = MetricsMinDomain(),
    val name: String = "",
    val symbol: String? = ""
) {

    var isSelected: Boolean = name == DEFAULT_ASSET // Default selected item in Market Cap

    companion object {
        const val DEFAULT_ASSET = "Bitcoin"
    }

}

I want to focus on changes with price and selected status so we have this setup on DiffUtil.

class DiffUtilAssetMin : DiffUtil.ItemCallback<AssetMinDataDomain>() {

    // DiffUtil uses this test to help discover if an item was added, removed, or moved.
    // Use attribute(s) that represent object's identity.
    override fun areItemsTheSame(
        oldItem: AssetMinDataDomain,
        newItem: AssetMinDataDomain
    ): Boolean {
        return oldItem.id == newItem.id
    }

    // Check whether oldItem and newItem contain the same data; that is, whether they are equal.
    // If there are differences between oldItem and newItem, this code tells DiffUtil that the item has been updated.
    // Note: If you are using data class and trying to detect changes based on properties outside primary constructor,
    // you may need to do additional checking since the default generated `equals` only uses properties inside primary constructor.
    override fun areContentsTheSame(
        oldItem: AssetMinDataDomain,
        newItem: AssetMinDataDomain
    ): Boolean {
        return oldItem == newItem && oldItem.isSelected == newItem.isSelected
    }

    override fun getChangePayload(oldItem: AssetMinDataDomain, newItem: AssetMinDataDomain): Any? {

        if (oldItem.id == newItem.id) {
            return if (oldItem.metricsMinDomain.priceUsd == newItem.metricsMinDomain.priceUsd
                && oldItem.isSelected == newItem.isSelected
            ) {
                super.getChangePayload(oldItem, newItem)
            } else {
                // Add object's attribute(s) that has changed using this payload
                Bundle().apply {
                    newItem.metricsMinDoman.priceUsd?.let {
                        putDouble(ARG_MARKET_PRICE, it)
                    }
                    putBoolean(ARG_IS_SELECTED, newItem.isSelected)
                }
            }
        }

        return super.getChangePayload(oldItem, newItem)

    }

    companion object {
        const val ARG_MARKET_PRICE = "arg.market.price"
        const val ARG_IS_SELECTED = "arg.is.selected"
    }
}

We are doing full bind or partial bind depending if the ViewHolder is recycled or the data gets updated after calling adapter.submitList(items)

class AssetMinAdapter(
    private val iconLink: String,
    private val glide: RequestManager,
    private val maximumSelectedAsset: Int,
    private val itemListener: ItemListener
) : FilterableListAdapter<AssetMinDataDomain, AssetMinAdapter.ItemView>(DiffUtilAssetMin()) {

    inner class ItemView(itemView: AssetMinCardBinding) : RecyclerView.ViewHolder(itemView.root) {
        internal val cardView = itemView.cardRoot
        private val assetName = itemView.assetName
        private val assetSymbol = itemView.assetSymbol
        private val assetPrice = itemView.assetPrice
        private val assetIcon = itemView.assetIcon

        // Full update/binding
        fun bindFull(domain: AssetMinDataDomain) {

            with(itemView.context) {

                bindTextData(
                    domain.name,
                    domain.symbol,
                    domain.metricsMinDomain.marketDataMinDomain.priceUsd,
                    domain.isSelected
                )

                glide
                    .load(
                        getString(
                            R.string.icon_url,
                            iconLink,
                            domain.id
                        )
                    )
                    .circleCrop()
                    .into(assetIcon)

            }

        }

        // Partial update/binding
        fun bindPartial(domain: AssetMinDataDomain, bundle: Bundle) {
            bindTextData(
                domain.name,
                domain.symbol,
                bundle.getDouble(DiffUtilAssetMin.ARG_MARKET_PRICE),
                bundle.getBoolean(DiffUtilAssetMin.ARG_IS_SELECTED)
            )
        }

        private fun bindTextData(name: String, symbol: String?, price: Double?, isSelected: Boolean) {

            with(itemView.context) {
                assetName.text = name
                assetSymbol.text = symbol ?: getString(R.string.empty)
                assetPrice.text =
                    getString(R.string.us_dollars, NumbersUtil.formatFractional(price))
                cardView.isChecked = isSelected
            }

        }

    }

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ItemView =
        ItemView(
            AssetMinCardBinding.inflate(
                LayoutInflater.from(parent.context),
                parent,
                false
            )
        )

    override fun onBindViewHolder(holder: ItemView, position: Int) {
        onBindViewHolder(holder, holder.bindingAdapterPosition, emptyList())
    }

    override fun onBindViewHolder(holder: ItemView, position: Int, payloads: List<Any>) {

        with(holder) {

            val domain = getItem(bindingAdapterPosition)

            // Upon scroll we need to rebind click listener regardless if full or partial update
            // this is to ensure that click listener is bound to correct item.
            cardView.setOnClickListener {

                if (domain.name.equals(AssetMinDataDomain.DEFAULT_ASSET, true))
                    return@setOnClickListener

                val selectedSize = currentList.filter { it.isSelected }.size

                // To avoid abuse usage, limit the selectable asset market cap
                // this also including the default asset which is BTC
                if (selectedSize >= maximumSelectedAsset && !domain.isSelected) {
                    itemListener.onLimitReached()
                    return@setOnClickListener
                }

                domain.isSelected = !cardView.isChecked
                cardView.isChecked = domain.isSelected
                itemListener.onAssetSelected()

            }

            if (payloads.isEmpty() || payloads.first() !is Bundle)
                bindFull(domain) // Full update/binding
            else {

                val bundle = payloads.first() as Bundle

                bindPartial(domain, bundle) // Partial update/binding

            }

        }

    }

    // Required when setHasStableIds is set to true
    override fun getItemId(position: Int): Long {
        return currentList[position].id.hashCode().toLong()
    }

    override fun onFilter(
        list: List<AssetMinDataDomain>,
        constraint: String
    ): List<AssetMinDataDomain> {

        return list.filter {
            it.name.lowercase().contains(constraint.lowercase()) ||
            it.symbol?.lowercase()?.contains(constraint.lowercase()) == true ||
            it.name.equals(AssetMinDataDomain.DEFAULT_ASSET, true)
        }

    }

    // Since adapter.currentList() does not immediately reflecting the actual update from filters
    // we can use this callback instead to listen and get the latest list
    override fun onCurrentListChanged(
        previousList: List<AssetMinDataDomain>,
        currentList: List<AssetMinDataDomain>
    ) {
        super.onCurrentListChanged(previousList, currentList)
        itemListener.onListUpdate(previousList, currentList)
    }

    interface ItemListener {

        fun onAssetSelected()
        fun onLimitReached()
        fun onListUpdate(
            previousList: List<AssetMinDataDomain>,
            currentList: List<AssetMinDataDomain>
        )

    }

}

This works well fine unless I call adapter.submitList(item) again with the same set of items and contents except the isSelected flag. What happening here is the setOnClickListener is not getting updated for first set of items visible to screen and still pointing to old set of items.

Let say we have RecyclerView with GridLayoutManager span count of 3.

// First Row
BTC(isSelected = true) // default
ETH(isSelected = false)
BNB(isSelected = false)

// Second Row
DAI(isSelected = false)
USDT(isSelected = false) 
SOL(isSelected = false)

// Third Row
ADA(isSelected = false)
XRP(isSelected = false)
DOGE(isSelected = false)

BTC, ETH, BNB, DAI, USDT, SOL are visible to the screen while ADA, XRP, DOGE are slightly visible.

Selecting ETH, DAI, USDT is working properly as the CardView got marked and the click listener is pointing to correct item and correct data.

// First Row
BTC(isSelected = true) // default
ETH(isSelected = true)
BNB(isSelected = false)

// Second Row
DAI(isSelected = true)
USDT(isSelected = true) 
SOL(isSelected = false)

// Third Row
ADA(isSelected = false)
XRP(isSelected = false)
DOGE(isSelected = false)

If we call adapter.submitList(items) with the same number of items and content except of course for isSelected flag which will be set to false again because that is the default state. It will refresh the RecyclerView and update all the checked mark status (isSelected) to default which is correct again.

// First Row
BTC(isSelected = true) // default
ETH(isSelected = false)
BNB(isSelected = false)

// Second Row
DAI(isSelected = false)
USDT(isSelected = false) 
SOL(isSelected = false)

// Third Row
ADA(isSelected = false)
XRP(isSelected = false)
DOGE(isSelected = false)

However, if you select BNB, SOL, ADA, XRP, and DOGE the click listener is still pointing to old reference of data set while selecting again the ETH, DAI, USDT are working just fine.

// First Row
BTC (isSelected = true) // default
ETH (isSelected = false) // working as expected
BNB(isSelected = true but pointing to old reference)

// Second Row
DAI(isSelected = false) // working as expected
USDT(isSelected = false) // working as expected
SOL(isSelected = true but pointing to old reference)

// Third Row
ADA(isSelected = true but pointing to old reference)
XRP (isSelected = true but pointing to old reference)
DOGE (isSelected = true but pointing to old reference)

I discover that override fun onBindViewHolder(holder: ItemView, position: Int, payloads: List<Any>) only do full bind on below items that are not visible yet and do partial bind to ETH, DAI, USDT (this is the work of DiffUtil getChangePayload and most likely the reason why it is having no issue) but skips the rebinding on BTC, BNB, SOL, ADA, XRP, and DOGE which are previously not touch.

My question is how to properly do binding for click listener here if the adapter's onBindViewHolder skips rebinding those item? For the framework it is unnecessary to rebind it even partially as there is nothing changed on the data of those item, but doing so leave us with an outdated setOnClickListener

1

There are 1 answers

0
Bitwise DEVS On BEST ANSWER

Finally found a fix to this, all you have to do is moved the click logic to Fragment or Activity that holds your RecyclerView and its adapter.

adapter = AssetMinAdapter(
               AppConfig.remote.iconLink,
               glide,
               object : AssetMinAdapter.ItemListener {

                    override fun onAssetSelected(position: Int, cardView: MaterialCardView) {

                        val domain = adapter.currentList[position]
                        if (domain.name.equals(AssetMinDataDomain.DEFAULT_ASSET, true))
                            return

                        val selectedSize = adapter.currentList.filter { it.isSelected }.size

                        // To avoid abuse usage, limit the selectable asset market cap
                        // this also including the default asset which is BTC
                        if (selectedSize >= 6 && !domain.isSelected) {
                            showToast(getString(R.string.max_size_reach), Toast.LENGTH_LONG)
                            return
                        }

                        domain.isSelected = !cardView.isChecked
                        cardView.isChecked = domain.isSelected
                        
                        ...

Then just use interface for callback so that you are not bounded to outdated adapter's currentList and getItem.

// Upon scroll we need to rebind click listener regardless if full or partial update
                // this is to ensure that click listener is bound to correct item.

 cardView.setOnClickListener {
      itemListener.onAssetSelected(bindingAdapterPosition, cardView)
 }

With this I can even moved the click listener to full bind only with confidence that its event will always access the latest data set.