Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FastAdapter flickers after loading from LiveData even though the data stays same #737

Closed
5 tasks done
afarber opened this issue Oct 30, 2018 · 13 comments
Closed
5 tasks done

Comments

@afarber
Copy link

afarber commented Oct 30, 2018

About this issue

Good evening, for my question I have prepared a simple test case at Github -

screenshot

My example app downloads a JSON array containing top 30 players in a game using okhttp and stores them into SQLite Room. In the fragment I observe the corresponding LiveData<List<TopEntity>> object and update the FastAdapter instance:

public class TopFragment extends Fragment {
    private final ItemAdapter<TopItem> mItemAdapter = new ItemAdapter<>();
    private final FastAdapter<TopItem> mFastAdapter = FastAdapter.with(mItemAdapter);

    private TopViewModel mViewModel;
    private ProgressBar mProgressBar;
    private RecyclerView mRecyclerView;

    @Override
    public View onCreateView(@NonNull LayoutInflater inflater,
                             ViewGroup container,
                             Bundle savedInstanceState) {

        mViewModel = ViewModelProviders.of(this).get(TopViewModel.class);
        mViewModel.getTops().observe(this, tops -> {
            mItemAdapter.clear();
            for (TopEntity top: tops) {
                TopItem item = new TopItem(top);
                mItemAdapter.add(item);
            }
        });

        View v = inflater.inflate(R.layout.top_fragment, container, false);
        mProgressBar = v.findViewById(R.id.progressBar);
        mRecyclerView = v.findViewById(R.id.recyclerView);
        mRecyclerView.setLayoutManager(new LinearLayoutManager(getActivity()));
        mRecyclerView.setAdapter(mFastAdapter);

        fetchJsonData();

        return v;
    }

My problem is: the recycler view flickers once - every time the data is downloaded.

Even though the top 30 does not change often, i.e. the data stays same.

When inspecting the example app in the FastAdapter library, I do not see any flickering there.

What am I missing please, probably something minor?

Details

  • [3.2.8] Used library version
  • [28.0.0] Used support library version
  • [3.2.1] Used gradle build tools version
  • [3.2.1] Android Studio version
  • [1.1.1] Room and arch lifecycle

Checklist

@FabianTerhorst
Copy link
Contributor

Hi,

you are clearing all items and adding them again each time your LiveData changes. https://github.com/afarber/android-questions/blob/master/TopPlayers/app/src/main/java/de/afarber/topplayers/TopFragment.java#L87
To fix that you could for example use DiffUtils to only notify changes on items that actually got changes. Take a look at the DiffUtils sample. https://github.com/mikepenz/FastAdapter/blob/develop/app/src/main/java/com/mikepenz/fastadapter/app/DiffUtilActivity.java#L137
(You don't have to use rxjava for it, as long as it runs outside the main thread it will be fine)

@afarber
Copy link
Author

afarber commented Oct 30, 2018

Fabian, thanks for your hint, but could you add few general words on how to use DiffUtils?

Just few words on what they produce and where to put their output then?

Especially unclear to me - where would I get the old data, while in the onChanged method of the LiveData?

@afarber
Copy link
Author

afarber commented Oct 30, 2018

Nevermind DiffUtils is google's class, I need to read up on the docs. Thanks

@afarber afarber closed this as completed Oct 30, 2018
@afarber afarber reopened this Oct 31, 2018
@afarber
Copy link
Author

afarber commented Oct 31, 2018

Good evening again! I am confused, how to use DiffUtil with FastAdapter -

I have added a callback class:

public class DiffCallback extends DiffUtil.Callback {
    private final List<TopItem> mOldList;
    private final List<TopItem> mNewList;

    public DiffCallback(List<TopItem> oldStudentList, List<TopItem> newStudentList) {
        this.mOldList = oldStudentList;
        this.mNewList = newStudentList;
    }

    @Override
    public int getOldListSize() {
        return mOldList.size();
    }

    @Override
    public int getNewListSize() {
        return mNewList.size();
    }

    @Override
    public boolean areItemsTheSame(int oldItemPosition, int newItemPosition) {
        TopItem oldItem = mOldList.get(oldItemPosition);
        TopItem newItem = mNewList.get(newItemPosition);

        return oldItem.uid == newItem.uid;
    }

    @Override
    public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) {
        TopItem oldItem = mOldList.get(oldItemPosition);
        TopItem newItem = mNewList.get(newItemPosition);

        return oldItem.elo == newItem.elo &&
                oldItem.given.equals(newItem.given) &&
                //oldItem.photo != null && oldItem.photo.equals(newItem.photo) &&
                oldItem.avg_time != null && oldItem.avg_time.equals(newItem.avg_time) &&
                oldItem.avg_score == newItem.avg_score;
    }

    @Nullable
    @Override
    public Object getChangePayload(int oldItemPosition, int newItemPosition) {
        return super.getChangePayload(oldItemPosition, newItemPosition);
    }
}

And then I am trying to use it in the onChanged method instead of just calling mItemAdapter.clear() but the RecyclerView stays empty even though the tops has elements:

    mViewModel = ViewModelProviders.of(this).get(TopViewModel.class);
    mViewModel.getTops().observe(this, tops -> {
        List<TopItem> oldList = mItemAdapter.getAdapterItems();
        List<TopItem> newList = new ArrayList<>();
        for (TopEntity top: tops) {
            TopItem item = new TopItem(top);
            newList.add(item);
        }

        DiffCallback diffCallback = new DiffCallback(oldList, newList);
        DiffUtil.DiffResult diffResult = DiffUtil.calculateDiff(diffCallback);

        mItemAdapter.getAdapterItems().clear();
        mItemAdapter.getAdapterItems().addAll(newList);
        diffResult.dispatchUpdatesTo(mFastAdapter);
    });

Please give me some hints.

Also, I don't want to use FastAdapterDiffUtil, because I don't have any expandable items.

@afarber
Copy link
Author

afarber commented Nov 1, 2018

Still, I have tried using FastAdapterDiffUtil too -

        mViewModel = ViewModelProviders.of(this).get(TopViewModel.class);
        mViewModel.getTops().observe(this, tops -> {
            List<TopItem> newList = new ArrayList<>();
            for (TopEntity top: tops) {
                TopItem item = new TopItem(top);
                newList.add(item);
            }

            DiffUtil.DiffResult diffResult = FastAdapterDiffUtil.calculateDiff(mItemAdapter, newList);
            FastAdapterDiffUtil.set(mItemAdapter, diffResult);
        });

and then the RecyclerView is updated correctly, but the flickering is still there.

@mikepenz
Copy link
Owner

mikepenz commented Nov 1, 2018

Your items hashcode, and equals method have a proper implementation?

The default is by identifier, if you base on the AbstractItem, if you use the interface alone you should implement these.

@afarber
Copy link
Author

afarber commented Nov 2, 2018

Hello Mike, thanks for your reply! I have added the 2 methods to my TopItem class (representing player objects with unique integer uid):

    @Override
    public boolean equals(Object other) {
        if (other instanceof TopItem) {
            return this.uid == ((TopItem) other).uid;
        }

        return false;
    }

    @Override
    public int hashCode() {
        return uid;
    }

But unfortunately the one-time flickering is still there when I try to use FastAdapterDiffUtil in my TopFragment:

            List<TopItem> newList = new ArrayList<>();
            for (TopEntity top: tops) {
                TopItem item = new TopItem(top);
                newList.add(item);
            }

            DiffUtil.DiffResult diffResult = FastAdapterDiffUtil.calculateDiff(mItemAdapter, newList);
            FastAdapterDiffUtil.set(mItemAdapter, diffResult);

I have also posted my question on Stackoverflow and few people suggest, that FastAdapter might have issues with DiffUtil or FastAdapterDiffUtil, but I think it is maybe just an issue with missing documentation...

Best regards
Alex

@mikepenz
Copy link
Owner

mikepenz commented Nov 2, 2018

and the new items definitely have the same identifier?

it definitely should work without a flicker as it does in the sample, and various apps I use it in.

@afarber
Copy link
Author

afarber commented Nov 2, 2018

Mike, yes - if you look at my TopItem - it uses:

    @Override
    public int getType() {
        return R.id.top_item_id;
    }

There are no other items in my simple test case. Or do you mean something else, maybe I misunderstand you?

@mikepenz
Copy link
Owner

mikepenz commented Nov 2, 2018

@afarber
Copy link
Author

afarber commented Nov 2, 2018

I have added

    @Override
    public long getIdentifier() {
        return uid;
    }

because each player uid is unique in my case (and have removed equals and hashCode methods again) - and now there is no flickering now, thank you!

So FastAdapterDiffUtil is like DiffUtil, but does not need a callback (however needs a unique id)?

Can Google's DiffUtil be used with FastAdapter too?

@mikepenz
Copy link
Owner

mikepenz commented Nov 2, 2018

Well the FastAdapterDiffUtil is just a convenience class to simplify usage directly.

You can use the DiffUtil directly too but you will have to take care of more stuff on your own.

Btw. the identifier should always be unique for elements. by default the IdDistributor will give it a unique id every time a new element is created. which will make the element different than a previous one with the same data, because it simply can't know that.

So it is always important if you can get a proper identifier that you also define your logic or set proper identifiers :)

@mikepenz mikepenz closed this as completed Nov 2, 2018
@mikepenz
Copy link
Owner

mikepenz commented Nov 2, 2018

Just one more additional detail. the "identifier" is nothing specific to the FastAdapter it is a general definition by the RecyclerView.Adapter and should already be implemented in an according way there too (without any lib).

The FastAdapter only offers a more "easy" handling for it for most super simple usecases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants