classification
Title: Finish IDLE Query dialog appearance and behavior.
Type: behavior Stage: test needed
Components: IDLE Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: markroseman, python-dev, serhiy.storchaka, steven.daprano, terry.reedy
Priority: normal Keywords: patch

Created on 2016-07-26 06:02 by serhiy.storchaka, last changed 2017-06-30 00:47 by terry.reedy.

Files
File name Uploaded Description Edit
idle_query_press_return.patch serhiy.storchaka, 2016-07-26 07:26 review
dlgonmac.png markroseman, 2016-07-29 21:46 snapshot of open module dialog running on mac showing background glitches
query.patch markroseman, 2016-08-01 17:55 review
query2.diff terry.reedy, 2016-08-04 02:09 put error messages in query widget review
Messages (25)
msg271326 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-26 06:02
When open new Query dialog in IDLE (e.g. "Open Module" dialog), move a focus on the Cancel button and press <Return> on keyboard, the OK button is invoked instead of Cancel.
msg271341 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-26 07:11
While writing #27620 and adding a binding to Query for <Escape>, a few hours ago, I realized that this would be the case, that it could be considered a bug, and that the fix will be a 'return' function that looks at the focus before invoking [Ok] (the default) or [Cancel] (when it has the focus).

    def return_key(self, event):
        if self.focus_get() == self.button_cancel:
            self.cancel()
        else:
            self.ok()

appears to work.  I will push tomorrow after writing a test.  Something similar is needed for config dialog.  I just tested the search dialog and it behaves the same as query does now.  If one tabs enough to put the focus on close, <Return> does find anyway.

Do you agree that Esc should always cancel regardless of where the focus is?  I would appreciate your input on #27620 as to the 'right' behavior we should aim at for the different dialogs.

While I think of it, the indication of ttk.Entry focus with the default Windows theme is a shift of the 1 pixel black border to a 1 pixel blue border.  It is not obvious.  I want to look as other Window's theme or possibly a custom style.  Now a priority yet.
msg271342 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-26 07:12
I made this a dependency of #27620.
msg271344 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-26 07:26
I have wrote similar patch. There are many ways to fix this issue. Tk code for dialog widgets is more complex and general.
msg271358 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2016-07-26 12:08
I expect that ESC should always cancel, and RETURN/ENTER should always accepts (OK or Save or whatever the "main" button is) regardless of where the focus is. If you want a keyboard shortcut to push the button with focus, use SPACE, not ENTER.
msg271360 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-26 12:32
This is against the convention.
msg271464 - (view) Author: Mark Roseman (markroseman) * Date: 2016-07-27 16:59
Side note that on Mac OS X, buttons normally don't get the focus, so that this isn't an issue. Well except that buttons are getting the focus here. :-)  

Also since we're reinventing the wheel, please note that alternative keyboard shortcuts (e.g. command-period) don't work, and there isn't a default button set for the dialog which there should be. 

Interestingly, you can change things in System Preferences -> Keyboard -> Shortcuts so that buttons can get the focus via tabbing through the interface. If this is enabled and you tab to the Cancel button and his Return, it should still be treated as if you hit the Okay button on Mac. Hitting space while focus is on the Cancel button does treat it as if you clicked on Cancel.
msg271481 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-28 01:46
I am not trying to re-invent the wheel.  I am trying to bring IDLE up to uniform standards.  In particular, make all dialogs usable from the keyboard.  The config dialog is the worst. See #27620, the master issue.

For a two button messagebox, I went back to 3.5.2, which still uses the commondialog box.  It has the default button marked as such.  <Return> always does the default action. <Escape> always cancels.  <Tab> moves the focus, but does not affect the above.  <Space> enters ' ' in text entry and 'clicks' buttons.  So this is at least 'a' Windows standard, as well as 'a' Mac standard.

The search dialogs act the same. The space behavior built-in to the TK widgets, at least on Windows. I presume this is a class binding.  The dialogs have no platform specific code, so they are also the defacto IDLE standard.

Serhiy, it is definitely a tk convention, and I presume more universal than that, that the 'default ring' always indicate what button/action is invoked by <Return>.  If the default moved with the focus, the default ring should move also.  I am not inclined to do that without a strong reason.

The button default ring is controlled by the 'default' option" 'active' = visible, 'normal' = possible, 'disabled' = not possible.  (From the tk docs.  The NMT reference is useless here.) I will add "default='active'" for the Ok button.

(Mark: "default=1", suggested on http://www.tkdocs.com/widgets/button.html, is a nasty bug.  "_tkinter.TclError: bad default "1": must be normal, active, or disabled".)
msg271482 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-28 01:49
I meant to direct the focus ring fix here instead #27620.  To me, this issue is done, at least for now.
msg271652 - (view) Author: Mark Roseman (markroseman) * Date: 2016-07-29 21:46
Terry, thanks for the TkDocs correction.

As you'll note from the attached dlgonmac.png, there's a bit of tweaking needed with regard to geometry management etc. to get the background right.

Now that ttk is ok (so to speak), would you be open to some patches that fix this up, a bit more akin to what you see in the 'query dialog' subsection if you scroll down a bit in http://www.tkdocs.com/tutorial/idle.html#idledialogs

Separately, would you be open to a patch changing things to use the "inline" error handling illustrated on the goto line dialog on that page (i.e. showing error message in query dialog in red vs. popping up an alert)?

Older code for that can be found here btw: https://github.com/roseman/idle/blob/master/querydialog.py
msg271663 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-30 04:15
In Tk widgets tk_dialog and tk_messageBox (corresponding Tkinter widgets dialog.Dialog and messagebox.Message) pressing Return invokes focused button. The comment in tk_dialog implementation:

    # 4. Create a binding for <Return> on the dialog if there is a
    # default button.
    # Convention also dictates that if the keyboard focus moves among the
    # the buttons that the <Return> binding affects the button with the focus.

I expected the same behavior in IDLE Query dialog. If this behavior considered outdated, I'm nice to close this issue.
msg271753 - (view) Author: Mark Roseman (markroseman) * Date: 2016-07-31 23:15
Serhiy, the tk_dialog has been superseded by tk_messageBox, and does not reflect current platform standards.  I just tried tk_messageBox on the Mac, which always activates the default button if you press 'return', even if another button has the focus. I expect Windows and X11 are different, and will check that out when I get a chance.
msg271756 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-01 00:51
Background: For Open Module, the old commondialog did not have the asymmetrical drop shadow.  That is an unintentional addition (from a frame?) and I don't like it.  Perhaps we should use grid instead.  Rows 0 and 1 for the initial label and entry, with columnspan 2, rows 8 and 9 for error message and buttons.  This leaves room for insertion of path message, path entry, and browse button.  Patch welcome.

A definite yes for error label in widget.  I dislike the popup.  Begone!  Should a blank label reserve space or should it be inserted as needed, making box expand.
msg271757 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-01 00:54
Serhiy: I believe Notepad, Wordpad, Libre Office Write behave as you describe.  But there are differences in details. So I don't consider issue closed, but it is low priority to me.
msg271761 - (view) Author: Mark Roseman (markroseman) * Date: 2016-08-01 01:57
Just to follow up, both Windows and Linux the 'correct' behaviour seems to be that space or return activates the button with the current focus. Mac behaves differently in that return key always activates default button even if focus is on another button (and normally on Mac, buttons don't get focus).

I'll put together a patch that cleans up the layout and does the error label in dialog thing.
msg271789 - (view) Author: Mark Roseman (markroseman) * Date: 2016-08-01 17:55
I've attached query.patch, which does the cosmetic and layout changes, and adds a couple Mac-specific things. I've added the inline error message widget but don't use it yet (as this will involve changes to the subclasses and the tests, given errors will show up when the dialog is running, not after).

Given I'm a bit rusty at this, would appreciate if someone could check this out and make sure I did things correctly. :-)
msg271870 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-03 02:35
The file has the correct format for review, applies cleanly, and tests pass.  The relief is gone (easy).  The buttons are no longer symmetrical; this looks 'wrong' to me.  (My reaction might be different if the widget was much wider, so the buttons were clearly right-justified rather than looking like their placement had a bug.)  This results from using 3 columns instead of 2.  I want to try the latter.   I will switch error messages to the widget, see what happens, and deal with needed test changes.  I will post before committing so you can test on Mac first.
msg271930 - (view) Author: Mark Roseman (markroseman) * Date: 2016-08-03 21:06
Thanks Terry! I'd be good if you want to put a width back on the buttons, but I'd suggest "width=6" rather than using 8 as it was before.
msg271937 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-04 02:09
I changed showerror from widget to query function, added a second error label to Help Source, adjusted validation functions, and rewrote tests.  I also made keypad enter work on all systems (it already works in Search and I use it often).  I want to look at spacing in a separate pass.
msg272073 - (view) Author: Mark Roseman (markroseman) * Date: 2016-08-05 23:38
Looks great Terry - thanks. Only nit is that test_click_help_source fails on Mac, courtesy a leading 'file://' added in the last few lines of path_ok
msg272220 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-09 07:15
See #27714 for more on the test failure.
msg272221 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-09 07:19
Actually, the query test failure is posted to #27380
msg272341 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-10 16:51
New changeset f0e86b60de5f by Terry Jan Reedy in branch 'default':
Issue #27621: Put query response validation error messages in query box
https://hg.python.org/cpython/rev/f0e86b60de5f
msg272342 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-10 16:56
There is still Serhiy's original issue about moving <Return> default with focus, plus possible appearance changes.
msg297337 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-06-30 00:47
#24812 specifically talks about Mac standards.
History
Date User Action Args
2017-06-30 00:47:04terry.reedysetmessages: + msg297337
2017-05-31 05:45:16terry.reedylinkissue27115 dependencies
2016-08-10 16:56:51terry.reedysetmessages: + msg272342
2016-08-10 16:51:21python-devsetnosy: + python-dev
messages: + msg272341
2016-08-09 07:19:57terry.reedysetmessages: + msg272221
2016-08-09 07:15:24terry.reedysetmessages: + msg272220
2016-08-05 23:38:56markrosemansetmessages: + msg272073
2016-08-04 02:10:02terry.reedysetfiles: + query2.diff

messages: + msg271937
2016-08-03 21:06:05markrosemansetmessages: + msg271930
2016-08-03 02:35:24terry.reedysetmessages: + msg271870
2016-08-01 17:55:24markrosemansetfiles: + query.patch

messages: + msg271789
2016-08-01 01:57:47markrosemansetmessages: + msg271761
2016-08-01 00:54:45terry.reedysetmessages: + msg271757
2016-08-01 00:51:10terry.reedysetmessages: + msg271756
2016-07-31 23:15:49markrosemansetmessages: + msg271753
2016-07-30 04:15:40serhiy.storchakasetmessages: + msg271663
2016-07-29 21:46:58markrosemansetfiles: + dlgonmac.png

messages: + msg271652
2016-07-28 01:49:56terry.reedysetmessages: + msg271482
2016-07-28 01:46:17terry.reedysetmessages: + msg271481
title: <Return> incorrectly works in IDLE Query dialogs -> Finish IDLE Query dialog appearance and behavior.
2016-07-27 16:59:37markrosemansetnosy: + markroseman
messages: + msg271464
2016-07-26 12:32:28serhiy.storchakasetmessages: + msg271360
2016-07-26 12:08:40steven.dapranosetnosy: + steven.daprano
messages: + msg271358
2016-07-26 07:26:16serhiy.storchakasetfiles: + idle_query_press_return.patch
keywords: + patch
messages: + msg271344
2016-07-26 07:12:55terry.reedysetmessages: + msg271342
2016-07-26 07:11:34terry.reedysetmessages: + msg271341
stage: test needed
2016-07-26 06:35:48terry.reedylinkissue27620 dependencies
2016-07-26 06:02:46serhiy.storchakacreate