CODEWITHSUNDEEP
androidlistview
user3501779
user3501779

Reputation: 169

Attempting to remove item from ListView always removes the first item

I have a custom adapter and a list which I add items to dynamically on button click. Each list row has a remove button (or image). That's my custom adapter class:

package com.ameer.easycooking;

import android.content.Context;
import android.media.Image;
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 android.widget.Toast;

import java.util.List;

/**
 * Created by ameer on 12/10/2016.
 */
public class IngredientAdapter extends ArrayAdapter<String> {
    private List<String> ingredientList;
    private Context context;

    public IngredientAdapter(List<String> ingredientList, Context context) {
        super(context, R.layout.ingredient_list_item, ingredientList);
        this.ingredientList = ingredientList;
        this.context = context;
    }

    public static class IngredientHolder {
        private TextView name;
        private ImageView remove;

        public IngredientHolder(TextView name, ImageView remove) {
            this.name = name;
            this.remove = remove;
        }

        public TextView getName() {
            return name;
        }

        public ImageView getRemove() {
            return remove;
        }
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View v = convertView;

        IngredientHolder holder;

        if (convertView == null) {
            LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
            v = inflater.inflate(R.layout.ingredient_list_item, null);

            holder = new IngredientHolder((TextView) v.findViewById(R.id.ingredientName), (ImageView) v.findViewById(R.id.remove));

            holder.getRemove().setTag(position);

            holder.getRemove().setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                    int index = (int) v.getTag();
                    Toast.makeText(context, ingredientList.get(index) + " removed", Toast.LENGTH_SHORT).show();
                    ingredientList.remove(index);
                    notifyDataSetChanged();
                }
            });

            v.setTag(holder);
        } else {
            holder = (IngredientHolder) v.getTag();
        }

        String ingredient = ingredientList.get(position);
        holder.name.setText(ingredient);

        return v;
    }
}

My problem is that no matter how many items I add or which one I choose to remove, it's always removing the first row in the ListView rather than the clicked one.

Upvotes: 0

Views: 111

Answers (3)

Viren Bhandari
Viren Bhandari

Reputation: 1

Better way you can make the position passed in getView() final public View getView(final int position, View convertView, ViewGroup parent) and use that position inside click listener.

Upvotes: 0

yrazlik
yrazlik

Reputation: 10777

You should not set your click listener inside your if statement, because you cannot know when a converted view is going to be used, and the clicked position may be wrong. So, just move the click listener out of the if statement and remove the holder.getRemove().setTag(position) statement:

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    View v = convertView;

    IngredientHolder holder;

    if (convertView == null) {
        LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        v = inflater.inflate(R.layout.ingredient_list_item, null);
        holder = new IngredientHolder((TextView) v.findViewById(R.id.ingredientName), (ImageView) v.findViewById(R.id.remove));
        v.setTag(holder);
    } else {
        holder = (IngredientHolder) v.getTag();
    }

    String ingredient = ingredientList.get(position);
    holder.name.setText(ingredient);

    holder.getRemove().setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            int index = (int) v.getTag();
            Toast.makeText(context, ingredientList.get(index) + " removed", Toast.LENGTH_SHORT).show();
            ingredientList.remove(index);
            notifyDataSetChanged();
        }
    });

    return v;
}

EDIT: Or as Blackbelt says, you could just simply move the holder.getRemove().setTag(position); statement out of if block.

Upvotes: 1

Carnal
Carnal

Reputation: 22064

You will only hit convertView == null once, that's why you have this problem. You should set holder.getRemove().setTag(position); after your if/else statements.

Upvotes: 1

Related Questions