Skip to content

Fix scroll thomb length changing on cards /tags lists of different size rows on bug#21367

Open
amoniaka-knabino wants to merge 18 commits into
ankidroid:mainfrom
amoniaka-knabino:fix-scroll-thomb
Open

Fix scroll thomb length changing on cards /tags lists of different size rows on bug#21367
amoniaka-knabino wants to merge 18 commits into
ankidroid:mainfrom
amoniaka-knabino:fix-scroll-thomb

Conversation

@amoniaka-knabino

@amoniaka-knabino amoniaka-knabino commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Purpose / Description

Fix scroll UI bug: before length changed while scrolling list of rows of different length and looked glitchy, I want to make it smooth for better view.

Fixes

Approach

RecyclerView.computeVerticalScrollRange() and computeVerticalScrollOffset() are estimates based on currently visible rows. With variable-height rows, these estimates can change while scrolling, which causes the scrollbar thumb to resize or jump.

This PR:

  • caches the initial scroll range and thumb height so their scale remains stable while scrolling
  • initializes the scroll offset from RecyclerView once, then tracks it using the actual pixel deltas (dy)
  • invalidates cached metrics when the adapter structure, list width, or viewport height changes, while ignoring content-only row updates that do not change the list structure
  • calibrates the cached range after observing a complete normal scroll from the real top to the real bottom
  • approaches the end of an uncalibrated range smoothly and animates the final correction when normal scrolling reaches the real bottom
  • handles thumb dragging separately from normal scrolling, keeping the thumb under the user's finger and reserving space for the final visible rows
  • avoids an unnecessary full row refresh when opening the note editor from single-select mode, preventing cached scroll metrics from being reset
  • applies the same custom fast scroller to the tags dialog instead of its native RecyclerView scrollbar
  • makes attachFastScroller() a safe no-op when the requested scroller is not present in the layout

How Has This Been Tested?

Manual UI test.

Before:

Current main performance.

telegram-cloud-document-2-5397669861138671532.mp4

After

Current branch performance.

FILE.2026-07-19.13.30.59-compressed.mp4

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@amoniaka-knabino
amoniaka-knabino marked this pull request as ready for review July 12, 2026 17:45
@amoniaka-knabino amoniaka-knabino changed the title Fix scroll thomb Fix scroll thomb length changing on lists of different size rows bug Jul 12, 2026
@amoniaka-knabino amoniaka-knabino changed the title Fix scroll thomb length changing on lists of different size rows bug Fix scroll thomb length changing on lists of different size rows on cards bug Jul 12, 2026
@amoniaka-knabino amoniaka-knabino changed the title Fix scroll thomb length changing on lists of different size rows on cards bug Fix scroll thomb length changing on cards /tags lists of different size rows on bug Jul 12, 2026
criticalAY

This comment was marked as outdated.

@sanjaysargam sanjaysargam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @amoniaka-knabino
Consider squashing the documentation and refactor commits for cleaner history

private var cachedHandleHeightItemCount = RecyclerView.NO_POSITION
private var cachedHandleHeightBarHeight = 0

private fun resetCachedHandleHeight() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

document it

handle.layout(handle.left, y.toInt(), handle.right, y.toInt() + calculatedHandleHeight)
}

private fun getCachedHandleHeight(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

document it

<RelativeLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_height="match_parent"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you explain why ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.


tagsArrayAdapter = TagsArrayAdapter(tags) { binding.root.showMaxTagSelectedNotice(tags) }
binding.tagsList.adapter = tagsArrayAdapter
binding.tagsList.attachFastScroller(R.id.tags_scroller)

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

@criticalAY criticalAY added the Needs Author Reply Waiting for a reply from the original author label Jul 13, 2026
@amoniaka-knabino

amoniaka-knabino commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

The item-index approach is the right call here, and it now lines up with the drag handler too. I put a small follow-up patch(please it test) on top of yours: it collapses the caching into one keyed value, brings back the hide-when-the-list-fits behaviour, and fixes two edge cases (Card Browser attaches the scroller before its adapter, so the data observer never registers and the thumb size goes stale after a Truncate toggle, and an empty search leaves a full-height thumb behind). I also pulled the math into two small pure functions with unit tests. Feel free to take whatever is useful.

@criticalAY Thank you for the patch!

I tested it, because of smaller scroll thomb size, position looks more glitchy after the patch (please look at videos below, especially the cards video). How do you think, it's OK or we need to change it somehow?

telegram-cloud-document-2-5380060933311732952.mp4
telegram-cloud-document-2-5380060933311732950.mp4

criticalAY

This comment was marked as resolved.

@amoniaka-knabino

Copy link
Copy Markdown
Contributor Author

@criticalAY I tried your second patch as it is, but glitch still was there. After I added accumulatedScrollOffset and it gets better. Position is smooth everywhere except start: scroll thomb "jumps" to start position when you scroll from end to start, but it's not really irritating, I think.

after patch 2:

telegram-cloud-document-2-5379845175629623469.mp4

after accumulatedScrollOffset: (scroll jump at 0:22)

telegram-cloud-document-2-5379845175629623475.mp4

@ZornHadNoChoice

Copy link
Copy Markdown
Collaborator

The scrolling in this last recording looks really nice, but:

  • The snapping at the ends isn't great.
  • The thumb height changes at around 0:07 when opening the note editor.

If it's not too much to ask, consider also testing the effects of this code on #20994. You can use the same deck I shared in #19480 (comment) to test both issues (make sure to import with scheduling information to get the flags; they help see the other issue).

@criticalAY

Copy link
Copy Markdown
Contributor

@amoniaka-knabino Can this be rebased? Or if you are not sure how then I can help here, just let me know

@amoniaka-knabino

Copy link
Copy Markdown
Contributor Author

If it's not too much to ask, consider also testing the effects of this code on #20994. You can use the same deck I shared in #19480 (comment) to test both issues (make sure to import with scheduling information to get the flags; they help see the other issue).

Thank you for noticing! I will try to fix it.

As I see, current version works OK on this collections, except scrolling in the end. At least, I can't see glitch and size changing while opening the editor on such small scroll thumb size.

telegram-cloud-document-2-5382096975443304703-compressed.mp4
telegram-cloud-document-2-5382096975443304705-compressed.mp4
telegram-cloud-document-2-5382096975443304708.mp4
telegram-cloud-document-2-5382096975443304707.mp4

@ZornHadNoChoice

Copy link
Copy Markdown
Collaborator

The problem at the ends is worse with this bigger deck. Also, for the other issue, you need to grab the scrollbar thumb and slowly drag it.

@amoniaka-knabino

amoniaka-knabino commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@ZornHadNoChoice I added scroll calibration and it become smooth.
Also I removed requestLayout() in onItemRangeChanged(), so opening card menu don't trigger thumb size change.

2026-07-15.16.11.14-compressed-under-10mb.mp4

UPD: I also tested grab&drag, it still have jump in the end. I'll see what can I do with it.

telegram-cloud-document-2-5384348775256988729.1.mp4

@ZornHadNoChoice

Copy link
Copy Markdown
Collaborator

It's definitely much smoother, but the thumb moves slower at the lower end.

@amoniaka-knabino

Copy link
Copy Markdown
Contributor Author

It's definitely much smoother, but the thumb moves slower at the lower end.

Thumb moves slower at the lower end because of calibration. It's hard to be fixed, because in the beginning of the scroll we don't know exactly height of all rows, so our scroll size and speed is not really precise. When we are close to end but still not at the last point, we recalculate scroll speed to let scroll thumb move until we really scroll to the last element.

@amoniaka-knabino

amoniaka-knabino commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ZornHadNoChoice @criticalAY @sanjaysargam !

I fixed things that you wrote to me in the comments and now I am satisfied with current scroll thumb performance. I hope this version of fix is final and can be merged.

You can see current version performance in the description of PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Author Reply Waiting for a reply from the original author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants