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

Settings Editor UI doesn't handle touch events #84851

Closed
rebornix opened this issue Nov 14, 2019 · 1 comment
Closed

Settings Editor UI doesn't handle touch events #84851

rebornix opened this issue Nov 14, 2019 · 1 comment
Assignees
Labels
ios-ipados safari Issues running VSCode in Web on Safari
Milestone

Comments

@rebornix
Copy link
Member

  • VSCode Version:
  • OS Version:

Steps to Reproduce:

  1. Open web selfhost in iPad/iOS
  2. Open Settings Editor UI
  3. We can only scroll the settings but not change the value, touch down doesn't work.

Root cause analysis

TLDR, Setting Editor is using List Widget, which registers the container to Gesture. Once the DOM node is registered in Gesture, we have to implement all touch events handler, as events are being preventDefault() on the DOM node.

List Widget and List View registers itself to Gesture to support the correct scrolling when touch events are triggered

this.disposables.add(Gesture.addTarget(list.getHTMLElement()));

this.disposables.add(Gesture.addTarget(this.rowsContainer));

As list view handles the touch events, they will be preventDefault

if (this.dispatched) {
e.preventDefault();
e.stopPropagation();
this.dispatched = false;
}

Considering that Gesture has a robust logic of translating touchStart, touchMove and touchEnd to Tap/Change/Start/End, one solution is we handle the right Gesture event in every Settings View Component.

A less favorite solution is letting List View by pass some of the touch events but I wonder that would lead to bugs for List View virtualization.

Settings Editor is probably the only major component affected by this bug as others are simple List View with special Tap/ContextMenu handlers, while Settings Editor contains a lot of other basic UI elements.

@rebornix
Copy link
Member Author

FYI @joaomoreno . I appreciate that List View handles touch events internally as ScrollableElement doesn't handle touch at all (only mouse). Without that, we will run into tons of scrolling issues in any place we use ScrollableElement, like #83715 (comment) . Luckily List View saved us.

It's also why I'd like to keep List View and Gesture untouched. @roblourens I can send a PR to add touch handlers in Settings Editor renderer (I already had some working locally and they look fine) if the approach sounds good to you.

@rebornix rebornix added this to the November 2019 milestone Nov 14, 2019
@rebornix rebornix added ios-ipados safari Issues running VSCode in Web on Safari labels Nov 14, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ios-ipados safari Issues running VSCode in Web on Safari
Projects
None yet
Development

No branches or pull requests

2 participants