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 15 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 15 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

Removed calculation scroll thomb position and size based on RecyclerView.computeVerticalScrollRange() / computeVerticalScrollOffset(), because it depended on scroll position and recalculated while scrolling and made glitch.
This PR:

  • caches the thumb height and scroll range so the thumb keeps a stable size and scale while scrolling
  • tracks the thumb position using accumulated real scroll deltas (dy) instead of recalculating it from RecyclerView's estimated scroll offset on every layout
  • snaps the accumulated offset to the exact start/end when RecyclerView reports that the list is at an edge
  • resets cached scroll metrics when adapter data, list width, or viewport height changes
  • applies the same custom fast scroller to the tags dialog, replacing the native RecyclerView scrollbar there

I also add the changed scroller to fix tags glitch.

How Has This Been Tested?

Manual UI test.

Before:

telegram-cloud-document-2-5373303824473696425.mp4
telegram-cloud-document-2-5377809133498049085.mp4

After

telegram-cloud-document-2-5379845175629623475.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.

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