Conversation
jorio
left a comment
jorio
left a comment
There was a problem hiding this comment.
Here's a test for Ctrl+Return in CommitDialog:
(Tip: you can use test.py --visual -k nameofyourtest and sprinkle the test with pause() calls to see what you're doing as you're writing your unit test)
def testCommitDialogCtrlReturn(tempDir, mainWindow):
wd = unpackRepo(tempDir)
rw = mainWindow.openRepo(wd)
triggerMenuAction(mainWindow.menuBar(), r"repo/commit")
acceptQMessageBox(rw, "create an empty commit anyway")
commitDialog: CommitDialog = findQDialog(rw, r"commit")
# Widget focus is finicky in offscreen tests unless we force the dialog to be active first
commitDialog.activateWindow()
waitUntilTrue(commitDialog.isActiveWindow)
commitDialog.ui.summaryEditor.setText("hello from ctrl+return")
# Make sure the shortcut works even when a QPlainTextEdit (multiline editor) has focus
commitDialog.ui.descriptionEditor.setFocus()
assert commitDialog.ui.descriptionEditor.hasFocus()
QTest.keySequence(commitDialog.focusWidget(), "Ctrl+Return")
assert rw.repo.head_commit_message.strip() == "hello from ctrl+return"I don't know if it's worth testing just "Return", because offscreen tests behave like KDE regardless of the host system's environment, so it wouldn't make a difference.
|
The test appears to be flaky in offscreen mode, but I think this will solve it: (Some of the CI jobs failed because of outdated APT packages on the CI runners, that's an unrelated problem) |
|
This rabbit hole keeps getting deeper... I never noticed before, but KDE already lets you click "OK" by hitting Ctrl+Return, in any dialog, in any Qt application! KDE seems to be adding a QShortcut("Ctrl+Return") to any dialog's default button after QDialog.show(). The kicker is, if create our own Ctrl+Return shortcut on top of that, it will conflict with KDE's own shortcut. Pressing Ctrl+Return will emit the My suggestion is to move the installDialogReturnShortcut calls after dialog.show() so KDE gets to install its shortcuts first. Then, in installDialogReturnShortcut, see if okButton has any shortcuts already. If it does, we're likely running on KDE so we can just bail. For example: if okButton.findChildren(QShortcut): # Don't conflict with KDE's shortcuts
returnDoes this approach still work in GNOME? |
|
I merged your changes manually and made a few tweaks for KDE compat. Sorry for hijacking the PR and thanks for your contribution! |
2 participants
Second take on #66. Apparently Return is supposed to work on these dialogues, but it doesn't quite work on GNOME. This makes it work everywhere.
I added both Return and Ctrl+Return. It seems like Qt cleverly figures out that Return in a multi-line text box (such as a long commit message) should not activate a shortcut on the parent widget, and in this case, it's nice for Ctrl+Return to also work.
@jorio, do you know if we can test shortcuts?