Allow the user to change font sizes with the text pane of turtledemo #66132
Comments
|
Currently, the turtledemo doesn't allow you to change font sizes of the demo code, and the default font size is really small. I can work on creating a patch to fix this. Best option be to have control-mousewheel change size, as is standard in browsers, as well as Ctrl + and Ctrl - to change sizes (since laptops don't have a mousewheel). If tk has a problem with that, a second choice would be a menu entry "Font size with choices such as the current size (9?), 10, 12, 14. Also, update the Help Text of this new feature. |
|
I have a version of this working with Ctrl-plus and Ctrl-minus. However, there is a bug with Tk 8.5.9 where binding to MouseWheel crashes Tkinter for Macs (bpo-10731), which I am running into. I need to update Tkinter to see if this works. |
|
Here is a patch for changing the font size using the scroll wheel. I also added the shortcuts "Ctrl-plus" to increase the font size and "Ctrl-minus" to decrease the font size. However, since the MouseWheel is now bound to changing the font Note, this patch also includes the window sash (bpo-21597). They are sort of dependent since I am also redefining the onResize method, so I clumped all the bindings to one method. But if you want that to be separate, I can try to make it into two separate patches. |
|
I plan to commit the sash patch before reviewing this. I would wait until then to do a separate patch. |
|
MOUSEWHEEL should continue to scroll. Get ^wheel to work right and I would like to add it to Idle, where ^wheel scrolls along with wheel. ^+ and ^- are pretty standard also, though LibreOffice does not recognize them. Perhaps this is because it is explicit cross platform. We can conditionally not bind wheel events on Mac setups where it fails. Does bpo-10731 have enough info to do that? In not... I have not yet looked into generating key/mouse events from code, but perhaps it would be possible to generate a wheel event inside try: except and unbind if there is an exception. |
|
Sounds good. I can wait till the sash code gets incorporated in order to I would have to generate a MOUSEWHEEL event and see if it fails. I have I will also try to figure out how to bind to Ctrl+MOUSEWHEEL and not just Lita On Mon, Jul 21, 2014 at 11:35 PM, Terry J. Reedy <report@bugs.python.org>
|
|
Lita, I tried the patch. From the perspective of an OS X user, while I might expect that using the zoom gesture on a mousepad or using a mousewheel (the equivalent) to increase or decrease the font size, I would even more expect scrolling to work especially if scrollbars are present. Clearly, scrolling is more important so, if it is not possible to bind Tk mousewheel events without affecting scrolling, I would abandon the mousewheel. On OS X, the standard way to provide size adjustment (of fonts or images) is to provide "Bigger" or "Smaller" menu items with the standard keyboard shortcuts of Command-Shift-Equal (and Command-Equal) which is displayed as "Command +" (so the user on a US keyboard just presses the Command key and the =/+ key) and Command-Hyphen ("Command -"). The Apple OS X Human Interface Guidelines go into more detail and you can see these shortcuts in action in many standard OS X applications (TextEdit, Mail, Safari, etc). As it stands today, turtledemo does not use the standard OS X menu bar where these commands would normally be placed. And that's a bit of a separate problem because since turtledemo doesn't change the root menu it defaults to a Tk-provided one which includes things "Run Widget Demo" under the file menu. To be a proper OS X app, turtledemo should customize the menu, at least removing the widget demo item and then it could add the Bigger and Smaller menu items to a Format menu. Actually, the turtledemo Examples and Help pulldown options would ideally also be available in the standard menu hierarchy. I'm not suggesting that is a requirement but that's what I think an OS X user would expect and what the Apple HIG would require. https://developer.apple.com/library/mac/documentation/userexperience/conceptual/applehiguidelines/KeyboardShortcuts/KeyboardShortcuts.html |
|
I completely agree about the mousewheel. However, would it make sense for I also think the shortcuts are not intuitive as an OS X user, as command What I can do check the operating system and define the font shortcuts On Mon, Jul 21, 2014 at 11:52 PM, Ned Deily <report@bugs.python.org> wrote:
|
|
On OS X, the actions associated with trackpad gestures are controlled by the Trackpad panel of System Preferences. The default settings map the "pinch with two fingers" gesture to "Zoom in or out" which Tk apps see as Mousewheel events: no programming needed! |
|
What really? That is so awesome! I will check that out! However, I figure I still need to create separate bindings for Linux, Lita On Tue, Jul 22, 2014 at 12:18 AM, Ned Deily <report@bugs.python.org> wrote:
|
|
"However, I figure I still need to create separate bindings for Linux, I'm not sure I understand: I think that Tk only provides one MouseWheel event binding. Keyboard or menu items might differ, yes, e.g. Cmd-+ vs Ctrl-+. |
|
Oh I see. And then pinch on the trackpad just generates a overall MouseWheel event, not a specific zoom-in event. For some reason, I thought there was a different event depending on operating systems. Before, Linux would trigger <Button-4> and <Button-5> events and not MouseWheel. On Jul 22, 2014, at 12:35 AM, Ned Deily <report@bugs.python.org> wrote:
|
|
I've added it so that if the OS is Mac, it will use Command-minus and Command-=. If it is on Windows, it uses Ctrl-minus and Ctrl-= (I wasn't sure if Windows uses shift sa well, but I don't think it does.) When I tried the "pinch" movement in Mac, the MouseWheel event didn't trigger at all. It is only when I did the scroll (for me two fingers moving downward or upward) movement is when that event triggered. I currently have it so that when Control+MouseWheel makes the font size move. Let me know what you think. I can also try adding a widget so that the user can change the font through the GUI. |
On windows, new patch gives this:
Traceback (most recent call last):
File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 295, in <module>
demo = DemoWindow()
File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 73, in __init__
graph_frame = self.makeGraphFrame(pane)
File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 115, in makeGraphFrame
self._makeBindings(turtle._Screen._canvas._rootwindow)
File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 130, in _makeBindings
root_window.bind_all('<%s-minus>' % shortcut, self._decreaseFont)
File "F:\Python\dev\4\py34\lib\tkinter\__init__.py", line 1049, in bind_all
return self._bind(('bind', 'all'), sequence, func, add, 0)
File "F:\Python\dev\4\py34\lib\tkinter\__init__.py", line 992, in _bind
self.tk.call(what + (sequence, cmd))
_tkinter.TclError: bad event type or keysym "Ctrl"/Ctrl/Control in "shortcut = 'Control" and demo runs. root_window.bind_all('<%s-minus>' % shortcut, self._decreaseFont)
root_window.bind_all('<%s-=>' % shortcut, self._increaseFont)^- shrinks on -_ key and num keypad. ^wheel either way maked giant type -- evt.delta on my machine is +-120! Out general policy is one patch per issue. The one for bpo-21587 already fixes two issues. After we finish that (see questions), produce a small patch for this. |
|
Sounds good. I will wait till bpo-21587 and create a small patch afterwards. On Tue, Jul 22, 2014 at 9:33 PM, Terry J. Reedy <report@bugs.python.org> >
> Terry J. Reedy added the comment:
>
> On windows, new patch gives this:
> Traceback (most recent call last):
> File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 295, in
> <module>
> demo = DemoWindow()
> File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 73, in
> __init__
> graph_frame = self.makeGraphFrame(pane)
> File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 115, in
> makeGraphFrame
> self._makeBindings(turtle._Screen._canvas._rootwindow)
> File "F:\Python\dev\4\py34\Lib\turtledemo\__main__.py", line 130, in
> _makeBindings
> root_window.bind_all('<%s-minus>' % shortcut, self._decreaseFont)
> File "F:\Python\dev\4\py34\lib\tkinter\__init__.py", line 1049, in
> bind_all
> return self._bind(('bind', 'all'), sequence, func, add, 0)
> File "F:\Python\dev\4\py34\lib\tkinter\__init__.py", line 992, in _bind
> self.tk.call(what + (sequence, cmd))
> _tkinter.TclError: bad event type or keysym "Ctrl"
>
> /Ctrl/Control in "shortcut = 'Control" and demo runs.
>
> root_window.bind_all('<%s-minus>' % shortcut, self._decreaseFont)
> root_window.bind_all('<%s-=>' % shortcut, self._increaseFont)
>
> ^- shrinks on -_ key and num keypad.
> &+ enlarges on =+ key but not num keypad. Fix by adding
> root_window.bind_all('<%s-plus>' % shortcut, self._increaseFont)
>
> ^wheel either way maked giant type -- evt.delta on my machine is +-120!
> self.txtfont['size'] += evt.delta // 120
> works like expected. And please spell out 'event'.
>
> Out general policy is one patch per issue. The one for python/cpython#65786 already
> fixes two issues. After we finish that (see questions), produce a small
> patch for this.
>
>
|
|
Oops! I was suppose to add 'Control' not 'Ctrl'. I can fix that quickly but I will wait till the other patch goes through. |
|
Here is an updated version of the patch now that Terry submitted the changes from bpo-21597. |
|
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. |
|
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?) 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: |
|
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. |
|
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.) |
|
In 3.5 lists should work as fine as tuples. But in earlier version it would be Configure's value is preprocessed in Misc._options. If it is a tuple or a list |
|
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.
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. |
|
On X11 <Button-4> and <Button-5> events are generated on mouse wheel roll. So you should add widget.bind('<Control-Button-4>', self._increaseFont)
widget.bind('<Control-Button-5>', self._decreaseFont)But mouse wheel events still are sent to widget and text is scrolled together with font size changing. |
|
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! |
|
ping! |
|
New changeset ce14092430b6 by Terry Jan Reedy in branch '3.4': New changeset e2e0c9f90a81 by Terry Jan Reedy in branch 'default': |
|
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. |
|
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. |
|
Unfortunately the bug on X11 is still here. Ctrl+mouse wheel not only changes font, but scrolls a text. |
|
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. |
|
Here is a patch which should fix this bug. In additional it fixes a bug on MacOS: mouse wheel only increased font size. |
|
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. |
|
This works on X11 (Linux). Actually it doesn't matter for what widget you call bind_all(). It works globally. |
|
New changeset ecc98ea50bc3 by Terry Jan Reedy in branch '3.4': |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: