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

Realm RecyclerView adapter #14

Merged
merged 2 commits into from May 12, 2016
Merged

Realm RecyclerView adapter #14

merged 2 commits into from May 12, 2016

Conversation

thesurix
Copy link
Contributor

@thesurix thesurix commented May 8, 2016

Fixes #972
Moved to new adapters project.

Old PR: realm/realm-java#2548

@@ -23,7 +23,7 @@ configurations {

dependencies {
compile 'com.android.support:appcompat-v7:23.3.0'

compile 'com.android.support:recyclerview-v7:23.3.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you append a new line after this?

Copy link
Contributor

Choose a reason for hiding this comment

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

require instead of compile? I guess we don't have strong dependency on recyclerview version?

Copy link
Contributor

Choose a reason for hiding this comment

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

provided isn't supported for AAR's yet. So we either need to find a work-around for this before releasing it or just document how to avoid including it:

compile('io.realm:android-adapters:1.0.1') {
  exclude group: 'com.android.support', module: 'recyclerview-v7'
}

In either case I think we should do that outside the scope of this PR

* Can return null if provided Realm instance by {@link OrderedRealmCollection} is closed.
*
* @param index index of the item.
* @return the item at the specified position, null if adapter data is not valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

null-> {@code null}

@beeender
Copy link
Contributor

besides some minor comments, 👍

}

private void addListener(OrderedRealmCollection<T> data) {
BaseRealm realm = getRealm(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since RecyclerView has proper lifecycle methods you can actually use the logic I accidentially added to RealmBaseAdapter here: https://github.com/realm/realm-android-adapters/blob/e395cb11b36f483dfa35f0b9d150f3d3c94e67ff/adapters/src/main/java/io/realm/RealmBaseAdapter.java

Because with a proper remove method you can safely use better listener on RealmResults instead of registering on Realm. That way we avoid updating the RecyclerView on any changes to Realm when using RealmResults.

@cmelchior
Copy link
Contributor

I think the methods for addListener/removeListener can be improved to behave better when working with RealmResults, but apart from that it looks good.

@thesurix
Copy link
Contributor Author

@beeender @cmelchior @dalinaum PR is ready :)

@cmelchior
Copy link
Contributor

Looks good to me 👍
Thanks for all the work @thesurix

@beeender
Copy link
Contributor

👍

@beeender beeender merged commit 278f98b into realm:master May 12, 2016
@beeender beeender removed the S:Review label May 12, 2016
@cmelchior cmelchior mentioned this pull request May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants