vivchar/RendererRecyclerViewAdapter

ViewState not working as expected

kAliert opened this issue · 10 comments

Hi!

First of all i want to thank you for this library!

I think i encountered a problem with the viewstate. It doesn´t work like expected and the behaviour can be reproduced with your given example app.

  • run the app
  • open view state
  • scroll the first item horizontally
  • scroll down
    => some elements have the same view state as the first item

This behaviour first occured in an example i made:

  • i created a recycler view with switches
  • after i opened the fragment i changed the view state of one item
  • than i scrolled the view
    => some elements were checked

The strange thing: when i first scroll the view and change afterwards the state of the items this bug doesn´t occure. So i downloaded your example to check if i made something wrong. But also in your example i was able to reproduce this unexpected behaviour.

Maybe you can try to reproduce this behaviour. Please let me know if im doing something wrong or you have a solution for this problem.

Hi, thank you! What version do you use? 2.6.0 or 2.5.1? Could you try 2.5.1?

Upgraded to version 2.6.0, but the problem still occurs.

Here my sample code:

Fragment

public class RecyclerFragment extends Fragment
{
    private View rootView;

    @Override
    public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState)
    {
        rootView = inflater.inflate(R.layout.fragment_resource, container, false);

        initRecyclerView();

        return rootView;
    }

    private void initRecyclerView()
    {
        RendererRecyclerViewAdapter adapter = new RendererRecyclerViewAdapter();
        initViewBinder(adapter);
        initRecyclerView(adapter);

        adapter.setItems(createItems());
        adapter.notifyDataSetChanged();
    }

    private void initViewBinder(RendererRecyclerViewAdapter adapter)
    {
        adapter.registerRenderer(new ViewBinder<>(R.layout.item_switch, SwitchItem.class, createSwitchItem(), new SwitchItemStateProvider()));
    }

    private void initRecyclerView(RendererRecyclerViewAdapter adapter)
    {
        RecyclerView recyclerView = rootView.findViewById(R.id.recyclerView);
        RecyclerView.LayoutManager layoutManager = new LinearLayoutManager(getContext());
        recyclerView.setLayoutManager(layoutManager);
        recyclerView.setItemAnimator(new DefaultItemAnimator());
        recyclerView.setAdapter(adapter);
    }

    private List<ViewModel> createItems()
    {
        List<ViewModel> viewModels = new ArrayList<>();

        for(int i = 1; i <= 1000; i++)
        {
            viewModels.add(new SwitchItem(i));
        }

        return viewModels;
    }

    public class SwitchItemState implements ViewState<ViewHolder>
    {
        private final boolean isChecked;

        SwitchItemState(ViewHolder holder)
        {
            Switch view = holder.getViewFinder().find(R.id.aSwitch);
            isChecked = view.isChecked();
        }

        @Override
        public void restore(@NonNull final ViewHolder holder)
        {
            holder.getViewFinder().setChecked(R.id.aSwitch, isChecked);
        }
    }

    public class SwitchItemStateProvider implements ViewStateProvider<SwitchItem, ViewHolder>
    {
        @Override
        public ViewState createViewState(@NonNull final ViewHolder holder)
        {
            return new SwitchItemState(holder);
        }

        @Override
        public int createViewStateID(@NonNull final SwitchItem model)
        {
            return model.getID();
        }
    }

    private BinderAdapter<SwitchItem> createSwitchItem()
    {
        return new BinderAdapter<SwitchItem>()
        {
            @Override
            public void bindView(@NonNull SwitchItem model, @NonNull ViewFinder finder)
            {
                finder.find(R.id.aSwitch);
            }
        };
    }
}

Model class

public class SwitchItem implements ViewModel
{
    private int id;
    private boolean isChecked;

    public SwitchItem(int id)
    {
        this.id = id;
    }

    public boolean isChecked()
    {
        return isChecked;
    }

    public void setChecked(boolean checked)
    {
        isChecked = checked;
    }

    public int getID()
    {
        return id;
    }
}

Layout (item_switch.xml)

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:orientation="horizontal">

    <Switch
        android:id="@+id/aSwitch"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"/>
</LinearLayout>

If you pass the checked state via a model class, then it is not "state" of an item. It is a part of your business logic. You should use your model always then.
It is not correct to use both flavor(model data and view state) to save the checked value.
I hope that you will understand me =)

Are you able to reproduce this if temporary remove the isChecked field value from your model?

ok, I reproduced it. I will investigate this.

Ok. I think i understood what you said* In my example code i didn´t used the isChecked field from the model class so i only used the view state. But glad to hear you are able to reproduce this behaviour.

Many thanks for your support!

  • just for clarification: if my model saves the state, i dont need to use the ViewState at all. I just "save" the value in my model and restore the value in the BindAdapter.

For example:

  private BinderAdapter<SwitchItem> createSwitchItem()
   {
       return new BinderAdapter<SwitchItem>()
       {
           @Override
           public void bindView(@NonNull final SwitchItem model, @NonNull ViewFinder finder)
           {
               final Switch aSwitch = finder.find(R.id.aSwitch);
               aSwitch.setChecked(model.isChecked());

               aSwitch.setOnClickListener(new View.OnClickListener()
               {
                   @Override
                   public void onClick(View view)
                   {
                       model.setChecked(aSwitch.isChecked());
                   }
               });
           }
       };
   }

Would this be the right way to do it?

I found a problem, I will think how to fix it.
As a temporary workaround please add the default value for your the Switch inside the bindView method, please see below:

private BinderAdapter<SwitchItem> createSwitchItem()
{
    return new BinderAdapter<SwitchItem>()
    {
        @Override
        public void bindView(@NonNull SwitchItem model, @NonNull ViewFinder finder)
        {
            finder.setChecked(R.id.aSwitch, false); /* set default value */
        }
    };
}

Would this be the right way to do it?

No, it is not right way. Because if you get a new list and update RecyclerView, then you lost this state.
You should pass this value to your BusinesLogicClass like as Fragment, Activity, Presenter or Controller:

adapter.registerRenderer(new ViewBinder<>(
                 R.layout. item_switch, 
                 SwitchItem.class,
		(model, finder, payloads) -> finder
				.setChecked(R.id. aSwitch, false) /* default state */
				.setOnClickListener(R.id. aSwitch, v -> {
					mPresenter.onCheckedItem(model, ((Switch)v).isChecked()); /* send event to your logic logic */
				}),
		new SwitchItemStateProvider()
));
public class Presenter {
        private final SparseArray<Boolean> mCheckedStates = new SparseArray<>();

        public void onItemChecked(SwitchItem item, boolean checked) {
                mCheckedStates.put(item.getID, checked);
                updateList();
        }
      
       private void updateList() {
               mView.updateItems(createItems());
       }

        private List<ViewModel> createItems() {
                List<ViewModel> viewModels = new ArrayList<>();
                for(int i = 1; i <= 1000; i++) {
                        boolean checked = mCheckedStates.get(i) != null && mCheckedStates.get(i);
                        viewModels.add(new SwitchItem(i, checked));
                }
                return viewModels;
        }
}

FYI: how to use presenter you can see there

Ok. Thanks for the explanation. I really appreciate your work!

Pls. What is this (model, finder, payloads) -> finder

@kAliert Hi, I fixed this issue
library fix:
3d59b18#diff-2d7438d26f1f6b07e864df9e62f720e8L7
use sample:
3d59b18#diff-83885bcfecb0019d99b1ea344164aa49R20

It will be released in v3.0.0 I suppose.