-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add Auto-Scroll Support for RecyclerView in Multiselect View #17792
Conversation
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.
Outcome is good. Change seems much more complicated than it needs to be
da173e2
to
67dc5e5
Compare
AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt
Outdated
Show resolved
Hide resolved
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.
A few comments
b309024
to
640eaaa
Compare
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.
This should be done in the ViewModel (with tests)
It should be simpler than we have now:
- Update
onLongPress
to provide theView
as a parameter, so you don't need to look it up - Perform the operation in the ViewModel, likely passing in the
topOffset
- React to an event that the ViewModel emits
AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserMultiColumnAdapter.kt
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
ae2957c
to
62d6d23
Compare
This comment was marked as resolved.
This comment was marked as resolved.
AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserMultiColumnAdapter.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserMultiColumnAdapter.kt
Outdated
Show resolved
Hide resolved
Also to note: this doesn't handle the inverse: removing the selection is also visually unstable |
It works now , I fixed it. |
test2.mp4 |
d3708b1
to
6d7ab87
Compare
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.
Much simpler, great job! One minor nitpick
If a test or a @NeedsTest
could be added, it'd be great
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.
Almost there! Code's good, the positioning isn't quite right, I have a list of cards, some of which are multi-line questions, on a tablet
I get instances during testing where I can long press a row, deselect it, and the row under the cursor has changed
@david-allison if could you send me a test video or something , it will be very useful. |
AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt
Outdated
Show resolved
Hide resolved
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.
Great stuff, thanks so much!!!
@Scapesfear Please be patient and don't request reviews especially if the PR is not ready to be reviewed. You can see that the Github UI is telling you that there are conflicts, meaning code was added to the main branch while this branch was opened and now there are conflicts between the current application code on main and the changes that were done in this PR. Your job is to fix those conflicts and update the PR so it can be merged(also squash the commits into one commit). |
9470d43
to
e263db1
Compare
done, fixed the merge conflicts and squash-merged the commits in one. |
…tiselectview and vice versa
e263db1
to
8b41d6f
Compare
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.
LGTM. Thanks
Nice one! |
Purpose / Description
Fixes the missing auto-scroll functionality when switching to multi-select mode in RecyclerView, ensuring a smoother user experience.
Fixes
Approach
This change adds the missing auto-scroll functionality, ensuring that the RecyclerView scrolls smoothly when entering multi-select mode, improving usability.
How Has This Been Tested?
Samsung F23 5G
https://github.com/user-attachments/assets/9b2dc926-2fbc-4a4c-b4df-7c44a461fc6f
Checklist
Please, go through these checks before submitting the PR.