gregkorossy / Android-Support-Preference-V7-Fix

Android androidx.preference support library has some issues, this lib tries to fix them.
https://discord.gg/87NVsSK
Apache License 2.0
497 stars 46 forks source link

Upgrade to support library v24.0.0 #25

Closed gregkorossy closed 8 years ago

gregkorossy commented 8 years ago

Working on the problem, but it seems like it's a bit more work than replacing some numbers in the gradle build files. Looks like Google messed up something with the preferences' paddings on API levels below 21...

Bad padding on API 17:

device-2016-06-16-174349


Good padding on API 21:

device-2016-06-16-174555

gregkorossy commented 8 years ago

Reported the issue to Google: https://code.google.com/p/android/issues/detail?id=213297

yccheok commented 8 years ago

Just about to report this issues. Thank you for reporting this. Is there any "hack" can be done, like what you had done on 23 support library?

gregkorossy commented 8 years ago

Tried a lot of "nice" things (i.e. not rewriting layouts / classes), nothing worked and I don't really get the reason why.

Here's a piece from the v17 preference_category_material.xml layout file:

android:paddingStart="?android:attr/listPreferredItemPaddingStart"
android:paddingEnd="?android:attr/listPreferredItemPaddingEnd" />

And this solution should work (it's in v17 styles.xml; the test app uses PreferenceFixTheme.Light.NoActionBar which inherits its attributes from PreferenceFixTheme.Light):

<style name="PreferenceFixTheme.Light" parent="@style/Theme.AppCompat.Light">
    <item name="preferenceTheme">@style/PreferenceThemeOverlay.v14.Material</item>
    <item name="colorAccent">@color/preference_accent</item>
    <item name="dialogTheme">@style/PreferenceFixTheme.Light.Dialog</item>
    <item name="alertDialogTheme">@style/PreferenceFixTheme.Light.Dialog.Alert</item>
    <!-- Padding fix -->
    <item name="android:listPreferredItemPaddingStart">16dp</item>
    <item name="android:listPreferredItemPaddingEnd">16dp</item>
</style>

The funny thing is that in the preference_material.xml file (notice the difference), there's also this attribute:

android:minHeight="?android:attr/listPreferredItemHeightSmall"

If I override it by defining its value in the same PreferenceFixTheme.Light style, it works:

<item name="android:listPreferredItemHeightSmall">100dp</item>

So I guess the padding is really just preferred and gets overridden by either a direct piece of code or the measurements done during layout.

yccheok commented 8 years ago

Thanks, In your previous workaround, I already include

<!-- Padding fix -->
<item name="android:listPreferredItemPaddingStart">16dp</item>
<item name="android:listPreferredItemPaddingEnd">16dp</item>

in v17. It works fine all time till recently when we switch to support library 24.

I also observe same problem in device with v16. But, android:listPreferredItemPaddingStart and android:listPreferredItemPaddingEnd are not found in v16.

Hope Google can deliver a fix on this. Without library 24, we can't take advantage of Firebase.

gregkorossy commented 8 years ago

It was just an example, all the API levels below 21 are affected by it. API 7-14 uses android:listPreferredItemPaddingLeft and android:listPreferredItemPaddingRight, but the problem persists.

gregkorossy commented 8 years ago

I did some experiments with it on API 14 by copying the preference_material.xml to the project and hard-coding the padding into it and still no success, so I'm guessing that it is being overridden in a class. Moreover, I set the height of the layout to the right padding's attribute and it worked which confirms my theory...

gregkorossy commented 8 years ago

Interesting, I've already had the PreferenceCategory class because it still messes up the accent color without it, so I decided to get the padding of the title view in onBindViewHolder(...). It returns plain 0 for all of the padding values. It pretty much seems like the inflater just doesn't care about the paddings anymore and sets(?) them to 0. The category itself has a hard-coded 16dip paddingTop, which is also ignored, while the bottom margin (also set to 16 dip) is there normally. If I set the paddings from code, they take effect and show just how normally they would.

gregkorossy commented 8 years ago

I found a way to hack in the left and right paddings, but the top one is still messed up.

gregkorossy commented 8 years ago

Okay, I found the bug (it is more of an Android View-handling thing though).

In PreferenceGroupAdapter's onCreateViewHolder(...) after inflating the layout, the paddings are good. Right after this, there's a setBackgroundDrawable(...) call which resets them all to zero. It is a known behavior as per Romain Guy:

The reason why setting an image resets the padding is because 9-patch images can encode padding.

The solution would be the following:

  1. Save the paddings after inflating the layout.
  2. Set the background drawable.
  3. Set the saved paddings on the layout.

Since the Android devs decided to make a lot of things private in this class (including an inner static class), it'd be extremely difficult to fix it only by overriding a few things...

gregkorossy commented 8 years ago

Seems that with reflection I've been able to fix this ridiculous bug. I'll release the v24.0.0.0-beta version soon...

gregkorossy commented 8 years ago

The fix has been released. Use compile 'com.takisoft.fix:preference-v7:24.0.0.0-beta' in your projects.

yccheok commented 8 years ago

Thanks for your work. Going to try it out tonight.

yccheok commented 8 years ago

Hi @Gericop

Thanks for the hard work.

However, I realize, it is not work perfectly with inner preference yet.

Here's the screnshot

0

1

Here's the XML for the preference screen - https://gist.github.com/yccheok/4172d782249757b1153cee9edd4c566b

Thanks.

yccheok commented 8 years ago

Sorry. The inner preference bug I mentioned is not valid anymore. Please ignore it.

https://github.com/Gericop/Android-Support-Preference-V7-Fix/issues/27