Fix a bug in TextBox where shortcut keys were not being reenabled#7585
Fix a bug in TextBox where shortcut keys were not being reenabled#7585NelleV merged 2 commits intomatplotlib:masterfrom mikehenninger:textbox_typo_fix
Conversation
…nabled in _click(), the self.capturekeystrokes flag was being set false before calling stop_typing(). stop_typing() expects this flag to still be set if the textbox was capturing keystrokes, and does not reenable shortcut keys if it is not set. The line setting self.capturekeystrokes false in _click() is deleted, which fixes the behavior in /examples/widgets/textbox.py. There is no test set setup for the textbox class and I'm not prepared to start one now, so no actual test.
lib/matplotlib/widgets.py
Outdated
| if event.inaxes != self.ax: | ||
| self.capturekeystrokes = False | ||
| self.stop_typing() | ||
| self.stop_typing() |
There was a problem hiding this comment.
You have one too many spaces on this line which is causing syntax errors.
There was a problem hiding this comment.
Idonteven... thanks.
I'm new to this whole contributing to public projects thing... I marked this PR as editable by the mpl powers that be, but I don't know that just means this thread or the committed code itself. Given that git is telling me that my remote is unchanged, I guess it means the former (your diff in which you show the fixed code notwithstanding), so I went ahead and pushed the typo fix myself. Is that right?
There was a problem hiding this comment.
To be clear, marking the PR as editable allows mpl committers to push changes to your branch--but that takes more than just commenting here.
|
In the absence of testing (which I agree is difficult for UI work) it would be nice if you could post a minimal example that was failing before this PR and works with it -- something that the maintainers can directly copy-paste to test the patch. |
|
minimal example in lieu of proper test: code below is just /examples/widgets/textbox.py.
* submit(text) does no validation and will barf at eval(text) if text isn't valid expression import numpy as np
import matplotlib.pyplot as plt
from matplotlib.widgets import TextBox
fig, ax = plt.subplots()
plt.subplots_adjust(bottom=0.2)
t = np.arange(-2.0, 2.0, 0.001)
s = t ** 2
initial_text = "t ** 2"
l, = plt.plot(t, s, lw=2)
def submit(text):
ydata = eval(text)
l.set_ydata(ydata)
ax.set_ylim(np.min(ydata), np.max(ydata))
plt.draw()
axbox = plt.axes([0.1, 0.05, 0.8, 0.075])
text_box = TextBox(axbox, 'Evaluate', initial=initial_text)
text_box.on_submit(submit)
plt.show() |
|
Upon further testing, it seems that the issues are different depending on whether toolmanager is enabled or not. It seems that this PR fixes the case without toolmanager, i.e. it reenables shortcuts after defocusing from the textbox. In any case this is already an improvement. |
anntzer
changed the title
|
@anntzer Yes, there are definitely other toolmanager bugs. "shortcuts never get disabled" is the toolmanager bug for TkAgg, while for Qt5Agg, the shortcuts never get enabled... even if there is no textbox. I talk a bit about this in a comment on issue #7571 This PR was for a bug that was on top of/independent of them, which is why I bothered to make a PR even with these (far bigger) bugs outstanding. |
Does the |
|
Can we hold off on backporting this to the rc? |
|
assuming I am not confused and it even can be backported. |
|
My plan was holding off until I found time to check whether it had to be backported. |
|
The only instance of |
QuLogic
modified the milestones:
2.1 (next point release),
2.0.1 (next bug fix release)
QuLogic
changed the title
6 participants
in _click(), the self.capturekeystrokes flag was being set false before
calling stop_typing() when the click was outside the textbox axes.
stop_typing() expects this flag to still be set if the textbox was
capturing keystrokes, and does not reenable shortcut keys
if it is not set. The line setting self.capturekeystrokes false in _click()
is deleted, which fixes the behavior of /examples/widgets/textbox.py. There
is no test suite set up for the textbox class and I'm not prepared to start
one now, so no tests submitted for this bugfix.
This is babby's first pull request so let me know if I'm doing it wrong.