improvement(browse): re-implement scrolling #17839
Draft
+488
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added in f0e4ceb or flags This was reverted, but the padding remained
Cause: 16620 (da72986) changed from a ListView to a RecyclerView and scrollbars were not included NOTE: commit message is incorrect due to a bad fixup ==== Fix I could not get `fastScrollEnabled` to work, and only a visual indicator was insufficient `recycler-fast-scroll` [Apache 2.0] was unmaintained but seemed like a reasonable solution. So it was vendored, refactored and used https://github.com/pluscubed/recycler-fast-scroll/tree/master Fixes 17786 ===== I tried `com.simplecityapps.recyclerview_fastscroll.views` * Rendering was slow * But it extended RecyclerView, rather than needing another View
david-allison
commented
Jan 17, 2025
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * https://github.com/pluscubed/recycler-fast-scroll/blob/3de76812553a77bfd25d3aea0a0af4d96516c3e3/library/src/main/java/com/pluscubed/recyclerfastscroll/RecyclerFastScroller.java |
There was a problem hiding this comment.
Note: this is vendored code, unsure how you want to review it
david-allison
added
Needs Author Reply
This comment was marked as outdated.
This comment was marked as outdated.
david-allison
added
the
Needs a new dev
david-allison
commented
Jan 20, 2025
Comment on lines
+432
to
+438
| fun updateRvScroll(dY: Int) { | ||
| try { | ||
| recyclerView?.scrollBy(0, dY) | ||
| } catch (t: Throwable) { | ||
| Timber.w(t) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not going to have capacity to take on this issue this week.
The issue with this PR is in this line. With 60k cards, this renders most of them if you scroll from top to bottom in order to calculate what dy means.
We need to move from a 'scroll' based scroller to a 'position' based scroller
Or, find another library if this is unacceptable.
I like this vendored library:
- Design is identical to ListView
- Fairly simple code given the domain
But if we can't make it fast, it needs to go
david-allison
removed
the
Needs Author Reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose / Description
Cause: #16620 (da72986) changed from a ListView to a RecyclerView and scrollbars were not included
NOTE: commit above message is incorrect due to a bad fixup
Fixes
Approach
recycler-fast-scroll[Apache 2.0] was unmaintained but seemed like a reasonable solution.So it was vendored, refactored and used
https://github.com/pluscubed/recycler-fast-scroll/tree/master
How Has This Been Tested?
S21. Colors are the same in night and light mode. Performance appears to be GREAT, but we can probably improve further.
Learning (optional, can help others)
I could not get
fastScrollEnabledto work on a RecyclerView, and the visual indicator without allowing a user to scroll quickly was insufficientI tried
com.simplecityapps.recyclerview_fastscroll.viewsChecklist