search

abarisain / dmix

A modern MPD Client for Android.
Apache License 2.0
588 stars 205 forks source link

PLaying queue fastscroll broken on 4.4 #349

Closed jcnoir closed 10 years ago

jcnoir commented 10 years ago

Same issue than #294 but needs an other fix (always displaying the fastscroll would make some playlist item right button difficult to reach)

abarisain commented 10 years ago

I just checked, and Google offsets the list everywhere to account for their ***\ fastscroll implementations. I can look into how it would look like for the play queue (maybe only pad the list and get the fastscroll if more than 30-40-whatever items)

jcnoir commented 10 years ago

Would be nice. I often use a 1000 item playlist to do some testing. Without fastscroll it's really not easy to use ...

abarisain commented 10 years ago

This only seems to work on 4.4, or at least is broken on 4.3, don't really know why.

EDIT : https://github.com/android/platform_frameworks_base/commit/26bb253b4001b7d4d876656b0c6bd11b6aab4131 This commit, near line 385 adds a relayout when setScrollbarStyle is called. This is missing from older android versions, and is a bug, so KitKat did not only add regressions ! They actually tried to fix a lot of the FastScroll behaviour.

The final code is not exactly the same (https://github.com/android/platform_frameworks_base/blob/d4bdd6befa4e3cc29bedaaea3678c1075e0b7d24/core/java/android/widget/AbsListView.java#L1349), but still fixes the issue. This method was not overriden in previous Android versions, so I need to find a way to work around this for API < 19.

abarisain commented 10 years ago

For the record, turns out I was wrong, but not so wrong. There is a padding recalculation bug that has been fixed in KitKat, but I'm not sure that the commits I linked are the one that fixes them.

We need to be careful not to swap these lines, so I added a lot of comments to make sure we don't.
I think that everybody will like the new FastScrolls.

EDIT : Turns out this breaks KitKat. I need to make a stupid API check.
A version independant workaround is to call setPadding, but I can't get the right padding without it taking the scrollbar offset into account. I would have to override the one set in the XML file (which is not a problem since there is no padding right on the concerned lists) but this would not be very maintainable.