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

Added possibility to get view types from the adapter to add support for GridLayoutManager.SpanSizeLookup #16

Merged
merged 9 commits into from
Jan 10, 2020

Conversation

meruiden
Copy link
Contributor

@meruiden meruiden commented Jan 8, 2020

Example usage:

private val loadingIndicatorType = adapter.getItemViewTypeFromClass(LoadingIndicator::class.java)

override fun getSpanSize(position: Int): Int {
    val itemType = adapter.getItemViewType(position)

    return if(itemType == loadingIndicatorType) {
        spanCount
    } else {
        1
    }
}

See #15

@idanatz idanatz changed the base branch from master to develop January 9, 2020 15:35
Copy link
Owner

@idanatz idanatz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Please see my comments

@@ -11,7 +11,7 @@ internal class ViewHolderCreatorsStore {
holderCreators[clazz] = creator
}

fun getCreatorUniqueIndex(clazz: Class<Diffable>): Int {
Copy link
Owner

Choose a reason for hiding this comment

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

What is the reason for this change?
Each item type the adapter holds must implement Diffable, it doesn't matter if its an internal holder type or external class from the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I changed this is to allow calling the function like this:

getCreatorUniqueIndex(LoadingIndicator::class.java)

When the type is < Diffable > rather then <*> the compiler complains that LoadingIndicator is not a Diffable even though it is. This does work with instances but not when using LoadingIndicator::class.java. You could do a runtime check maybe and throw if it doesn't inherit from Diffable.

Copy link
Owner

Choose a reason for hiding this comment

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

ViewHolderCreatorsStore is an internal class of the library, it should not be used with classes other than DIffable.

The way i see it, the function on OneAdapter.kt can be called with type < * >, but inside InternalAdapter, the function getItemViewTypeFromClass will validate the class if it is of type DIffable (using the Validator class) and throw a new exception (that will be written in the Exceptions.kt)

class UnsupportedClassException(msg: String = "Class must implement Diffable interface") : Throwable(msg)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed the changes, let me know what u think :)

@@ -352,6 +352,10 @@ internal class InternalAdapter(val recyclerView: RecyclerView) : RecyclerView.Ad
}
//endregion

fun getItemViewTypeFromClass(clazz: Class<*>): Int? {
Copy link
Owner

Choose a reason for hiding this comment

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

change to Class < Diffable > for the reason written in
ViewHolderCreatorsStore.kt

Copy link
Owner

Choose a reason for hiding this comment

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

and place it below
fun getItemViewType

@@ -79,6 +79,14 @@ class OneAdapter(recyclerView: RecyclerView) {
}
}

fun getItemViewType(position: Int): Int {
Copy link
Owner

Choose a reason for hiding this comment

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

change to Class < Diffable > for the reason written in
ViewHolderCreatorsStore.kt

Copy link
Owner

Choose a reason for hiding this comment

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

and place these functions at the end of the file

idanatz and others added 3 commits January 9, 2020 17:43
@idanatz
Copy link
Owner

idanatz commented Jan 9, 2020

Looks good.

a couple of final thins:

  1. Before addressing the next bullets, pull from develop branch, I've made local changes before your PR and pushed them a few hours ago, after that address the issues below.

  2. I know I wrote it but on seconds thought, change
    class UnsupportedClassException(msg: String = "Class must implement Diffable interface") : Throwable(msg)
    to
    class UnsupportedClassException(msg: String = "") : Throwable(msg)
    and move the msg "Class must implement Diffable interface" to the throw site.
    it's more understandable when reading the code to see the actual message.

  3. Why is
    fun getItemViewTypeFromClass(clazz: Class<*>): Int?
    returning Int? ? getCreatorUniqueIndex returns Int, it will never be nullable.

  4. add this documentation below
    /**

    • Retrieves the view type of an item with a given class.
    • Note that this class must implement the Diffable interface and the adapter must contain items of that class.
    • @throws UnsupportedClassException if the class does not implement the Diffable interface.
      /
      fun getItemViewTypeFromClass(clazz: Class<
      >): Int?

Note that there are some validations that need to be implemented now that the getItemViewType and getCreatorUniqueIndex are publicly accessible, but I will add them after this PR will be merged.

@meruiden
Copy link
Contributor Author

Why is
fun getItemViewTypeFromClass(clazz: Class<*>): Int?
returning Int? ? getCreatorUniqueIndex returns Int, it will never be nullable.

Whoops that shouldn't be nullable indeed. I'll remove this

…e changes (some small refactoring) requested from the maintainer
@meruiden
Copy link
Contributor Author

Looks good.

a couple of final thins:

  1. Before addressing the next bullets, pull from develop branch, I've made local changes before your PR and pushed them a few hours ago, after that address the issues below.

  2. I know I wrote it but on seconds thought, change
    class UnsupportedClassException(msg: String = "Class must implement Diffable interface") : Throwable(msg)
    to
    class UnsupportedClassException(msg: String = "") : Throwable(msg)
    and move the msg "Class must implement Diffable interface" to the throw site.
    it's more understandable when reading the code to see the actual message.

  3. Why is
    fun getItemViewTypeFromClass(clazz: Class<*>): Int?
    returning Int? ? getCreatorUniqueIndex returns Int, it will never be nullable.

  4. add this documentation below
    /**

    • Retrieves the view type of an item with a given class.
    • Note that this class must implement the Diffable interface and the adapter must contain items of that class.
    • @throws UnsupportedClassException if the class does not implement the Diffable interface.
      /
      fun getItemViewTypeFromClass(clazz: Class<
      >): Int?

Note that there are some validations that need to be implemented now that the getItemViewType and getCreatorUniqueIndex are publicly accessible, but I will add them after this PR will be merged.

I pulled the latest changes from develop and changed the stuff you requested, merged all in to master and pushed.

@idanatz idanatz merged commit a6e58c1 into idanatz:develop Jan 10, 2020
@idanatz
Copy link
Owner

idanatz commented Jan 10, 2020

merged your PR, thanks!
will be released as a minor version in a few days after il check everything is good.

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

2 participants