Repository navigation
Allow the user to change font sizes with the text pane of turtledemo #66132
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2014-09-05.01:02:05.446>
created_at = <Date 2014-07-07.15:18:15.486>
labels = ['type-bug']
title = 'Allow the user to change font sizes with the text pane of turtledemo'
updated_at = <Date 2014-09-05.01:02:05.445>
user = 'https://bugs.python.org/LitaCho'bugs.python.org fields:
activity = <Date 2014-09-05.01:02:05.445>
actor = 'terry.reedy'
assignee = 'none'
closed = True
closed_date = <Date 2014-09-05.01:02:05.446>
closer = 'terry.reedy'
components = []
creation = <Date 2014-07-07.15:18:15.486>
creator = 'Lita.Cho'
dependencies = []
files = ['36019', '36040', '36077', '36122', '36124', '36126', '36135', '36155', '36532', '36534']
hgrepos = []
issue_num = 21933
keywords = ['patch']
message_count = 36.0
messages = ['222466', '223619', '223620', '223628', '223629', '223630', '223631', '223632', '223635', '223637', '223639', '223646', '223713', '223720', '223721', '223730', '223906', '224093', '224099', '224104', '224106', '224109', '224150', '224157', '224261', '225323', '225330', '226319', '226321', '226325', '226336', '226347', '226360', '226368', '226369', '226394']
nosy_count = 6.0
nosy_names = ['terry.reedy', 'ned.deily', 'jesstess', 'python-dev', 'serhiy.storchaka', 'Lita.Cho']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21933'
versions = ['Python 3.4', 'Python 3.5']
Activity
LitaCho commentedon Jul 23, 2014
Oops! I was suppose to add 'Control' not 'Ctrl'. I can fix that quickly but I will wait till the other patch goes through.
LitaCho commentedon Jul 24, 2014
Here is an updated version of the patch now that Terry submitted the changes from bpo-21597.
LitaCho commentedon Jul 27, 2014
I was going to add a dropdown menu to change the font size as well, but I am going to wait till Serhiy's patch gets committed in bpo-22065 before I submit my patch.
terryjreedy commentedon Jul 27, 2014
v3 missed 3 of the 4 fixes I requested. I would like to commit the attached simplified version of v3 that includes all fixes, leaves txtfont global as a list, and uses subscripting consistently instead of .config. But I would first like confirmation that this version works well enough on Mac to do do.
Ned, with this binding, (ignored on Mac?)
widget.bind_all('<Control-MouseWheel>', self._updateFont)
plain wheel moves scroll as desired while Control-wheel changes font sizes. This is standard behavior on Windows.
I am not sure a menu entry is needed, but if one were added, you are right that it should follow Serhiy's menu patch
Serhiy, a question partly for you: various places specify a tuple when a font is specified by family, size, style. By experiment, tkinter.font.Font requires a tuple for such a sequence. However, a list seems to work fine in 2.7 and 3.x when setting the font attribute of text:
text['font'] = txtfont
Does tkinter or _tkinter convert to tuple? Can we depend on this, or is it an accident? To be safe, for now, I am leaving tuple(txtfont), but I wonder if it is needed.
LitaCho commentedon Jul 27, 2014
Hi Terry,
I originally had txtfont as a list, but I guess I was worried about readability and accessing attributes by indices being unpythonic. But I might have been over-doing it for this case. :)
In Mac, you don't need to divide event.delta by 120, only Windows and X11 systems. This is according to a Stackoverflow answer: http://stackoverflow.com/questions/17355902/python-tkinter-binding-mousewheel-to-scrollbar
But maybe I'm wrong? I've attached my small change.
terryjreedy commentedon Jul 27, 2014
While we are at it, lets compute "sys.platform == 'darwin'" just once and use conditional expressions. (I just condensed config_gui() with conditional expressions, but avoided changing the context for the patches here.)
serhiy-storchaka commentedon Jul 27, 2014
In 3.5 lists should work as fine as tuples. But in earlier version it would be
safer to convert list to tuple.
Configure's value is preprocessed in Misc._options. If it is a tuple or a list
with only int and str items, it will stringified. But if a list contains non-
int or non-str items, it will passed as is, and _tkinter in pre-3.5 will fail
because lists are not supported.
ned-deily commentedon Jul 28, 2014
Several review comments on tdemo-font-34-t2.diff:
That seems to work on OS X as well (with both the Cocoa and Carbon Tks), although IMO having a control-mousescroll affect the font size is not something an OS X user would expect; I know of no other GUI application on OS X with that behavior. But it doesn't hurt.
That said, the "sign" of the mousescroll is reversed on OS X (as Kevin Walzer's comment implies here (http://wiki.tcl.tk/3893): note the minus sign).
AFAICT, "=" is not a legal Tk keysym (http://www.tcl.tk/man/tcl8.6/TkCmd/keysyms.htm), some Tk variants either ignore it, others (e.g. OS X X11) raise an exception. Should be "equal".
If the user repeatedly decrements the font size (either with the control(or command) "-" or with control-mousescroll, txtfont[1] can go negative but it appears Tk effectively uses its absolute value. So the font decreases to near invisibility then starts to increase again. Suggest adding a "minimum font size" (perhaps 8) and not allow txtfont[1] to decrease below it.
Attached patch tdemo-font-34-t3.patch addresses the above issues.
Someone should test this all with a Linux X11 setup with a mouse wheel and/or trackpad. The OS X X11 Tk variant does not seem to support the mouse wheel events and there are hints in the above Tk mousewheel link that no X11 Tk's do without some hackery. The documentation for turtledemo should be clear about which platforms are supported.
serhiy-storchaka commentedon Jul 28, 2014
On X11 <Button-4> and <Button-5> events are generated on mouse wheel roll. So you should add
But mouse wheel events still are sent to widget and text is scrolled together with font size changing.
LitaCho commentedon Jul 29, 2014
Hi Terry,
I've added to the patch, so that the user is able to change the font size through the GUI. I tried to match Google Doc's behaviour. I also added a max font size. I choose 400 since that is what Google Docs limits their font size.
If you prefer to split out the GUI functionaly out of this patch and submit a new patch after this has been committed, that's totally cool!
LitaCho commentedon Aug 14, 2014
ping!
terryjreedy commentedon Aug 15, 2014
I am working on backporting the bpo-10291 patch and re-synchronizing 3.4 and 3.5 so revised version of this patch, including help change, and of bpo-22065 patch can merge forward properly.
python-dev commentedon Sep 3, 2014
New changeset ce14092430b6 by Terry Jan Reedy in branch '3.4':
Issue bpo-21933: Users can now change the font size for example code.
http://hg.python.org/cpython/rev/ce14092430b6
New changeset e2e0c9f90a81 by Terry Jan Reedy in branch 'default':
Issue bpo-21933: Merge with 3.4
http://hg.python.org/cpython/rev/e2e0c9f90a81
terryjreedy commentedon Sep 3, 2014
I refactored the callbacks to eliminate duplication. I had to redo the menu addition to work with the new menu. This works great on Windows. I am confident I did not change the logic, but it would still be good if someone tried font changing again on linux and mac.
I am not going to backport this to the 2.7 Demo version, as backporting has always been a nuisance and now the help and menu redesigns have also not been backported.
terryjreedy commentedon Sep 3, 2014
Lita, thank you for sticking with this. bpo-17642 is about doing something similar for Idle. The issue is necessarily more complicated, but what we learned here about system difference and tk behavior oddities will be needed for Idle too. I am making 'what we learned' comments there.
serhiy-storchaka commentedon Sep 4, 2014
Unfortunately the bug on X11 is still here. Ctrl+mouse wheel not only changes font, but scrolls a text.
terryjreedy commentedon Sep 4, 2014
I am aware of this because the Windows has the same behavior. As I noted in my post to bpo-17642, I consider this behavior a tolerable glitch rather than a patch-blocking bug for turtledemo because the text is relatively short and read only, so there is no issue of needing to keep a cursor in place and visible. The issue would be different for Idle if indeed this is a tk rather than turtle problem. Also, people would typically only move a few clicks up or down, and scrolling up when already at the top of the file has no effect. So I do not regard this as a bug for this issue and regard this as still closed unless you happen to have thought of a workaround.
If you don't have a fix now, you could open a separate issue to investigate whether the linkage is a tk, tkinter, or turtle bug.
As part of bpo-17535, I discovered that when a Text widgets change the font size, they scroll up, down too far (or down and up too far), and back to where they should be. This is not visible with small files like the turtle examples, but is with 3000-line files like idlelib/EditorWindow.py, especially when line numbers are enabled. My test script attached to bpo-17535 shows that Text indeed calls scrollbar.set 3 times. I regard this as a tk bug. My point here is that Text widget font size changes are a bit flaky even without the mousewheel issue.
serhiy-storchaka commentedon Sep 4, 2014
Here is a patch which should fix this bug. In additional it fixes a bug on MacOS: mouse wheel only increased font size.
terryjreedy commentedon Sep 4, 2014
Great. That works better on Windows too. Attached is an augmented patch to also move the other size bindings to text (I verified that 'bind_all' is needed), move the resize binding back to where it was, delete the now-empty binding function, and add a not to the docstring that the mousewheel only resizes when the mouse is over the text. Please verify that this also works for non-windows.
serhiy-storchaka commentedon Sep 4, 2014
This works on X11 (Linux).
Actually it doesn't matter for what widget you call bind_all(). It works globally.
python-dev commentedon Sep 5, 2014
New changeset ecc98ea50bc3 by Terry Jan Reedy in branch '3.4':
Issue bpo-21933: Make Control-Mousewhell only change font size and not also scroll.
http://hg.python.org/cpython/rev/ecc98ea50bc3