2

I have been following the ViewHolder pattern for a ListView, below the code. Actually it works well - but I have a problem.

My items in the adapter have the following layout:

================================================================
|    Item title                    |AddIcon|RemoveIcon|EditIcon|
================================================================

The problem is the following: When I click on the RemoveIcon, after the code runs, I want the RemoveIcon to change color. I do this in the second-last line of the code below with setImageResource. Now that also works fine - the icon is changed. But now, when I scroll, at every specific interval (on my phone it's 8 items), the icon ALSO CHANGES. Looks like caching kicks in here?

Questions:

  1. Can I use this caching when layout is essentially individual for each item?
  2. If I maybe cannot, will it be ok to use a un-cached layout and repeat it for each item? Currently I may have around 100 items in the list. Is there a better way to do this?

    public class ItemListAdapter extends ArrayAdapter<StoreItem> {
        static class ViewHolder {
            public TextView title;
            public ImageButton more;
            public ImageButton less;
            public ImageButton edit;
        }
    
    
        @Override
        public View getView(final int position, View convertView, ViewGroup parent) {
            View rowView = convertView;
            // reuse views
            if (rowView == null) {
                LayoutInflater inflater = mContext.getLayoutInflater();
                rowView = inflater.inflate(R.layout.item_layout, parent, false);
    
                ViewHolder viewHolder = new ViewHolder();
                viewHolder.title = (TextView) rowView.findViewById(R.id.title);
    
                viewHolder.more = (ImageButton) rowView.findViewById(R.id.item_mas);
                viewHolder.less = (ImageButton) rowView.findViewById(R.id.item_menos);
                viewHolder.edit = (ImageButton) rowView.findViewById(R.id.item_edit);
                rowView.setTag(viewHolder);
            }
    
            final ViewHolder holder = (ViewHolder) rowView.getTag();
    
            final StoreItem item = mItems.get(position);
            holder.title.setText(item.getTitle());
    
            holder.less.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                    //dialog is a confirmation dialog, initialization omittted for brevity
                    dialog.setConfirmClickListener(new View.OnClickListener() {
                        @Override
                        public void onClick(View v) {
                            removeItem(item, v);
                            holder.less.setImageResource(R.drawable.x_ok);
                            dialog.getDialog().cancel();
                        }
                    });
                }
            });
    
Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
transient_loop
  • 5,984
  • 15
  • 58
  • 117

1 Answers1

1

The color of the items is a type of state. When using any kind of caching pattern, such as the View Holder pattern, you must make sure to represent that state when setting up your views.

The problem here is not that you change the drawable to R.drawable.x_ok on click, the problem is that you don't account for the unclicked state in your initial setup.

First, save the state in your Holder object

public void onClick(View v) {
    removeItem(item, v);
    holder.setLessClicked(true);
    holder.less.setImageResource(R.drawable.x_ok);
    dialog.getDialog().cancel();
}

Now, whenever you go through the getView method, apply that state correctly:

final ViewHolder holder = (ViewHolder) rowView.getTag();

final StoreItem item = mItems.get(position);
holder.title.setText(item.getTitle());
if(holder.isLessClicked()){
    holder.less.setImageResource(R.drawable.x_ok);
}
else{
    holder.less.setImageResource(R.drawable.x_normal);
}

The important part to remember is the else condition. You are now correctly setting both states, clicked and unclicked for every possible row. Whereas before, you were not properly handling the unclicked state for all other cells besides your clicked one.

jacobhyphenated
  • 2,105
  • 2
  • 17
  • 27