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

MvxRecyclerViewAdapter: Support worker threads #2477

Merged
merged 2 commits into from
Dec 19, 2017

Conversation

nmilcoff
Copy link
Contributor

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Code improvement

⤵️ What is the current behavior?

When a user tries to update a bound ItemsSource collection from a worker thread, MvxRecyclerViewAdapter just 💥💥.

🆕 What is the new behavior (if this is a feature change)?

MvxRecyclerViewAdapter handles collection changes from worker threads.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

Run TestProjects/Android-Support. ExampleRecyclerViewModel updates a collection from background.

📝 Links to relevant issues/docs

Closes #2144

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated (docs style guide)
  • Nuspec files were updated (when applicable)
  • Rebased onto current develop

@nmilcoff nmilcoff added p/android-support Android Support Packages platform Code improvement labels Dec 19, 2017
@nmilcoff
Copy link
Contributor Author

@softlion can you maybe review this PR? 🙂

@@ -241,7 +243,15 @@ protected virtual void SetItemsSource(IEnumerable value)

protected virtual void OnItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
NotifyDataSetChanged(e);
if (Looper.MainLooper == Looper.MyLooper())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a Mvx method that checks the thread and runs it on main if necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is MvxAndroidMainThreadDispatcher. However this class can't inherit from MvxMainThreadDispatchingObject. So this is a good solution.

@softlion

This comment was marked as abuse.

@@ -241,7 +243,15 @@ protected virtual void SetItemsSource(IEnumerable value)

protected virtual void OnItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
NotifyDataSetChanged(e);
if (Looper.MainLooper == Looper.MyLooper())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is MvxAndroidMainThreadDispatcher. However this class can't inherit from MvxMainThreadDispatchingObject. So this is a good solution.

@martijn00 martijn00 merged commit bb7fa90 into MvvmCross:develop Dec 19, 2017
@nmilcoff nmilcoff added this to the 5.6.3 milestone Dec 20, 2017
@nmilcoff nmilcoff deleted the recycler-adapter branch December 20, 2017 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p/android-support Android Support Packages platform
Development

Successfully merging this pull request may close these issues.

MvxRecyclerAdapter NotifyDataSetChanged not called on mainthread
4 participants