Listview - changing the backgrond color of an item

Question

I'm trying to have the background color of rows the user already clicked in a listview changed, to do this I'm using an arraylist saved in SharedPreferences contining the ids of the clicked rows, the problem is that the color changes almost randomly.

code:

    private static class ItemHolder {
        public TextView TXTTitle, TXTArtist, TXTid, TXTLikes;
        public RelativeLayout back;
    }

    @Override
    public View getView(final int position, View view, ViewGroup root) {
        ItemHolder holder = new ItemHolder();

        if (view == null) {
            view = inflater.inflate(R.layout.list_item, root, false);

            RelativeLayout _back = (RelativeLayout) view.findViewById(R.id.bg_item);

            holder.back = _back;

            // paint oldies
            Set<String> _set = new HashSet<String>();
            _set = Prefs.getStringSet("arrOfOldies", _set);
            for(int i = 0; i <= _set.size()  ; i++){
                if(_set.contains(IDs.get(position))){
                //oldie
                            holder.back.setBackgroundColor(Color.parseColor("#D0D0D0"));
                    }
            }

            view.setTag(holder);
        } else {
                holder = (ItemHolder) view.getTag();       
        }
    }

I think it has something to do with the way a listview is being built, what's the problem and how do I fix it?

How do you handle the `holder` ? Please post above code. The whole `Adapter` would be great ;) — Manitoba, Jun 26 '14 at 14:41
Check this one [GOOD LISTVIEW TUTORIAL](http://www.vogella.com/tutorials/AndroidListView/article.html) focus on holder patern so you dont need save selections in array — user3455363, Jun 26 '14 at 15:08

score 1 · Accepted Answer · answered Jun 26 '14 at 15:02

I would add to Manitoba's answer, you also have to reset the background if the item has never been selected. Understand that the point of using a convertView and an item holder is reusing views for optimisation. If you reuse a view from an item that has been selected for an item that's never been : the background color will still be here.

try :

@Override
public View getView(final int position, View view, ViewGroup root)
{
    ItemHolder holder = null;

    if (view == null)
    {
        view = inflater.inflate(R.layout.list_item, root, false);
        holder = new ItemHolder();

        holder.back = (RelativeLayout) view.findViewById(R.id.bg_item);
        view.setTag(holder);
    }
    else
    {
        holder = (ItemHolder) view.getTag();
    }

    // paint oldies
    Set<String> _set = new HashSet<String>();
    _set = Prefs.getStringSet("arrOfOldies", _set);
    for(int i = 0; i <= _set.size()  ; i++)
    {
        if(_set.contains(IDs.get(position)))
        {
            holder.back.setBackgroundColor(Color.parseColor("#D0D0D0"));
        }
        else 
        {
            //add this else clause.
            holder.back.setBackgroundColor( YOUR_DEFAULT_BACKGROUND_COLOR ); 
        }
    }
}

Manitoba · Answer 2 · 2014-06-26T15:02:27.910

You have made a small mistake, move the backgroundpart after you've retrieve the existing holder. Here is the clean code to use.

private static class ItemHolder
{
    public TextView TXTTitle, TXTArtist, TXTid, TXTLikes;
    public RelativeLayout back;

    @Override
    public View getView(final int position, View view, ViewGroup root)
    {
        ItemHolder holder = null;

        if (view == null)
        {
            view = inflater.inflate(R.layout.list_item, root, false);
            holder = new ItemHolder();

            holder.back = (RelativeLayout) view.findViewById(R.id.bg_item);
            view.setTag(holder);
        }
        else
        {
            holder = (ItemHolder) view.getTag();
        }

        // paint oldies
        Set<String> _set = new HashSet<String>();
        _set = Prefs.getStringSet("arrOfOldies", _set);
        if(_set.contains(IDs.get(position)))
        {
            //oldie
            holder.back.setBackgroundColor(Color.parseColor("#D0D0D0"));
        }else{
            holder.back.setBackgroundColor(Color.parseColor("#ffffff"));
        }
    }
}

thanks for the answer but the backgroung keeps changing randomly, it starts in the right way but changes when I scroll — Developer, Jun 26 '14 at 15:00

score 0 · Answer 3 · edited May 23 '17 at 11:49

Your logic is wrong. You are not taking recycling ListViews into consideration in your code. Also, You do not need need to check if the _set contains your item AS MANY TIMES AS THERE ARE OBJECTS IN YOUR SET. You should refactor your code to reflect these points:

Your biggest problem is that you are NEVER setting you holper variable unless your view is null, and it is only null as many times as there are list items that fit on your screen at the same time - when they get recycled, your color gets weird. Fix this, and it will get resolved. Here is some ListView recycling info
Move your _set out of the getView function
get rid of for loop iterating the values of your _set

Listview - changing the backgrond color of an item

3 Answers3