androidandroid-recyclerviewxml-drawablesetbackground

Drawable Recyclerview background change slow, error doing too much work on the main thread


I am trying to set the state of item selected on a recyclerview. UI do this by changing the background of the single cell framelayout one drawable xml to another.

In the recyclerview onBindViewHolder I make the change like this:

 @Override
    public void onBindViewHolder(final DeviceAlarmTonesHolder holder, final int position) {
        final DeviceAlarmTone alarmTone = alarmTones.get(position);
        // set click listener
        holder.itemView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {

                holder.alarmTonesButtonBackground.setBackgroundResource(R.drawable.layout_bg_selected);
                itemClickListener.onItemClicked(holder, alarmTone, position, oldPostion);

                // Refresh the ui for the previous button
                if (oldPostion != -1){
                    notifyItemChanged(oldPostion);
                }
                oldPostion = position;
            }
        });


        String alarmToneString = alarmTones.get(position).getNotificationTitle();
        holder.alarmTonesNameTextView.setText(alarmToneString);

    }

Basically in the onclick I am changing background for the current item and then calling notifyitemchanged on the old item to revert the background back to unselected.

The backgrounds that are being changed are xml drawables like this:

Unselected Background:

<?xml version="1.0" encoding="UTF-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android">
    <solid android:color="#212121"/>
    <corners android:radius="10dip"/>
    <padding android:left="0dip" android:top="0dip" android:right="0dip" android:bottom="0dip" />
</shape>

Selected Backgound:

<?xml version="1.0" encoding="UTF-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android">
    <solid android:color="#0091EA"/>
    <corners android:radius="10dip"/>
    <padding android:left="0dip" android:top="0dip" android:right="0dip" android:bottom="0dip" />
</shape>

Thanks in advance for your help.

UPDATE here is my full adapter:

public class DeviceAlarmToneAdapter extends RecyclerView.Adapter<DeviceAlarmTonesHolder>{

    Context context;
    ArrayList<DeviceAlarmTone> alarmTones;
    DeviceAlarmToneClickListener itemClickListener;

    int oldPostion = -1;

    public DeviceAlarmToneAdapter(Context context, ArrayList<DeviceAlarmTone> alarmTones, DeviceAlarmToneClickListener itemClickListener) {
        this.context = context;
        this.alarmTones = alarmTones;
        this.itemClickListener = itemClickListener;
    }

    @Override
    public DeviceAlarmTonesHolder onCreateViewHolder(ViewGroup parent, int viewType) {

        View v = LayoutInflater.from(parent.getContext()).inflate(R.layout.singlecell_devicealarmtoneslist, parent, false);
        DeviceAlarmTonesHolder holder = new DeviceAlarmTonesHolder(v, context);
        return holder;
    }

    @Override
    public void onBindViewHolder(final DeviceAlarmTonesHolder holder, final int position) {
        final DeviceAlarmTone alarmTone = alarmTones.get(position);
        // set click listener
        holder.itemView.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {

                holder.alarmTonesButtonBackgroundUnselected.setVisibility(View.INVISIBLE);
                holder.alarmTonesButtonBackgroundSelected.setVisibility(View.VISIBLE);
                itemClickListener.onItemClicked(holder, alarmTone, position, oldPostion);

                // Refresh the ui for the previous button
                if (oldPostion != -1){
                    notifyItemChanged(oldPostion);
                }
                oldPostion = position;
            }
        });


        String alarmToneString = alarmTones.get(position).getNotificationTitle();
        holder.alarmTonesNameTextView.setText(alarmToneString);

    }

    @Override
    public int getItemCount() {
        return alarmTones.size();
    }

}

Solution

  • public class DeviceAlarmToneAdapter extends RecyclerView.Adapter<DeviceAlarmTonesHolder>{
    
    Context context;
    ArrayList<DeviceAlarmTone> alarmTones;
    DeviceAlarmToneClickListener itemClickListener;
    
    int oldPostion = -1;
    
    public DeviceAlarmToneAdapter(Context context, ArrayList<DeviceAlarmTone> alarmTones, DeviceAlarmToneClickListener itemClickListener) {
        this.context = context;
        this.alarmTones = alarmTones;
        this.itemClickListener = itemClickListener;
    }
    
    @Override
    public DeviceAlarmTonesHolder onCreateViewHolder(ViewGroup parent, int viewType) {
    
        View v = LayoutInflater.from(parent.getContext()).inflate(R.layout.singlecell_devicealarmtoneslist, parent, false);
        DeviceAlarmTonesHolder holder = new DeviceAlarmTonesHolder(v, context);
        return holder;
    }
    
    @Override
    public void onBindViewHolder(final DeviceAlarmTonesHolder holder, final int position) {
        final DeviceAlarmTone alarmTone = alarmTones.get(position);
    
    
        String alarmToneString = alarmTones.get(position).getNotificationTitle();
        holder.alarmTonesNameTextView.setText(alarmToneString);
    
    }
    
    @Override
    public int getItemCount() {
        return alarmTones.size();
    }
    
    
    public class DeviceAlarmTonesHolder extends RecyclerView.ViewHolder implements View.OnClickListener{
    
        // view fields are here
    
        public DeviceAlarmTonesHolder(View itemView){
            super(itemView);
    
            // find view by ids by using itemview.findViewById(id)
    
            itemView.setOnClickListener(this);
        }
    
        @Override
        public void onClick(View view) {
            // you shouldnt use two views as background objects to show unselected or selected background. Use selector drawables xml instead... and call view.setSelected(true or false);
            // Since your modifyng view object itself you dont need to call notifyItemChanged...
            // with this tecnique you have only one View.OnClickListener object which belongs to your holder that have only one instance. This will lower your memory usage and fluid scrolls.
    
            if (itemClickListener != null){
                int position = getAdapterPosition();
                alarmTone = alarmTones.get(position);
    
                itemClickListener.onAlarmClicked(alarmTone);
            }
        }
    }
    

    }

    I just bypassed to update previous background to check if problem occurs from there.

    You need to lower processes in onBindHolder and avoid creating objects inside it. Because each time you scroll you dont want create more objects instead using old one.

    // you shouldnt use two views as background objects to show unselected or selected background. Use selector drawables xml instead... and call view.setSelected(true or false); Also optimize your backgrounds. Make them less sized. There are lots of online tools to do it for you. On the other hand if they are pictures like jpegs and pngs put them into drawable-nodpi folder to exclude resizing.

    Since your modifyng view object itself you dont need to call notifyItemChanged...

    With this tecnique you have only one View.OnClickListener object which belongs to your holder that have only one instance.

    This will lower your memory usage and fluid scrolls.

    If problem occurs you may need to use Single Choice RecyclerView libraries or finding another solution.

    Good luck there