-
-
Notifications
You must be signed in to change notification settings - Fork 638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a MessageDialog
API
#17304
base: master
Are you sure you want to change the base?
Create a MessageDialog
API
#17304
Conversation
…logButton` if provided
… rather than normal functions.
It probably is, no section of the guide jumped out at me as being the best place to put the notes though.
That's why I created a new one for the Extension Points chapter.
I suggest you do the same thing here.
A new chapter called "Communicating with the user" or "Communicating to the user" or similar.
We can talk about ui.message and ui.browseableMessage in the future, and the speech/braille specific versions. For now, it can just be your dialog API.
Glad to hear it! It's one of the few features I regularly miss from Java when working in Python.
Ditto. It was one of the (few) things I really liked about Java, and was very disappointed to find it so rarely used in Python.
|
… unpacked tuple of Buttons and DefaultButtonSets
…s a valid EscapeId
Hello Sascha, With gui.messageBox reimplemented on top of gui.messageDialog.MessageDialog, here is the traceback I got when trying to instal an add-on that calls gui.messageBox in its onInstall task function: ERROR - unhandled exception (01:32:50.339) - systemUtils.ExecAndPump(<function installAddonBundle at 0x03AD2668>) (23372): The above exception was the direct cause of the following exception: SystemError: <class 'wx._core.ActivateEvent'> returned a result with an exception set Relevant ad-on code fragment (not the exact code, but is written similar to this): minimumWinVer = winVersion.WinVersion(10, 0, 27800, releaseName="Windows 11 Selenium") Thanks. |
@josephsl I believe the exception you encountered in #17304 (comment) should be resolved now. Would you mind testing again with at least fca7eb8? Regarding the errors encountered when calling |
Hi, The error is resolved. As for main thread detection: perhaps we could use main thread detection. However, it might be possible that an operation is performed right after dismissing this dialog that might block the main thread i.e. the dialog asks to perform a task that could take a long time to do such as network connectivity. Thanks. |
Hi, As a follow-up: the error still happens if gui.messageDialog.MessageDialog.ShowModal() is called directly as proposed in the message dialog notes posted a few days ago, so it might be worth looking into main GUI thread detection. Another way could be reminding folks to wrap ShowModal call inside wx.CallAfter function (or rather, define the dialog and the ShowModal call inside a dedicated function and call that function as an argument of wx.CallAfter). Thanks for considering our feedback in implementingg this PR. |
…nd waiting for their return.
…ageDialog shim (untested)
… (probably more to do still)
Link to issue number:
Closes #13007
Summary of the issue:
Currently, there are several ways to create message boxes/dialogs in NVDA, all of them with their own drawbacks.
Additionally, subclassing
wx.Dialog
(or using it directly) brings with it the risk of breaking parts of NVDA. See #13007 for in-depth discussion.Thus, a new API for creating highly customizable dialogs that work well with NVDA is desired.
Description of user facing changes
This PR does not, in itself, introduce any end-user facing changes. However, it lays the groundwork for closing a number of issues.
Description of development approach
Subclassed
wx.Dialog
to add the desired functionality in #13007Features and tasks
Quits from keyboard with about dialog open
is re-enabled and works.MessageDialog
s open.MessageDialog
s without default actions when exiting.gui.MessageDialog
togui.message
.gui.message.messageBox
gui.nvdaControls.MessageDialog
gui.nvdaControls._ContinueCancelDialog
Testing strategy:
Unit, system and manual tests (TBD)
Message box shim tested with a variety of inputs and unittests. This is a fairly straightforward adapter.
nvdaControls.MessageDialog
tested that screen curtain and add-on install warning dialogs still work as expected.Known issues with pull request:
Internal dialogs are yet to be migrated. This should be done gradually before the existing means of creating message dialogs are removed.
Slightly odd behaviour when a callback function blocks, and
focusBlockingInstances
is called, where a hidden dialog is focused. This can be worked around, but possibly not without unintended consequences.Some invalid combinations (namely a button which is set as the default action but set not to close the dialog) are silently changed. While documenting this should be sufficient, logging or raising an exception may be warranted.
Visual proportions of the re-implemented About dialog are different.
The
ctrl+c
behaviour of copying the dialog's title, contents and buttons is not supported.Code Review Checklist:
@coderabbitai summary