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

Use android.R.attr.listDivider as the default Drawable #45

Closed
fondesa opened this issue Nov 9, 2019 · 20 comments
Closed

Use android.R.attr.listDivider as the default Drawable #45

fondesa opened this issue Nov 9, 2019 · 20 comments
Labels
enhancement Introduces a new feature, a behavior change or a performance change

Comments

@fondesa
Copy link
Owner

fondesa commented Nov 9, 2019

Related to #41

@fondesa fondesa added this to the 4.x milestone Nov 9, 2019
@fondesa fondesa added the enhancement Introduces a new feature, a behavior change or a performance change label Nov 10, 2019
@fondesa fondesa removed this from the 4.x milestone May 18, 2020
@fondesa
Copy link
Owner Author

fondesa commented May 18, 2020

Just released the version 3.2.0 which uses android:listDivider by default to get the divider's Drawable.

@fondesa fondesa closed this as completed May 18, 2020
@AndroidDeveloperLB
Copy link

And in code, like this:

RecyclerViewDivider.with(context).addTo(this) 

Meaning no need for this:

fun RecyclerView.setDefaultDividers() {
    val context = this.context
    context.obtainStyledAttributes(intArrayOf(android.R.attr.listDivider)).use { typedArray ->
        typedArray.getDrawable(0)
                ?.let { RecyclerViewDivider.with(context).drawable(it).hideLastDivider().build().addTo(this) }
    }
}

?

@fondesa
Copy link
Owner Author

fondesa commented May 18, 2020

The RecyclerViewDivider API is deprecated in favour of the new DividerBuilder API.

You can replace this whole function:

fun RecyclerView.setDefaultDividers() {
    val context = this.context
    context.obtainStyledAttributes(intArrayOf(android.R.attr.listDivider)).use { typedArray ->
        typedArray.getDrawable(0)
                ?.let { RecyclerViewDivider.with(context).drawable(it).hideLastDivider().build().addTo(this) }
    }
}

With just:

recyclerView.addDivider()

Because:

  • now the default divider reads the value of android.R.attr.listDivider
  • now the default divider hides the last divider by default

@AndroidDeveloperLB
Copy link

AndroidDeveloperLB commented May 19, 2020

Seems not working for me on dark theme for some reason, while working fine on normal theme.

Can you test, or should I try on POC ?

At least using the old method I've made seems to still work as before.

@fondesa
Copy link
Owner Author

fondesa commented May 19, 2020

I'll try it soon, thanks!

@AndroidDeveloperLB
Copy link

Please let me know about it.

@fondesa
Copy link
Owner Author

fondesa commented May 19, 2020

@AndroidDeveloperLB I reproduced it and I found the line of code which causes it.
It's the following one:

DrawableCompat.setTintList(wrappedDrawable, null)

This line is necessary to reset the tint of the single divider to avoid the tint color to propagate to the other dividers.

Now I have to understand why it changes the Drawable on night theme.

@fondesa fondesa reopened this May 19, 2020
@fondesa
Copy link
Owner Author

fondesa commented May 19, 2020

It's very similar to this issue on Android: https://issuetracker.google.com/issues/141678225
The problem is that the default divider color on API > 21 is #ffdadce0 on light theme and #85ffffff on dark theme.

The problem is tinting a GradientDrawable with a color which isn't fully opaque, that's why it fails only on dark theme.

I'll replace the call to setTintList with a ColorFilter if I can't succeed to make it working in the standard way.

@AndroidDeveloperLB
Copy link

OK for now I will use the previous method. It still works fine.

@AndroidDeveloperLB
Copy link

Is there a shorter workaround than using what I wrote, BTW?

@fondesa
Copy link
Owner Author

fondesa commented May 20, 2020

You can probably change the value of android:listDivider to a plain color with those values that I wrote above (the one in the Android material version). But I'll release a fix pretty soon so I don't think it's worth

@AndroidDeveloperLB
Copy link

I need in code. Anyway, please let me know when you fix this.

@fondesa
Copy link
Owner Author

fondesa commented May 20, 2020

Yes, I'll notify you for any update

@AndroidDeveloperLB
Copy link

Thank you. If you with, this is my app that uses this repository.

@fondesa
Copy link
Owner Author

fondesa commented May 20, 2020

@AndroidDeveloperLB Released the version 3.2.1 containing the fix. Can you let me know if addDivider() works for you now?

@AndroidDeveloperLB
Copy link

I think now it's indeed fixed and working well.
Thank you.

BTW, why add extension function?
This isn't something that's common for libraries. Usually it's reserved for the main project itself, not for libraries, unless it's a very core function (kotlin basic classes, for example).

@fondesa
Copy link
Owner Author

fondesa commented May 20, 2020

Nice, to hear.

I added the extension function to create a divider using the shortest syntax and made it more accessible for Kotlin users of this library since creating the divider is the core API of this library (other APIs like RecyclerViewDividerLog which aren't related to the core one, are still expressed without extension functions). I did it also following bigger repositories written in Kotlin like Coil.

I honestly think it's more a matter of personal taste than something objective in general.

(I'm closing this issue since the bug was fixed)

@fondesa fondesa closed this as completed May 20, 2020
@AndroidDeveloperLB
Copy link

OK your choice. Are there more extension functions on the library now?

@fondesa
Copy link
Owner Author

fondesa commented May 20, 2020

Since the version 3.2.0 (the one with new APIs) there are these 4 extensions, all 4 on RecyclerView:

  • addDivider()
  • dividerBuilder()
  • addStaggeredDivider()
  • staggeredDividerBuilder()

@AndroidDeveloperLB
Copy link

ok thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Introduces a new feature, a behavior change or a performance change
Projects
None yet
Development

No branches or pull requests

2 participants