onListViewItemClick View argument refer to multiple listitems

Question

In onListItemClick I want to change clicked item background, this is my code:

public void onListItemClick(ListView parent, View v, int position, long id){

    if (position != previous_position){
        v.setBackgroundResource(R.drawable.category_clicked_item_bg);
        if (previous_category != null)
            previous_category.setBackgroundResource(R.drawable.category_item_bg);
        previous_category = v;
        previous_position = position;
    }
}

previous_category and previous_position are protected variables.

The problem is, that on item click multiple items in list are highlighted (more specifically 1 or 2). In list there are 20 rows, if I click one in middle, only one gets highlighted, if I click any row in beginning or end, other row in opposite side gets highlighted with interval 13 rows (that's probably the reason why pressing middle row don't highlight second).

I need list to highlight only one row the clicked one, what is the problem?

ADDITIONAL CODE

If that can help, here is also ListView adapter class:

package domehotel.guestbook.page.category;

import android.app.Activity;
import android.content.Context;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ArrayAdapter;
import android.widget.ImageView;
import android.widget.TextView;
import domehotel.guestbook.R;

public class CategoryAdapter extends ArrayAdapter<CategoryItem>{

Context context; 
int layoutResourceId;    
CategoryItem data[] = null;

public CategoryAdapter(Context context, int layoutResourceId, CategoryItem[] data) {
    super(context, layoutResourceId, data);
    this.layoutResourceId = layoutResourceId;
    this.context = context;
    this.data = data;
}

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    View row = convertView;
    CategoryHolder holder = null;

    if(row == null)
    {
        LayoutInflater inflater = ((Activity)context).getLayoutInflater();
        row = inflater.inflate(layoutResourceId, parent, false);

        holder = new CategoryHolder();
        holder.category_name = (TextView)row.findViewById(R.id.category_name);

        row.setTag(holder);
    }
    else
    {
        holder = (CategoryHolder)row.getTag();
    }

    CategoryItem category = data[position];
    holder.category_name.setText(category.category_name);

    return row;
}

static class CategoryHolder
{
    TextView category_name;
}
}

Answer 1

In fact you are reusing views. getView takes as 2nd parameter a View named convertView. The ListView calls this method with an old View item (if there is one), which is instantiated (inflated) but not displayed at the moment. Your meth. onListItemClick now sets the backround of one of these Views, but this View will be reused later for another row, without resetting it's backround.

A possible soloution:

1. The click listener should only store the background color, or a clicked flag, for a clicked position somewhere and then ask the View (row) in question to rerender (it could also save state and then set the background directly).

2. The getView(..) method should always set the background color of a View according to the saved color/state for its position. Set a normal background for positions not marked as clicked or set another background for positions marked as clicked by the click listener.

Answer 2

Since the ListView row View objects are reused each for multiple rows its not good idea to keep information about which row is active in row View, better idea is to keep this information in Adapter (in my case CategoryAdapter). data Array of type CategoryItem keeps objects of following class:

public class CategoryItem {

    public String category_name;
    public Boolean is_active = false;

    public CategoryItem(){
        super();
    }

    public CategoryItem(String category_name){
        super();
        this.category_name = category_name.toUpperCase();
    }
}

And adapter, when redrawing an view item in onView method, checks if current row is active:

if (category.is_active)
    row.setBackgroundResource(activeBgDrawable);
else
    row.setBackgroundResource(inactiveBgDrawable);

So, when I click an item, I set CategoryItem is_active property to true and previous clicked is_active property to false:

if (position != previous_position){
    CategoryAdapter adapter = (CategoryAdapter)parent.getAdapter();
    CategoryItem current_item = adapter.getItem(position);
    current_item.is_active = true;
    if (previous_position != -1){
        CategoryItem previous_item = adapter.getItem(previous_position);
                    previous_item.is_active = false;
        }
    previous_position = position;
    adapter.notifyDataSetChanged();
}

Why setting the View objects background dont work? If I have ListView which contains ListView items more than the ListView can show, list will become scrollable and, for example, if I scroll ListView down, then when the upper item will slide out of visible part, the ListView item View object of this row will be reused for the next row which will appear at bottom. So, if the upper row would be the one whose View object background I would have changed then the when It would disappear the bottom row would be redrawn with upper ones View object with its background and thats why I some times had two colored rows.

Answer 3

Try the following code in OnItemClick

public void onListItemClick(ListView parent, View v, int position, long id){

if (position != previous_position){
    v.setBackgroundResource(R.drawable.category_clicked_item_bg);   
    Object view = parent.getTag();
    if(view!=null){
       // typecasting the Object to View
       ((View)view).setBackgroundResource(R.drawable.category_item_bg);
    }
    previous_position = position;
    parent.setTag(v);
}
}

I Hope this will solve your problem.