2

While implementing the viewHolder pattern in an extended ArrayAdapter, and implementing an onClickListener on a Linear Layout, the background color of this LinearLayout gets changed but for other random rows as well!

Here is my code (Have stripped down the viewHolder to only include the error):

public class CustomFeedsAdapter extends ArrayAdapter<CustomCommunityQuestion>{

    private static final String TAG = "CustomFeedsAdapter";
    private Context context;
    private ArrayList<CustomCommunityQuestion> customCommunityQuestions;

    public CustomFeedsAdapter(Context context, ArrayList<CustomCommunityQuestion> customCommunityQuestions) {
        super(context, -1, customCommunityQuestions);
        this.context = context;
        this.customCommunityQuestions = customCommunityQuestions;
    }

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

        // reuse views
        final ViewHolder holder;
        if (convertView == null) {
            holder = new ViewHolder();
            LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
            rowView = inflater.inflate(R.layout.cards_feeds_row, null, false);
            holder.buttonFollow = (LinearLayout) rowView.findViewById(R.id.follow_layout);

            rowView.setTag(holder);
        } else {
            holder = (ViewHolder) rowView.getTag();
        }

        final CustomCommunityQuestion currentQuestion = customCommunityQuestions.get(position);

        // Set onClick Listeners
        holder.buttonFollow.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                VolleySingleton volleySingleton = VolleySingleton.getInstance(context);
                StringRequest followJsonObjectRequest = new StringRequest(Request.Method.GET,
                        "http://private-e9644-popxoandroid.apiary-mock.com/quest_follow/1/0",
                        new Response.Listener<String>() {
                            @Override
                            public void onResponse(String response) {
                                try {
                                    Log.d(TAG, response);
                                    JSONObject responseObject = new JSONObject(response);
                                    String message = responseObject.getString("message");
                                    Integer success = responseObject.getInt("success");
                                    Toast.makeText(context,message,Toast.LENGTH_SHORT).show();
                                    if(success!=0){
                                        holder.buttonFollow.setBackgroundColor(R.color.holo_blue_bright); // Here the button I wish to update doesn't get the color, but instead some weird random row's button get's changed.
                                        holder.buttonFollow.setEnabled(false);
                                    }
                                } catch (Exception e) {
                                    e.printStackTrace();
                                }
                            }
                        },
                        new Response.ErrorListener() {

                            @Override
                            public void onErrorResponse(VolleyError error) {
                                Toast.makeText(context, "There was some error! You were unable to follow", Toast.LENGTH_SHORT).show();
                                error.printStackTrace();
                                Log.e(TAG,error.getMessage());
                            }
                        });
                volleySingleton.addToRequestQueue(followJsonObjectRequest);
            }
        });

        return rowView;
    }

    private class ViewHolder {
        private LinearLayout buttonFollow;
     }
}
Saket Jain
  • 1,352
  • 2
  • 12
  • 25
  • Hey Saket, You can refer to this http://stackoverflow.com/questions/35622883/settext-changes-at-multiple-times-in-textview. Some one added a comment Which I believe is a right answer. – Sandeep Bhandari Feb 25 '16 at 09:44
  • 2
    Seriously I never used POJO classes, What I prefer is in your onclick listener download whatever you want and then dont update the UI there rather update the dataset of your adapter and reload the list using notifyDataSetChanged(); and in getView handle the UI thats all. Believe me it works am using the same solution in my project :) – Sandeep Bhandari Feb 25 '16 at 09:46
  • @SandeepBhandari: Hmmm.. Sort of makes sense, but I would rather keep the UI functionality separate from the core class if possible. Let's see if some answer can help me out with that. – Saket Jain Feb 25 '16 at 09:53

2 Answers2

3

What you are doing

The currently visible rows are at let's say position 0,1,2. You are updating a particular ViewHolder that is already been set as the tag of a particular (for use of a better word) - view. This view is then re-used later on when you call holder = (ViewHolder)rowview.getTag(). Let's assume for the purpose of this explanation that you are updating view at position 1.

What happens when you scroll a listView

As you scroll a listview, the old view is re-used by getting the holder from the tag. This means that the view that you changed earlier is now being re-used.

What you need to do instead

So when you need to update your view, first of all update a relevant variable. For example:

Create an attribute in your CustomCommunityQuestion class and implement a getter and setter like this:

boolean isBlue;

public void setIsBlue(booelan isblue){
     this.isBlue = isblue;
}

public boolean isBlue(){
    return isBlue;
} 

Now in your button onClickListener, you can set the variable isBlue to yes, and call notifyDataSetChanged(). Moreover, handle the attribute isBlue like so:

Just check the isBlue state and set the color accordingly

if(customCommunityQuestions.get(position).isBlue()){
       holder.buttonFollow.setBackgroundColor(R.color.holo_blue_bright);
}else{
     holder.buttonFollow.setBackgroundColor(R.color.black);
}

Hope it helps!

Saket Jain
  • 1,352
  • 2
  • 12
  • 25
Mustanser Iqbal
  • 5,017
  • 4
  • 18
  • 36
  • Thanks! I got it. I was implementing the solution as you wrote, but your approach is better. Thank you so much! – Saket Jain Feb 25 '16 at 10:12
1

The problem is the recycling of the listitems:

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

    // reuse views
    final ViewHolder holder;
    if (convertView == null) {
        ...
    } else {
        holder = (ViewHolder) rowView.getTag();
        //!!!
        // here the view item is recycled but
        // holder.buttonFollow is not reset to its initial state
    }

Solution: your CustomFeedsAdapter must remeber success-stat for every item

k3b
  • 14,517
  • 7
  • 53
  • 85
  • Thanks k3b. I think I am getting what you are saying. However, could you please tell if there is a best-practice approach or something for remembering the success-stat? I think that would be really helpful. :) – Saket Jain Feb 25 '16 at 09:50