-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix scroll thomb length changing on cards /tags lists of different size rows on bug #21367
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
addf2a9
0bd67a1
a93b8ed
981be0e
f508abc
99e2579
82eba9e
13409ed
a54f144
4a38ab9
680e705
bbef5ae
070a523
16c681c
a34c549
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,10 +106,68 @@ class RecyclerFastScroller | |
|
|
||
| private var hideOverride = false | ||
| private var adapter: RecyclerView.Adapter<*>? = null | ||
|
|
||
| private var cachedHandleHeight = 0 | ||
| private var cachedHandleHeightItemCount = RecyclerView.NO_POSITION | ||
| private var cachedHandleHeightBarHeight = 0 | ||
| private var cachedVisibleItemCount = 0 | ||
| private var cachedVisibleItemCountItemCount = RecyclerView.NO_POSITION | ||
| private var cachedVisibleItemCountBarHeight = 0 | ||
|
|
||
| private fun resetCachedVisibleItemCount() { | ||
| cachedVisibleItemCount = 0 | ||
| cachedVisibleItemCountItemCount = RecyclerView.NO_POSITION | ||
| cachedVisibleItemCountBarHeight = 0 | ||
| } | ||
|
|
||
| private fun resetCachedHandleHeight() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. document it |
||
| cachedHandleHeight = 0 | ||
| cachedHandleHeightItemCount = RecyclerView.NO_POSITION | ||
| cachedHandleHeightBarHeight = 0 | ||
| } | ||
|
|
||
| /** | ||
| * Cached scroll metrics are tied to the current adapter contents and viewport size. | ||
| * They are reset when adapter data changes so the thumb can be recalculated for the | ||
| * new list, but stay stable during normal scrolling. | ||
| */ | ||
| private fun resetCachedScrollMetrics() { | ||
| resetCachedHandleHeight() | ||
| resetCachedVisibleItemCount() | ||
| } | ||
|
|
||
| private val adapterObserver: RecyclerView.AdapterDataObserver = | ||
| object : RecyclerView.AdapterDataObserver() { | ||
| override fun onChanged() { | ||
| super.onChanged() | ||
| resetCachedScrollMetrics() | ||
| requestLayout() | ||
| } | ||
|
|
||
| override fun onItemRangeChanged( | ||
| positionStart: Int, | ||
| itemCount: Int, | ||
| ) { | ||
| super.onItemRangeChanged(positionStart, itemCount) | ||
| resetCachedScrollMetrics() | ||
| requestLayout() | ||
| } | ||
|
|
||
| override fun onItemRangeInserted( | ||
| positionStart: Int, | ||
| itemCount: Int, | ||
| ) { | ||
| super.onItemRangeInserted(positionStart, itemCount) | ||
| resetCachedScrollMetrics() | ||
| requestLayout() | ||
| } | ||
|
|
||
| override fun onItemRangeRemoved( | ||
| positionStart: Int, | ||
| itemCount: Int, | ||
| ) { | ||
| super.onItemRangeRemoved(positionStart, itemCount) | ||
| resetCachedScrollMetrics() | ||
| requestLayout() | ||
| } | ||
| } | ||
|
|
@@ -253,6 +311,7 @@ class RecyclerFastScroller | |
| this.adapter?.unregisterAdapterDataObserver(adapterObserver) | ||
| adapter?.registerAdapterDataObserver(adapterObserver) | ||
| this.adapter = adapter | ||
| resetCachedScrollMetrics() | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -394,33 +453,115 @@ class RecyclerFastScroller | |
| bottom: Int, | ||
| ) { | ||
| super.onLayout(changed, left, top, right, bottom) | ||
| if (recyclerView == null) return | ||
|
|
||
| val scrollOffset = recyclerView!!.computeVerticalScrollOffset() + appBarLayoutOffset | ||
| val verticalScrollRange = ( | ||
| recyclerView!!.computeVerticalScrollRange() + | ||
| recyclerView!!.paddingBottom | ||
| ) | ||
| val recyclerView = recyclerView ?: return | ||
| val layoutManager = recyclerView.layoutManager as? LinearLayoutManager ?: return | ||
| val itemCount = recyclerView.adapter?.itemCount ?: return | ||
| if (itemCount == 0) return | ||
|
|
||
| val firstVisiblePosition = layoutManager.findFirstVisibleItemPosition() | ||
| val lastVisiblePosition = layoutManager.findLastVisibleItemPosition() | ||
| if (firstVisiblePosition == RecyclerView.NO_POSITION || lastVisiblePosition == RecyclerView.NO_POSITION) return | ||
|
|
||
| val barHeight = bar.height | ||
| val ratio = scrollOffset.toFloat() / (verticalScrollRange - barHeight) | ||
| val visibleItemCount = | ||
| getCachedVisibleItemCount( | ||
| barHeight, | ||
| itemCount, | ||
| firstVisiblePosition, | ||
| lastVisiblePosition, | ||
| ) | ||
| val calculatedHandleHeight = getCachedHandleHeight(barHeight, itemCount, visibleItemCount) | ||
|
|
||
| val ratio = | ||
| getScrollProportion( | ||
| recyclerView, | ||
| layoutManager, | ||
| itemCount, | ||
| visibleItemCount, | ||
| firstVisiblePosition, | ||
| ) | ||
|
|
||
| var calculatedHandleHeight = (barHeight.toFloat() / verticalScrollRange * barHeight).toInt() | ||
| if (calculatedHandleHeight < minScrollHandleHeight) { | ||
| calculatedHandleHeight = minScrollHandleHeight | ||
| hideOverride = false | ||
|
|
||
| val y = ratio * (barHeight - calculatedHandleHeight) | ||
| handle.layout(handle.left, y.toInt(), handle.right, y.toInt() + calculatedHandleHeight) | ||
| } | ||
|
|
||
| /** | ||
| * Calculates thumb position from adapter positions so the scroll progress remains | ||
| * stable when rows have different heights. The fractional offset within the first | ||
| * visible row keeps the thumb moving smoothly between adapter positions. | ||
| */ | ||
| private fun getScrollProportion( | ||
| recyclerView: RecyclerView, | ||
| layoutManager: LinearLayoutManager, | ||
| itemCount: Int, | ||
| visibleItemCount: Int, | ||
| firstVisiblePosition: Int, | ||
| ): Float { | ||
| if (!recyclerView.canScrollVertically(-1)) return 0f | ||
| if (!recyclerView.canScrollVertically(1)) return 1f | ||
|
|
||
| val firstVisibleView = layoutManager.findViewByPosition(firstVisiblePosition) ?: return 0f | ||
| val hiddenHeight = (recyclerView.paddingTop - firstVisibleView.top).coerceAtLeast(0) | ||
| val firstItemFraction = hiddenHeight.toFloat() / firstVisibleView.height.coerceAtLeast(1) | ||
|
|
||
| val scrollableItems = (itemCount - visibleItemCount).coerceAtLeast(1) | ||
| return ((firstVisiblePosition + firstItemFraction) / scrollableItems).coerceIn(0f, 1f) | ||
| } | ||
|
|
||
| /** | ||
| * Keeps the visible item count stable during scrolling. Recomputing it for every | ||
| * layout pass could change the thumb size as rows with different heights enter or | ||
| * leave the viewport. | ||
| */ | ||
| private fun getCachedVisibleItemCount( | ||
| barHeight: Int, | ||
| itemCount: Int, | ||
| firstVisiblePosition: Int, | ||
| lastVisiblePosition: Int, | ||
| ): Int { | ||
| if (itemCount != cachedVisibleItemCountItemCount || barHeight != cachedVisibleItemCountBarHeight) { | ||
| resetCachedVisibleItemCount() | ||
| cachedVisibleItemCountItemCount = itemCount | ||
| cachedVisibleItemCountBarHeight = barHeight | ||
| } | ||
|
|
||
| if (calculatedHandleHeight >= barHeight) { | ||
| translationX = hiddenTranslationX.toFloat() | ||
| hideOverride = true | ||
| return | ||
| if (cachedVisibleItemCount == 0) { | ||
| cachedVisibleItemCount = | ||
| (lastVisiblePosition - firstVisiblePosition + 1) | ||
| .coerceAtLeast(1) | ||
| .coerceAtMost(itemCount) | ||
| } | ||
|
|
||
| hideOverride = false | ||
| return cachedVisibleItemCount | ||
| } | ||
|
|
||
| val y = ratio * (barHeight - calculatedHandleHeight) | ||
| /** | ||
| * Calculates thumb height from the visible item count and total item count so the | ||
| * size remains stable while scrolling through rows with different heights. | ||
| */ | ||
| private fun getCachedHandleHeight( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. document it |
||
| barHeight: Int, | ||
| itemCount: Int, | ||
| visibleItemCount: Int, | ||
| ): Int { | ||
| if (itemCount != cachedHandleHeightItemCount || barHeight != cachedHandleHeightBarHeight) { | ||
| resetCachedHandleHeight() | ||
| cachedHandleHeightItemCount = itemCount | ||
| cachedHandleHeightBarHeight = barHeight | ||
| } | ||
|
|
||
| handle.layout(handle.left, y.toInt(), handle.right, y.toInt() + calculatedHandleHeight) | ||
| if (cachedHandleHeight == 0) { | ||
| cachedHandleHeight = | ||
| (barHeight.toFloat() * visibleItemCount / itemCount) | ||
| .toInt() | ||
| .coerceAtLeast(minScrollHandleHeight) | ||
| .coerceAtMost(barHeight) | ||
| } | ||
|
|
||
| return cachedHandleHeight | ||
| } | ||
|
|
||
| companion object { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,13 +64,25 @@ | |
| </LinearLayout> | ||
|
|
||
|
|
||
| <androidx.recyclerview.widget.RecyclerView android:id="@+id/tags_list" | ||
| android:scrollbars="vertical" | ||
| <RelativeLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_height="match_parent" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you explain why ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This wrapper needs to fill the available list area because it now contains both the RecyclerView and the overlay fast scroller. Keeping wrap_content would size the wrapper from its children instead of the available viewport, so the scroller track could end up with the wrong height. |
||
| android:layout_above="@id/options_group" | ||
| android:layout_below="@id/toolbar" | ||
| tools:listitem="@layout/item_tag" /> | ||
| android:layout_below="@id/toolbar"> | ||
|
|
||
| <androidx.recyclerview.widget.RecyclerView | ||
| android:id="@+id/tags_list" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:scrollbars="none" | ||
| tools:listitem="@layout/item_tag" /> | ||
|
|
||
| <com.ichi2.anki.ui.RecyclerFastScroller | ||
| android:id="@+id/tags_scroller" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="match_parent" | ||
| android:layout_alignParentEnd="true" /> | ||
| </RelativeLayout> | ||
|
|
||
| <androidx.coordinatorlayout.widget.CoordinatorLayout | ||
| android:layout_width="wrap_content" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is scroller ID is not found ? It may crash
worth adding a null safe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thank you. I made attachFastScroller null safe