Skip to content

Add touch event handlers to the webagg and nbagg backends #18851

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pelson
Copy link
Member

@pelson pelson commented Oct 30, 2020

PR Summary

This PR compliments #8041 to add touch support for the webagg and nbagg backends. This will also unlock the door for the ipympl backend to be able to implement touch handlers for free, should they so wish.

I didn't want to tread on #8041's implementation, and believe it is healthy to keep a strong distinction between new backend behaviour and introducing an entirely new class of event handlers (something that perhaps should be done for the original PR too), so whilst the events are being fired and sent to the backend, they currently don't do anything (not even fire any matplotlib event). So in other words, this is a non-zero change for zero immediate benefit, and I'm completely willing to accept that we must have the backend_bases event infrastructure in place before merging this PR.

This also fixes a bug which prevents the webagg from running in device compatibility mode in chrome (Developer Tools -> Toggle device) in which there was a problem with the websocket having not yet been connected. The error message there was:

Uncaught DOMException: Failed to execute 'send' on 'WebSocket': Still in CONNECTING state

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@tacaswell
Copy link
Member

This also collides with #16931

@pelson pelson force-pushed the webagg_and_nbagg_touch_events branch from def1394 to 58fb0d9 Compare October 30, 2020 23:19
on_touch_event_closure('touch_move')
);
rubberband_canvas.addEventListener(
'touchenter',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I've not seen this event (nor touchleave) actually fire. Perhaps I don't have the right kind of device (mobile or chrome developer tools).

@anntzer
Copy link
Contributor

anntzer commented Oct 31, 2020

From a very quick look, it seems like on the GUI toolkits side (especially gtk, from which we got the event names (button_press_event, etc.), but also the others) typically have a single touch_event handler, and then let the single callback differentiate based on some flag present on the event; unless there's a strong reason to do things differently here I'd suggest also sticking to just touch_event + flag-on-the-event-object on Matplotlib's side.

It may also be nice to review how the various GUI toolkits handle multitouch (which is relevant for zooming), keeping in mind that we'd like to keep the API the same across all backends (except for the guiEvent attribute which refers, well, to the native event). Again, from a quick look:

So I guess another question would be whether to expose at all the low-level touch events or the higher level gestures in Matplotlib (after all the actual event handlers may be easier to write against a gesture API?)

@pelson
Copy link
Member Author

pelson commented Nov 21, 2020

Thanks for taking the time to look over the various touch event options across the backends @anntzer. I definitely agree with the idea of standardising on something which is consistent across the backends, and it isn't a given that the JS approach is the one we should necessarily opt for.

In terms of implementation, it doesn't change a huge amount I think - we just need to make a decision and then it can be implemented. How would you like to proceed with making the decision - I definitely don't have enough experience of touch events across backends to be able to make an informed decision that doesn't risk precluding certain types of touch event handling.

@anntzer
Copy link
Contributor

anntzer commented Nov 21, 2020

I don't actually know anything about touch event handling other than what I listed above (which basically just comes from some quick googling...). So I don't have much to contribute on the API design side. However, once again I think that even if you don't provide implementation (of whatever API you choose) for all backends, it would be nice to at least be sure that everything you propose is at least in theory implementable across them.

@timhoffm
Copy link
Member

timhoffm commented Nov 23, 2020

I'm also not experienced with touch events. But from a usability point of view, a gesture API is simpler to use. And it's simpler to get right because it's a higher level abstraction that's less dependent on the low-level API of the frameworks (we can wrap that). We can always add an additional low-level touch API later if needed.

@jklymak jklymak marked this pull request as draft May 11, 2021 00:53
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants