Selection not updated to reflect items removed from adapter. [138932671]

Fixed

Bug

[AOSP] assigned

Status Update

No update yet.

Description

ss...@gmail.com

created issue #1

Aug 5, 2019 05:55PM

Artifact used: androidx.recyclerview:recyclerview-selection:1.0.0
Version used: N/A
Theme used: N/A
Devices/Android versions reproduced on: Emulator / Android 9.0 (Google APIs)

public class MyAdapter extends RecyclerView.Adapter<String> {
private List<String> mData;

public MyAdapter () {
mData = new ArrayList<>();
mData.add("Test1");
mData.add("Test2");
}

public void removeFirst() {
mData.remove(0);
notifyItemRemoved(0);
}
}

Suppose that the first item is selected. Removing it calling MyAdapter.removeFirst() will also call notifyItemRemoved() but the selection tracker observe are not issued (none of its callbacks are invoked). In fact, the selection tracker keeps the selection state saying that there is a selection.

Comments

vi...@google.com <vi...@google.com> #2Sep 27, 2019 08:12AM

Assigned to vi...@google.com.

I believe that this is following on from issue 123186596, and as mentioned there, is a regression from version 1.0.0.

In the linked thread it is suggested that withOnItemActivatedListener can be used to correctly handle click events on the view. I've had a look through the sample project here and it seems like this will work for most cases. It also makes sense why this is required.

For my use-case I would like a reference to the view in the ViewHolder for shared element transitions - setting on onClickListener in the ViewHolder made this easy. I'll have to work around by finding the view holder with the motion event for now.

I think it should be clearer in the version release notes that this behaviour change is present.

ss...@gmail.com <ss...@gmail.com> #3Sep 27, 2019 11:30AM

Thanks for the report. The behavior relating to onClick handlers should be consistent from 1.0 to 1.1, so, subject to verification, seems like a bug to me that will need to be addressed as part of the 1.1 release.

ss...@gmail.com <ss...@gmail.com> #4Oct 3, 2019 02:17PM

Thanks for verifying the first issue.
But your response sounds like you're focusing on that one and consider the second point a non-issue, just because it was already present in 1.0?
Please note that it does not only affect custom OnClickListeners, but also standard widgets like checkboxes. If you click through items (read: click on the items' checkboxes) slowly after activating selection mode, you will only select/unselect further items. If you click faster, selection will skip some items and instead toggle the checkbox. And as I said, even without interactive items, selection just feels utterly sluggish when it skips user clicks. The whole user experience is far, far away from the "good old" list view.

Offtopic: Those new androidx/jetpack libraries don't feel like they're pretty well geared to each other. Combining RecyclerView and SelectionTracker with ItemTouchHelper opens another issue. When using a drag handle within a selectable list item, clicking the handle and starting to move the item slowly will select the item and start selection mode while drag mode is active. It gives weird user experience and requires lots of workarounds to come to a sane "standard" behaviour.

SelectionTrackerIssue.zip

826 KB

Download

vi...@google.com <vi...@google.com> #5Oct 11, 2019 11:37AM

Thanks again for all the feedback.

The second issue you describe is tracked by 159025478.
The last issue you described is likely related to 139141511.

vi...@google.com <vi...@google.com> Jan 14, 2020 12:11PM

Reassigned to sh...@google.com.

sh...@google.com <sh...@google.com> Jan 14, 2020 05:18PM

Reassigned to sm...@google.com.

sm...@google.com <sm...@google.com> #6Jan 28, 2020 07:07PM

None of these are accessible.

ap...@google.com <ap...@google.com> #7Aug 12, 2020 09:31PM

I don't honestly know how to control bug visibility, that's someone else's business as they know what's okay to make public and not, I don't.

Just be advised that the issues are being tracked.

...478 > Replace use of GestureDetector with custom event detection
...511 > Handle onRequestDisallowInterceptTouchEvent correctly

sm...@google.com <sm...@google.com> #8Aug 12, 2020 09:56PM

Marked as fixed.

I haven't been able to reproduce issue #1 described in the initial report (onClick fired after long press). For issue #2 it appears that GestureDetector is firing onDoubleTap in cases where there is a sequence of taps in rapid succession inside the "slop square". I'm investigating a fix.

Separately I landed a fix to comply with onRequestDisallowInterceptTouchEvent requests. This will appear in 1.1.0-rc02. Fancy demo app was updated with "swipe away" supporting using ItemTouchHelper to demonstrate compliance.

ap...@google.com <ap...@google.com> #9Aug 19, 2020 12:20AM

I have a fix out for review that address the issue where selection lib isn't handling the second tap of of two that fall into the double-tappy category as GestureDetector would have it. This addresses the issue where the second of two rapid taps in close proximity is not handled correctly.

That said, there are still cases where OnClickListeners added to child views are being called (at least using the Fancy demo activity).

1) Long press on an existing selected item.
2) When making many multi-taps (whacking the screen repeatedly with multiple fingers).

In both of those cases I haven't been able to track down a cause, but have verified that *when* TouchInputHandler receives events, it handles them correctly. It doesn't rule out issues elsewhere in the library, but I don't see any obvious paths of investigation. <Steve glares directly at GestureDetector once again>.

ap...@google.com <ap...@google.com> #10Aug 19, 2020 12:22AM

danielstone411@gmail.com, I'm curious where I might be able to add a note about use of View#OnClickListener vs OnItemActivatedListener. Where would you first look for your docs? Javadoc? If so, which class?

I'd like to update the docs to provide some guidance, but am not sure where would be most effective.

ap...@google.com <ap...@google.com> #11Aug 19, 2020 01:54AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 801fbf8d34a6c5b3939932c0b143b55e3ef1859b
Author: Steve McKay <smckay@google.com>
Date: Fri Aug 14 16:46:52 2020

Fix an issue where TouchInputHandler misinterpreted events from GestureDetector.

Consume up events from double tap stream, ignore double tap event iteself.

Bug: 161162268
Test: Added coverage to TouchInputHandler.
Change-Id: Ib0293818216d1c388e49860e976b639742838784

M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/TouchInputHandlerTest.java
M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/testing/TestDragListener.java
M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/testing/TestOnItemActivatedListener.java
M recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/MouseInputHandler.java
M recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/TouchInputHandler.java
M samples/Support7Demos/src/main/java/com/example/android/supportv7/widget/selection/fancy/DemoItemHolder.java
M samples/Support7Demos/src/main/java/com/example/android/supportv7/widget/selection/fancy/FancySelectionDemoActivity.java

https://android-review.googlesource.com/1401410

ap...@google.com <ap...@google.com> #12Aug 19, 2020 01:57AM

The mishandling of GestureDetector double tap events has been addressed. I pulled out a separate issue track just that as the fix will be included in 1.1.0-rc02.

Unfortunately I don't have a fix the issue of child View onClickHandlers firing at this time and will retain this bug to track the issue.

ap...@google.com <ap...@google.com> #13Aug 19, 2020 02:18AM

The separate issue is

https://buganizer.corp.google.com/issues/165030422

an...@google.com <an...@google.com> #14Aug 19, 2020 02:18AM

Project: platform/frameworks/support
Branch: snap-temp-L55000000669795486

commit a74207a20aa7a3b1ccd381654d7d34cc7c8bbb65
Author: Steve McKay <smckay@google.com>
Date: Fri Aug 14 16:46:52 2020

Fix an issue where TouchInputHandler misinterpreted events from GestureDetector.

Consume up events from double tap stream, ignore double tap event iteself.

Bug: 161162268
Test: Added coverage to TouchInputHandler.
Change-Id: Ib0293818216d1c388e49860e976b639742838784
(cherry picked from commit 18714872d72e6771c4c25ab856a0f42c253aec16)

M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/TouchInputHandlerTest.java
M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/testing/TestDragListener.java
M recyclerview/recyclerview-selection/src/androidTest/java/androidx/recyclerview/selection/testing/TestOnItemActivatedListener.java
M recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/MouseInputHandler.java
M recyclerview/recyclerview-selection/src/main/java/androidx/recyclerview/selection/TouchInputHandler.java
M samples/Support7Demos/src/main/java/com/example/android/supportv7/widget/selection/fancy/FancyItemHolder.java
M samples/Support7Demos/src/main/java/com/example/android/supportv7/widget/selection/fancy/FancySelectionDemoActivity.java

https://android-review.googlesource.com/1404004