-
Notifications
You must be signed in to change notification settings - Fork 228
FIX: port the key handling changes from mpl #310
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
Conversation
ports changes from matplotlib/matplotlib#19146 closes matplotlib#309
Style checkers: 2 Tom: nil |
@@ -241,6 +242,68 @@ def send_binary(self, data): | |||
def new_timer(self, *args, **kwargs): | |||
return TimerTornado(*args, **kwargs) | |||
|
|||
if matplotlib.__version__ < '3.4': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woah neat! I had no idea this was a thing you can do
👍 Makes sense to me. It would also be great to have somewhere to put your test case #309 (comment) somewhere (especially for whenever a proper widget testing framework solidifies - which I think will be based on https://github.com/jupyterlab/galata ???) A good place would be the UAT notebook from #305. But if this gets merged first lets just make sure to open a follow up issue for adding to the UAT. |
That would be great indeed. |
I'm not sure what the current ipympl release schedule is, but can this go out as soon as possible? We are probably going to do mpl 3.4 in the next few days. |
I think the release schedule has been: Something gets merged, then we wait a few days until someone remembers to make a new release. So lets release right after merging this. (Or maybe after this + #305) |
I tried this out, but it's not actually clear to me what this fixes? Is it only possible to see the bug in a non-qwerty layout? Overall I trust that this fixes the issue though |
Without this patch, but with mpl3.4.0rcX / v3.4.x / master when you type keys you get back the key code as a string. mpl3.3 and before we shipped back the key codes and then converted to keys a (private) method on the Python side. We overhauled that code for mpl34 which required changes to both the py and js sides. ipympl re-uses the Python side logic (did not trace it through fully, but it is via inheritance) but has its own version of the js side. This PR back-ports the js side, so that we feed mpl34 the values it expects, but that would break ipypmpl with any, ah, currently released version of Matplotlib which seems bad. So this also backports the conversion function and hides it behind a mpl version-gated at class-definition time check to selectively over-ride the private method. Thus if we do change the private function again (without changing the protocol to with the js side), then ipympl will get the improvements and old versions of mpl still work :) |
Thanks! |
This should be in version |
ports changes from matplotlib/matplotlib#19146
closes #309