Recyclerview is putting images from one object in multiple positions-CodePudding

So I have found the source of the problem. Inside my Adapter for my Recyclerview I am trying to check if the imageName is null or empty, if it isn't then we can get the image from the local storage and put it into the holder's ImageView. When doing this check and only having an image in the first item in the list of items in the adapter, the image will be placed in multiple positions... And if I keep spinning the recyclerview it continuously adds more. The reason I want to do this is because I want a default image of a plus icon to trigger more logic, otherwise the image that's stored if it's available. Below is the first part of the code that creates the issue, and some images for you to see what's happening.

package com.example.dukebox.adapters

import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Button
import android.widget.ImageView
import android.widget.TextView
import androidx.core.view.isVisible
import androidx.recyclerview.widget.RecyclerView
import com.bumptech.glide.Glide
import com.example.dukebox.FORWARD_SLASH
import com.example.dukebox.R
import com.example.dukebox.RECORD_IMAGES
import com.example.dukebox.models.Record
import com.example.dukebox.viewmodel.HomeViewModel

class NumberSelectionAdapter(
    private val recordInfoTitle: TextView,
    private val viewModel: HomeViewModel
): RecyclerView.Adapter<NumberSelectionAdapter.ViewHolder>() {

    var selectedPosition = 0
    private var records: List<Record> = emptyList()
    private val imagesPath = "${recordInfoTitle.context.getExternalFilesDir(null)?.absolutePath}$RECORD_IMAGES$FORWARD_SLASH"

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
        return ViewHolder(LayoutInflater.from(parent.context).inflate(R.layout.number_item, parent, false))
    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {
        val record = records[position]

        if (!record.imageName.isNullOrEmpty()){
            Glide.with(recordInfoTitle.context).load("$imagesPath$FORWARD_SLASH${record.imageName}").into(holder.recordCover)
        }
    }

    override fun getItemCount(): Int {
        return records.size
    }

    fun updateList(newRecords: List<Record>) {
        records = newRecords
        notifyDataSetChanged()
    }

    class ViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {
        val recordCover: ImageView = itemView.findViewById(R.id.recordCover)
    }
}

And then the code that only puts the image where it's supposed to be the entire time. As you can see, the only difference is the null or empty check in the onBindViewHolder.

package com.example.dukebox.adapters

import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Button
import android.widget.ImageView
import android.widget.TextView
import androidx.core.view.isVisible
import androidx.recyclerview.widget.RecyclerView
import com.bumptech.glide.Glide
import com.example.dukebox.FORWARD_SLASH
import com.example.dukebox.R
import com.example.dukebox.RECORD_IMAGES
import com.example.dukebox.models.Record
import com.example.dukebox.viewmodel.HomeViewModel

class NumberSelectionAdapter(
    private val recordInfoTitle: TextView,
    private val viewModel: HomeViewModel
): RecyclerView.Adapter<NumberSelectionAdapter.ViewHolder>() {

    var selectedPosition = 0
    private var records: List<Record> = emptyList()
    private val imagesPath = "${recordInfoTitle.context.getExternalFilesDir(null)?.absolutePath}$RECORD_IMAGES$FORWARD_SLASH"

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
        return ViewHolder(LayoutInflater.from(parent.context).inflate(R.layout.number_item, parent, false))
    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {
        val record = records[position]
        
        Glide.with(recordInfoTitle.context).load("$imagesPath$FORWARD_SLASH${record.imageName}").into(holder.recordCover)
    }

    override fun getItemCount(): Int {
        return records.size
    }

    fun updateList(newRecords: List<Record>) {
        records = newRecords
        notifyDataSetChanged()
    }

    class ViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {
        val recordCover: ImageView = itemView.findViewById(R.id.recordCover)
    }
}

And some pictures of what it looks like, because it tries to load an image that doesn't exist, the plus signs aren't there. But the image is only where it's suppose to be. I span it multiple times, and there is only ever one.

CodePudding user response：

RecyclerViews reuse (recycle) their ViewHolders, so in onBindViewHolder you're usually getting one that's already displaying stuff for another item. You need to update it so it looks right for your current item.

Here's what you're doing

override fun onBindViewHolder(holder: ViewHolder, position: Int) {
    val record = records[position]
    // this ONLY updates IF there's an imageName
    if (!record.imageName.isNullOrEmpty()){
        Glide.with(recordInfoTitle.context).load("$imagesPath$FORWARD_SLASH${record.imageName}").into(holder.recordCover)
    }
}

The problem is you set a new pic if there's an image name - but you don't clear it if there isn't. So if you're given a ViewHolder that was previously displaying an item with a image, that image is still there!

If you don't explicitly remove it, it's still going to show that old data. That goes for anything in onBindViewHolder - you need to update everything to the correct display state for the current item. That's why you're seeing the image "repeat" when it should only be on the first item - it's just the ViewHolder that was used for the first item showing up again, and unless you change the contents of recordCover, it's gonna be there every time

Since you're using Glide, you should use its clear() function to wipe the image, as it explains in the docs:

By calling clear() or into(View) on the View, you’re cancelling the load and guaranteeing that Glide will not change the contents of the view after the call completes. If you forget to call clear() and don’t start a new load, the load you started into the same View for a previous position may complete after you set your special Drawable and change the contents of the View to an old image.

Basically, because you're telling Glide to load an image into a particular ViewHolder's ImageView, and that can happen asynchronously, it's possible (if you're scrolling fast) that you'll get to another item that uses that ViewHolder, set the contents to blank, but then Glide will show up after a delay with that earlier requested image and set it on that ImageView.

By using the clear() call through Glide, it can internally keep track of what jobs are queued up, and it can cancel a request for that ImageView if a newer one comes in, including a request to clear it. Let Glide take care of it all, basically!

So:

override fun onBindViewHolder(holder: ViewHolder, position: Int) {
    val record = records[position]
    // always update the ImageView to the correct state
    if (!record.imageName.isNullOrEmpty()){
        Glide.with(recordInfoTitle.context).load("$imagesPath$FORWARD_SLASH${record.imageName}").into(holder.recordCover)
    } else {
        Glide.with(recordInfoTitle.context).clear(holder.recordCover)
    }
}