-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
IsItemClickEnabled and SlidableListItem #316
Comments
Since phones don't use mouse for interaction it is working properly, so it is intented. I don't see a point of using ItemClick with SlidableListItem though. And also event is triggered after the swipe operation is complete. So nothing being triggered during swipe. Maybe we can implement some kind of a SwipeCompleted and SwipeStarted events to provide more control over this. |
I think item click in addition to selection is worth having no matter what item type is used. |
Considering the default mail app on phones use swipe, selection and itemclick, we should try to deal with it. |
I just want to add some opinions after some more thinking about this:
|
Before UWPCommunityToolkit, we had developed our own SlidableListItem for our app FeedLab Windows 10 (private beta until the end of September).
On our app, if the event ItemClick is thrown but the value of IsSwiping is true, we ignore the click |
Interesting idea, @lemoinea :-). I was just on the way to suggest a similar thing. Currently there is a SwipeStatus property that could be used to check if swiping is occurring. The only problem right now is that ItemClicked event is triggered after the swiping is completed. But if the essential code in ManipulationCompleted is executed with Dispatcher.RunAsync ItemClick will be triggered before swiping is completed. Suppressing the ItemClick event, like I suggested earlier, seems to be hard to do in safe way. By listening on PointerPressed or PointerReleased event it's possible to do that. But that will also stop some animations to work so it seems to be a bad idea. This is just my thoughts, I’m really no expert in this :-). |
#403 should fix this issue. Let me know if I missed anything. I simply swallow the pointerreleased event if the user was swiping so the itemclicked or selection events don't fire in that case. |
@nmetulev, this fix introduces a new problem. If you enable IsItemClickEnabled in the sample application and push down on an item in the list, swipes and then release the list item will still look likes it pressed: Isn’t reordering the events, like suggested above, a safer solution? |
You are correct that with the current implementation the item looks pressed after swiping, but it resets as soon as the mouse moves away from the control. Reordering the events can work for the IsItemClickEnabled, but that moves the code from the control to the developer to implement in their app anyway. In the case of SelectionMode, not sure if the reordering of the events will even work. |
BTW, still investigating into making the best of both worlds, so I'll reopen this issue until it has been verified |
OK, so I added a new property to gate this behavior and it is disabled by default. I also implemented your suggestion to delay the swipe status changed event to Idle if you want to handle it yourself. It will not help for selection, but in those cases you can use the new property to handle the PointerReleased event in the control. To prevent the above issue from happening, one solution is to change the template of the ListViewItem to remove the animations (for example). Thoughts? ref #412 |
Hmm… Sorry, but I’m not getting it. Is there any issue with selection? Either way, I really like your PR. I think it covers all needs :-) |
I like it too! Really good job |
I’m trying to use SlidableListItem in a ListView where IsItemClickEnabled is true. If I’m doing this the ItemClick event is triggered even when an attempt to swipe was made (not what I want). Any suggestion how to deal with that?
Tested on version 1.0.0 from NuGet.
Update: This happens on a computer when I swipe with a mouse. On a phone it seems to work differently.
The text was updated successfully, but these errors were encountered: