codenameone / CodenameOne

Cross-platform framework for building truly native mobile apps with Java or Kotlin. Write Once Run Anywhere support for iOS, Android, Desktop & Web.
https://www.codenameone.com/
Other
1.72k stars 408 forks source link

Swipe action for SwipeableContainer too easily triggered when scrolling a list #3160

Open ThomasH99 opened 4 years ago

ThomasH99 commented 4 years ago

I've noticed an annoying UX issue: when scrolling up or down a long list of SwipeableContainers the swipe effect is triggered much too easily, meaning that very often a simple scroll leaves the user with an unintended open SwipeableContainer.

The code below can be used to see the issue: scroll the list rapidly up/down and a minimum of finger movement in the x-axis will open the Swipeable below the finger (not every time, it requires a little bit of finger movement in the x-axis).

In a normal siutation, the Swipeable will open with a 'short and fast' swipe which is exactly what you want. My guess is that when scrolling a list that same x-movement occurs easily due to the finger scroll movement and is tricking the Swipeable into reacting like it was a 'pure' x-movement.

It seems the solution should be to ignore the short and fast x-movement if it comes with a much bigger y movement (implying user is scrolling rather than trying to activate swipe).

Form hi = new Form("drag-scrolling a list too easily triggers swipeContainer", BoxLayout.y());
hi.add(new SpanLabel("To reproduce, scroll list rapidly up/down and the Swipeable under the finger will open too easily despite very little movement in the x-axis"));
for (int i=0; i<40;i++)
    hi.add(new SwipeableContainer(new Button("SwipeButton"+i), new Button("SwipeButton"+i),new Label("Topcomponent Label "+i)));
hi.show();
ThomasH99 commented 3 years ago

Any chance you could look at this? It makes my UI (a scrollable list of side-swipeable elements) really awful and I believe anyone else using SwipeableContainer in scrollable lists (I guess this would be a typical case) will have the same issue.

By the way, I only notice the issue on a device (iPhone). It could be because I scroll in a more controlled way with the mouse in the Simulator, but maybe it is due to some device-specific issue.

shai-almog commented 3 years ago

Sorry it slipped a bit under the radar. Did you look at the contacts demo in the kitchen sink? It has a scrollable list and swipable entries. Does the problem happen there?

ThomasH99 commented 3 years ago

Thanks for following up. No, it doesn't happen in the KitchenSink demo. However, I could reproduce it with the example code above, so it seems to be a CN1 issue (I haven't tried again on the device since submitting this bug since I try to avoid using the precious build-points for reproducing bugs). Could it be an issue with a later CN1 version than the one the KitchenSink demo was build with (I saw that the demo was uploaded to Apple store 4 years ago)?

ThomasH99 commented 3 years ago

Let me know if I can help!

I've tried debugging myself (since this is killing the usability of my app), but not managed to find the cause so far. It is also difficult because I only see the issue on devices, not the simulator. At one point I thought it was due to having many buttons 'under' the swipe (so being wider than in the Kitchensink demo), but that doesn't seem to be the reason.

I've also not seen any other reasons in my code to explain this (although I guess there must be something). If you have any hints on what could impact the triggering of the swipe, I'll investigate.

The only additional indication I've found so far is that it seems to only happen with the Right swipe (where you swipe left to reveal the buttons on the right side of the screen). It doesn't seem to happen for buttons in the left swipe. So, seems there is some asymmetry, maybe this can help understand what happens?

ThomasH99 commented 2 years ago

Hello, I'm getting back to this. It is still a major issue for my app and I've no clue about what's causing it. It's also seems to be much worse on iPhone than on the simulator, but maybe that's due to how you scroll a list using the mouse in the Simulator (click+draw - so maybe it registers the sideways component of the mouse path differently).

To summarize: when scrolling a list with SwipeableContainers rapidly up and down, even a tiny sideways move of the finger triggers the swipe container (revealing the underlying buttons). It seems that speed of finger movement matters: a fast flick to scroll will almost inevitably swipe the element, a slow scroll much less. If scrolling up/down directly vertically this doesn't happen, nor if scrolling the list with a sideways component in the opposite direction of what's used to reveal the swipe buttons. I spend weeks last year investigating this but didn't find any explanations in my code and even though the KitchenSink doesn't seem to exhibit this issue, it is easily reproduced with the code above (I just rested on an iPhone).

Any suggestions for what might cause this (or how I can investigate further) would be greatly appreciated :-)

ThomasH99 commented 2 years ago

I've tried digging into to this issue and I believe I've identified the issue. What helped me find it was that the overly sensitive triggering of the swipe only happens for a left-wards swipe but never for a right-ward swipe.

In the code below in the SwipeableContainer's internal class SwipeListener's method actionPerformed, the code if (topX > 0) checks if swipe is already opened to the right, but if that is not the case, it implicitly means it was opened to the left, which is obviously not always the case. Adding the additional check else if (topX < 0) prevents this and checking in the simulator confirms this resolves the issue (I can't test on device since this is CN1 code).

I'm not extremely confident I understand everything here, so I don't want to do a pull request, but if you confirm my fix is correct, I would greatly appreciate if you would update the code.

//in SwipeableContainer.SwipeListener:
case RELEASE: {
                    if (waitForRelease) {
                        initialX = -1;
                        //if (!isOpen()) {
                            int topX = topWrapper.getX();
                            //it's opened to the right
                            if (topX > 0) {
                                if (Display.getInstance().getDragSpeed(false) < 0) { 
                                    open = false;
                                    openedToRight = false;
                                    openToRight();
                                } else {
                                    open = true;
                                    close();
                                }
                            } else if (topX < 0) { // <-- check *explicitly* if opened to the left
                                if (Display.getInstance().getDragSpeed(false) > 0) {
                                    open = false;
                                    openedToLeft = false;
                                    openToLeft();
                                } else {
                                    open = true;
                                    close();
                                }
                            }
                        //}
                        waitForRelease = false;
                    }
                    break;
                }
ThomasH99 commented 2 years ago

Do you approve this fix? Do I need to make PR to move this forward?

ThomasH99 commented 2 years ago

Not sure if my follow-up on this (now fairly old) issue was seen so adding @shai-almog. It would be greatly appreciated if you could check/implement this (small) fix which will have a huge positive impact for me :-)