Skip to content
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

Context menu false positives while scrolling (and ability to disable it) #227

Closed
Alex0007 opened this issue Aug 30, 2023 · 8 comments
Closed
Labels
bug Something isn't working need investigation

Comments

@Alex0007
Copy link

Alex0007 commented Aug 30, 2023

Here is an example of context menu appearing when I'm just trying to stop scrolling feed. On mobile this menu often appears when I don't want to.

Maybe show it only on post timestamp hold (instead of whole post area). Or add timeouts to prevent it from showing it immediately after scrolling? Or add a setting to disable it 🤔

For me it just feels wrong 😭

I'm using dev.phanpy and Google Chrome on Android 14

VID_20230830_230407_776.mp4
@Alex0007 Alex0007 changed the title Context menu false positives (and ability to disable it) Context menu false positives while scrolling (and ability to disable it) Aug 30, 2023
@cheeaun cheeaun added bug Something isn't working need investigation labels Aug 31, 2023
@cheeaun
Copy link
Owner

cheeaun commented Aug 31, 2023

Adding context: should have been fixed by #165 but somehow still happening on your mobile.

@Alex0007
Copy link
Author

@cheeaun it still happening

can we prevent context menu opening if click happened faster than 500-1000 ms after "scroll end" event

@Alex0007
Copy link
Author

Alex0007 commented Sep 29, 2023

@cheeaun

Problem solved on system level.

Screenshot_20230929-133428~2.png

https://support.google.com/accessibility/android/answer/6006989?hl=en

To adjust the touch and hold delay:

Open your device's Settings app .
Tap Accessibility and then Touch & hold delay.
Select Short, Medium, or Long.

For some reason the timing value was "Short"

@Alex0007 Alex0007 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 29, 2023
@Alex0007 Alex0007 reopened this Sep 29, 2023
@Alex0007
Copy link
Author

@cheeaun

A different problem appeared after I have changed "hold delay" to "long".

Menu appears early without vibration feedback and disappears if you stop holding.

If you hold longer - vibration feedback happens and menu reappears

VID_20230929_135136_150.mp4

@cheeaun
Copy link
Owner

cheeaun commented Sep 29, 2023

@Alex0007 I think "Short" is actually the default (I also see it on my Pixel 3). I don't think this should affect the bug much and if it does, I'd have no idea what's happening as there's no touch-hold/longpress event in JS at all, so it's likely conflicting with what the browser/OS is doing, with no way for JS to control 😅 Duration for touch-hold/longpress is also hardcoded in JS, so it's not able to respect the OS Hold Delay setting.

Anyway, I've reduced cancelOnMovement to 2 pixels, maybe you can change hold delay back to "Short" and try again 🙏

Thanks a lot for keep trying this! 🙇‍♂️

@Alex0007
Copy link
Author

Alex0007 commented Sep 30, 2023

@cheeaun

If you set big threshold in useLongPress – you will notice that there are two events triggering context menu:

contextmenu: respects system hold "delay setting", it has vibration feedback

touchstart: this one is coming from useLongPress hook and uses threshold value from code. It doesn't have vibration feedback.

In my opinion contextmenu event should be preferable

Screenshot 2023-09-30 at 08 47 33

Two events while i was doing single long press:
Screenshot 2023-09-30 at 08 46 43

Context menu is still shown with contextmenu event after this change:
Screenshot 2023-09-30 at 08 57 24

@cheeaun
Copy link
Owner

cheeaun commented Sep 30, 2023

@Alex0007 thanks!! Didn't know contextmenu actually works on Chrome Android 😱 (it doesn't work on Mobile Safari, so I assumed other mobile browsers followed its path). I'll do further testing, maybe need to first feature-detect contextmenu support as it's way more reliable than the hacky "longpress" event.

@Alex0007
Copy link
Author

Alex0007 commented Oct 1, 2023

It feels much better without useLongpress, thanks @cheeaun!

Also it seems like this issue was also because of useLongpress

@Alex0007 Alex0007 closed this as completed Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need investigation
Projects
None yet
Development

No branches or pull requests

2 participants