Repository navigation
IDLE: replace DynOptionMenu with a ttk Combobox or OptionMenu #71942
Description
Activity
jfoo commented
on Aug 13, 2016 jfoomannequinMannequinAuthorOne aspect of the IDLE interface that looks extremely old is the the dropdown menu. In the patch, I think I've preserved whatever essential functionality DynOptionMenu used to have, but I'm relatively unfamiliar with Tk so I'm not sure.
jfoo commented
on Aug 14, 2016 jfoomannequinMannequinAuthorAh, I've noticed the folly of my ways. This sort of stuff is already being managed with patches filed under various other issues. It just wasn't obvious to me as I wasn't seeing many new improvements in the default branch or much communication on IDLE-dev.
Justin, we are using ttk in 3.6, so the idea is feasible if it works. I do not remember a specific proposal to use ttk Combobox, as it could not be done before now.
Mark, any comments?
jfoo commented
on Aug 15, 2016 jfoomannequinMannequinAuthorI wasn't sure if the ongoing work in bpo-24781 essentially rendered my patch obsolete, so I keenly await Mark's response.
Upon reflection, I think my patch is a cheap win even if it's later overhauled by other improvements.
roseman commented
on Aug 17, 2016 rosemanmannequinMannequinJustin, as you say, I think your patch is entirely reasonable as an interim step, as eventually doing a broader improvement on the preferences dialog as suggested in bpo-24781 makes sense.
My reworked version used Combobox in similar ways; I think we can safely do away with the wrapper class and just use the ttk widget directly in the dialog (as the widget already handles the dynamic changes to the list, which the old tk_optionMenu didn't)
jfoo commented
on Oct 20, 2016 jfoomannequinMannequinAuthorIs bpo-24781 likely to make it into Python 3.6? Otherwise, would this patch be of any benefit in the meantime?
I have not been working on IDLE for the last month, but may get back to it. Patched like this can go in any release, not just .0 version releases. (See PEP-434 for why.)
Your ping got me to look at the doc for tk_optionMenu, which is a function rather than a widget (http://www.tcl.tk/man/tcl8.6/TkCmd/optionMenu.htm), the code for tkinter.OptionMenu (the only accurate doc I know of), which is a widget with the function API, the code for DynOptionMenu(OptionMenu), which is both too general-purpose and too special-cased, the doc (and a bit of code) for ttk.Combobox, and finally your patch. Conclusion: I really want to replace DynOptionMenu with Combobox, but not with the current patch.
Micro-issue: it is too late to add anything to tkinter. In any case, it should be a separate issue, with possible a check for anything else that is missing. IDLE can easily define the constant if needed, though I prefer literal strings myself.
Real issue: no tests. Tests for a new class, if any, can await finalization of the API. However, I want to have automated tests for configdialog before making substantive changes. (Justin, what OS are you working on?)
A plus for the patch is that aside from inessential name changes, it is nearly a drop-in replacement. I could *almost* be persuaded that it would be safe to just do it. However, this is a result of perpetuating what I consider to be an awful API. I will come back to this, but on to the methods.
Enable/disable: I am not a fan of trivial half-line functions. "Cb.enable()" would be the same as "Cb['state'] = 'readonly', Beside this, the cb for font size should perhaps eventually be 'normal' so users can type in a font size. The current patch will not apply cleanly because I since augmented the list with sizes needed for code projection on classroom screens. That itself was a substitute for entry or other adjustmen method.
On_select: I believe "self.variable.set(self.get())" should be redundant, so that '<<ComboboxSelected>>' can be bound directly to command when there is one (currently only one case, but I might want to change this.
Reset: passing a display string should not be options, and IDLE always passes one. 'self.variable.set(selection)' should be redundant. This leaves two lines to set the entry and the option list. For one or two boxes, a wrapper function would not be worthwhile. But for multiple instances, it might be, whether as helper function or wrapper class method.
.__init__: IDLE never passes anything for '*values' and always passes
None for 'value'. This ends up as the default '' for the current value. Both parameters and the corresponding lines can go. So can 'self.enable': the initial state should be added to kwargs. If 'variable' is needed, it should be passed in the normal widget way: 'textvariable=mystringvar'. 'self.set(variable.get())' is redundant because 'variable is initially '' and so it the entry.But are textvariables needed? Not for setting and getting Combobox entry values. IDLE uses them to trace changes to the current selection. But with Combobox, the change functions could be bound to '<<ComboboxSelected>>' instead of the Var tracer. I think I would prefer this, as the tracer bindings have to be explicitly removed separately from .destroy().
Do I want this binding combined with initialization? Currently the change handler bindings are done in one function. There is a lot of duplication, and I am working to factor some of it out. (But I need tests before applying.) So I may want to leave them together, which means no wrapper class.
Testing a dialog requires two things: simulating user actions and capturing the result. The first depends on the widget, so it makes sense to focus on testing one widget class (like Combobox) rather than, say, one pane with mixed widget types. The second requires a mock that captures the changes sent to the config database when [Ok] or [Apply] are invoked.
I did some experiments that cleared up some previous puzzlements about seemingly inconsistent behavior between interactive commands and those in scripts. The following prints '.mycombo' twice, on two lines, after selecting 'beta'.
import tkinter as tk from tkinter import ttk root = tk.Tk() #root.withdraw() cb = ttk.Combobox(root, name='mycombo', values=('alpha', 'beta')) cb.pack() def here(event): print(event.widget) cb.bind('<<ComboboxSelected>>', here) cb.set('none') root.update() cb.event_generate('<Button-1>') root.update() cb.event_generate('<Key-Down>') root.update() cb.event_generate('<Key-Return>') root.update() cb.event_generate('<<ComboboxSelected>>') root.update()
cb.set does *not* trigger ComboboxSelected. Key_Return after Button-1 does. So does directly generating the event.
Uncomment .withdraw() and Key-Return does not trigger. But the directly generating the pseudoevent still works. About a month ago, I added .withdraw to tests to stop (obnoxious) screen flashes. I don't call it for shell interaction. Removing at least some of the .update()s will also stop triggering. In IDLE's interactive mode, tcl update is called 20 times a second, which is at least once per statement types and entered. Hence scripted and interactive statements had different effects.
Directly simulating user actions is more satisfying than generating the pseudoevent, but IDLE tests do not need to verify that Combobox works as advertised -- there is a ttk gui test for that. So I believe set('value') + generate('ComboboxSelected') should be sufficient on the input end (after conversion to Combobox and event binding).
I did more experiments with DOM (DynamicOptionMenu) and Combobox (CB). This was partly to evaluate CB as replacement for DOM for font size; partly to consider CB for the font list. At least by default, both start closed. Font list should start open. CB has an Entry area, DOM does not. They get focus by clicking. Both open with Down or click on Down button. DOM also opens on Up, not essential. CB adds scrollbar if needed. DOM? Needed for font list. Up and Down move the selection highlight but do not select. Selection by click and <Return> close the box and generate <<ComboboxSelect>> This is unlike Listbox where moving highlight generates <<ListboxSelect>>. bpo-30870 fixed code so that Up and Down, in addition to click, caused sample change. I want to keep this for font choice and add it for size choice. For CB, click on Down button closes without changing selection. For DOM, it does not, a deficiency.
jfoo commented
on Jul 15, 2017 jfoomannequinMannequinAuthorThanks for analysing further, Terry. My original goal was a drop-in replacement for an aspect of IDLE I found quite annoying (on Windows).
Unfortunately, my limited playing with tkinter didn't inspire me to go beyond this.
FYI, I believe that the ttk.OptionMenu is essentially the same as the DynOptionMenu. ttk.OptionMenu added a
set_menuwhich is the same code as DynOptionMenu.SetMenu, except the options are radiobuttons (which marks the current item.)The only difference is that DynOptionMenu has a highlightthickness, but this isn't directly available on any widgets in ttk.
One other thing to consider is a Spinbox for the fontsize and maybe for the indentsize. There is a ttk Spinbox in the ttk documentation, but it doesn't appear to be implemented in Python.
https://www.tcl.tk/man/tcl/TkCmd/ttk_spinbox.htm
Maybe that should be added to ttk? Or maybe it didn't change from tkinter so that's why it wasn't added?
roseman commented
on Aug 7, 2017 rosemanmannequinMannequinCheryl, regarding the spinbox, as per http://www.tkdocs.com/tutorial/morewidgets.html#spinbox, the ttk version appeared in Tk 8.5.9, which might explain it's absence in tkinter.
A wrapper isn't too hard to do of course... e.g. https://stackoverflow.com/questions/30783603/create-new-ttk-widget-from-tkinter
Maybe there should be a bug tracker issue to add it to ttk.py?
roseman commented
on Jun 26, 2018 rosemanmannequinMannequinGiven the difference between the old and new (ttk) spinbox, especially on macOS, I'd like to incorporate it into IDLE when available. See screenshot spinbox.png, noting white border around old one.
Terry, can we add a spinbox wrapper to IDLE for the time being? If so, would you prefer it done as its own (very little) module? Or is there any value to adding a module to hold various small UI wrappers and convenience procs?
Yes, we should use ttk.Spinbox: bpo-33962.
- changed the title
[-]Retire DynOptionMenu with a ttk Combobox[/-][+]IDLE: replace DynOptionMenu with a ttk Combobox or OptionMenu[/+]on Jan 27, 2023 All current uses of DynOptionMenu are in configdialog. The only use of 'highlightthickness' is for the color-select target list (normal text, etc). I cannot see that it makes much of any difference, as the instances are left out of tab navigation. (And Enter does not select highlighted items anyway.) The only other kwargs passed for other instances (default and custom lists for highlights and keysets) are
command=None, which I presume is default.
Metadata
Metadata
Assignees
Projects
- Status
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: