25

whenever i check a checkbox in my listview , other random checkboxes get checked too . It could be due to item recycling by listview.

I also tried setting android:focusable="false" to checkbox in my layout as suggested in some places, but still the onListItemClick() is not called for a row when its checkbox is checked.Only when I click somewhere else it gets called.

What I want is that only the user-checked checkboxes should remain checked until the user unchecks them.

I give below the code which is complete and could be run directly.

Activity code- ProjActivity.java:

public class ProjActivity extends ListActivity {
/** Called when the activity is first created. */
@Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    PackageManager pm = getPackageManager();
    List<ApplicationInfo> packages = pm.getInstalledApplications(PackageManager.GET_META_DATA);

    final CopyOfMyCustomAdapter a = new CopyOfMyCustomAdapter(this, packages);
    getListView().setAdapter(a);
}}

And finally, the custom layout file- testlayout.xml

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="horizontal" 

>

<CheckBox
    android:id="@+id/checkBox1"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:layout_marginLeft="20dp"
    android:text="CheckBox" 
    android:focusable="false"
    />

<ImageView
    android:id="@+id/imageView1"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:src="@drawable/ic_launcher" 
    android:focusable="false"
    />

UPDATE : My CustomAdapter after the suggestion in an answer below:

public class MyCustomAdapter extends ArrayAdapter<ApplicationInfo>  {

private List<ApplicationInfo> appInfoList;
private LayoutInflater mInflater;
private PackageManager pm;
ArrayList<Boolean> positionArray;
private Context ctx;
int[] visiblePosArray;
private volatile int positionCheck; 

public MyCustomAdapter(Context context, List<ApplicationInfo> myList) {
    super(context, NO_SELECTION);
    appInfoList = myList;
    ctx=context;
    mInflater =     (LayoutInflater)context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    pm = context.getPackageManager();

    positionArray = new ArrayList<Boolean>(myList.size());
    for(int i =0;i<myList.size();i++){
        positionArray.add(false);
    }
}
@Override
public int getCount() {
    // TODO Auto-generated method stub
    return appInfoList.size();
}

@Override
public View getView(final int position, View convertView, ViewGroup parent) {

    View row = convertView;
    Holder holder = null;

    if(row==null){
        row = mInflater.inflate(R.layout.testlayout, null); 
        //  visiblePosArray[position%visiblePosArray.length]=position;
        holder = new Holder();
        holder.appIcon = (ImageView)row.findViewById(R.id.imageView1);

        holder.ckbox =(CheckBox)row.findViewById(R.id.checkBox1);

        row.setTag(holder);
    } else {

        holder = (Holder) convertView.getTag();
    }

    holder.ckbox.setFocusable(false);
    holder.appIcon.setImageDrawable(appInfoList.get(position).loadIcon(pm));
    holder.ckbox.setChecked(positionArray.get(position));
    holder.ckbox.setText(appInfoList.get(position).loadLabel(pm));
    holder.ckbox.setOnCheckedChangeListener(new OnCheckedChangeListener() {

        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            if(isChecked ){
            System.out.println(position+"--- :)");
                positionArray.add(position, true);
            }else
                positionArray.add(position, false);
        }
    });

    return row;
}
static class Holder
{
    ImageView appIcon;
    CheckBox ckbox;

}

}

When I scroll up and down I could see random indices changed to true in my boolean Arraylist when in syso them.

Akhil
  • 13,888
  • 7
  • 35
  • 39

3 Answers3

41

When a listview recycles views , it recycles its present state as well as listeners attached to it. In my example, if the checkbox was checked and has a onCheckedChangeListener set, both will remain a part of recycled view based on position. So it is our responsibility to reset all states and remove previous listeners.

So when I was unchecking the recycled view, the onCheckedChange listener was getting executed. one line made the program work perfectly. The listener was removed by :

holder.ckbox.setOnCheckedChangeListener(null); 

Below is the working code of Adapter for people who may stumble upon this problem:

public class MyCustomAdapter extends ArrayAdapter<ApplicationInfo>  {

private List<ApplicationInfo> appInfoList;
private LayoutInflater mInflater;
private PackageManager pm;
ArrayList<Boolean> positionArray;
private Context ctx;
int[] visiblePosArray;
private volatile int positionCheck; 

public MyCustomAdapter(Context context, List<ApplicationInfo> myList) {
    super(context, NO_SELECTION);
    appInfoList = myList;
    ctx=context;
    mInflater =     (LayoutInflater)context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    pm = context.getPackageManager();

    positionArray = new ArrayList<Boolean>(myList.size());
    for(int i =0;i<myList.size();i++){
        positionArray.add(false);
    }
}
@Override
public int getCount() {
    // TODO Auto-generated method stub
    return appInfoList.size();
}

@Override
public View getView(final int position, View convertView, ViewGroup parent) {

    View row = convertView;
    Holder holder = null;

    if(row==null){
        row = mInflater.inflate(R.layout.testlayout, null); 
        //  visiblePosArray[position%visiblePosArray.length]=position;
        holder = new Holder();
        holder.appIcon = (ImageView)row.findViewById(R.id.imageView1);

        holder.ckbox =(CheckBox)row.findViewById(R.id.checkBox1);

        row.setTag(holder);
    } else {

        holder = (Holder) convertView.getTag();
    holder.ckbox.setOnCheckedChangeListener(null);

    }

    holder.ckbox.setFocusable(false);
    holder.appIcon.setImageDrawable(appInfoList.get(position).loadIcon(pm));
    holder.ckbox.setChecked(positionArray.get(position));
    holder.ckbox.setText(appInfoList.get(position).loadLabel(pm));
    holder.ckbox.setOnCheckedChangeListener(new OnCheckedChangeListener() {

        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            if(isChecked ){
            System.out.println(position+"--- :)");
                positionArray.set(position, true);

            }else
                positionArray.set(position, false);
        }
    });

    return row;
}
static class Holder
{
    ImageView appIcon;
    CheckBox ckbox;

}

}

Akhil
  • 13,888
  • 7
  • 35
  • 39
  • 2
    Keep in mind that by setting checked change listener to null every time consumes memory as you are always setting new Listener to every row. *(i.e 100 Listener of 100 rows every time you scroll)*, the metter of getting true position can be solved by the edit made to my answer. *(e.g setting the position as tag to the `CheckBox`)* – Adil Soomro Sep 12 '12 at 07:35
  • Hi, I've the same problem. May I know what does `Holder` or `holder` do? – Preeyah Sep 14 '12 at 03:40
  • a Holder class keeps a reference to the view so that we dont need to make expensive findViewById calls to get back view references. It makes things more efficient. – Akhil Sep 14 '12 at 09:19
  • was looking for the solution from two weeks.. this one worked brilliant thanks – Srikanth Pai Jun 21 '13 at 10:53
  • An example of creating a View holder can be found [here](http://stackoverflow.com/questions/14087495/android-implementing-viewholder). – Krøllebølle May 15 '15 at 12:56
  • save an if test: positionArray.set(position, isChecked); – Aderbal Nunes Oct 25 '17 at 18:35
9

You need to keep track of the check state, because ListView re-uses the Views. so the state for position one which was previously enabled/disabled may appear as is for position 7.

So what you need to do is keep the checked state in an array boolean or whatever you prefer.

Take a class level boolean [] checkedState; initialize it in constructor, according to your data array size, you can use ArrayList<Boolean> too for dynamic size.

set OnStateChangeListener to your CheckBoxes in getView(), whenever it is checked or un-checked, take the position and save it in the array of checkedState like this:

checkedState[position] = false;// or true accordingly

and when setting other data for View like TextView or ImageView for any specific position, set the checked state also accordingly like this:

holder.appIcon.setImageDrawable(appInfoList.get(position).loadIcon(pm));
holder.ckbox.setChecked(checkedState[position]);

A very good explanation and example:

Android custom image gallery with checkbox in grid to select multiple

Edit: Actually what is happening is, you position is getting buggy, to solve this add these lines:

holder.ckbox.setText(appInfoList.get(position).loadLabel(pm));
holder.ckbox.setTag(String.valueOf(position));   // to properly track the actual position
holder.ckbox.setOnCheckedChangeListener(new OnCheckedChangeListener() {
      @Override
      public void onCheckedChanged(CompoundButton v, boolean isChecked) {
            int pos = Integer.parseInt( v.getTag().toString()) ; //to take the actual position
            positionArray.add(pos, isChecked);  // we don't need to check whether it is true or false, however you can put if-else to debug the app.

      }
});
Adil Soomro
  • 37,609
  • 9
  • 103
  • 153
  • 1
    I have tried what is in your answer but it doesnt seem to working, random checkboxes sare still getting checked when i scroll down and up. I am posting my updated code of adapter. I wasnt able to open the link u gave as it is blocked at my workplace. Will see at home. – Akhil Jun 26 '12 at 08:42
  • The position is coming fine. Just random positions were added.I have found the solution and posting it. +1 for your efforts. – Akhil Jun 28 '12 at 08:03
  • My problem was I was doing lots of this stuff in the if(convertView == null) statement, which of course it should be outside - doh! Thanks for this answer. – Daniel Wilson Mar 12 '14 at 12:43
  • 1
    over 6 years later, but this was exactly my problem. This should probably be the accepted answer. Setting the listeners to null works, but is incredibly ineffecient and will slow down the app. – yitzih Jul 15 '18 at 07:53
6

A combination of these two approaches worked for me:

I have a boolean array on class level which I use to keep track of the value of checkboxes.

boolean [] checkedItems = new boolean[listItems.size()];

In getView() :

    public View getView(int position, View convertView, ViewGroup parent) {
        ViewHolder holder;

        if (convertView == null) {

            convertView = inflater.inflate(R.layout.menu_item_list_item,
                    parent, false);

            holder = new ViewHolder();

            holder.name = (TextView) convertView
                    .findViewById(R.id.menuItemLargeName);
            holder.mainItemCheckBox = (CheckBox) convertView
                    .findViewById(R.id.menuItemLargeCheckBox);

            convertView.setTag(holder);
        } else {
            holder = (ViewHolder) convertView.getTag();
            // remove the listener so that it does not get attached to other chechboxes. 
            holder.mainItemCheckBox.setOnCheckedChangeListener(null);
            //update the checkbox value from boolean array
            holder.mainItemCheckBox.setChecked(checkedItems[position]);
        }


        holder.name.setText(listItems.get(position).getName());

        holder.mainItemCheckBox
                .setOnCheckedChangeListener(onCheckedListener);
        holder.mainItemCheckBox
                .setTag(R.id.menuItemLargeCheckBox, position);

        return (convertView);
    }

In my OnCheckedChangeListener() : update the boolean array.

    OnCheckedChangeListener onCheckedListener = new OnCheckedChangeListener() {

    @Override
    public void onCheckedChanged(CompoundButton buttonView,
            boolean isChecked) {

        int position = (Integer) buttonView
                .getTag(R.id.menuItemLargeCheckBox);

        MenuItemObject menuItem = listItems.get(position);

        if (isChecked) {

            cartItems.add(menuItem);
            checkedItems[position] = true;

        } else {

            cartItems.remove(menuItem);
            checkedItems[position] = false;
        }


    }
};
Sayyam
  • 959
  • 10
  • 22