backport: bitcoin-core/gui#509: Respect dialog modality and fix a regression in wallet unlock #6603
Conversation
This change is required for the following commits.
Also dropped useless tooltip.
This change is required for the following commit.
The AskPassphraseDialog modal dialog must be synchronous here as expected in the WalletModel::requestUnlock() function. Fixed an introduced regression.
-BEGIN VERIFY SCRIPT- sed -i 's/ShowModalDialogAndDeleteOnClose/ShowModalDialogAsynchronously/' -- $(git grep -l -e "ShowModalDialogAndDeleteOnClose") -END VERIFY SCRIPT- It is important to highlight that a modal dialog is showed asynchronously as there are cases when the synchronous QDialog::exec() is required.
WalkthroughThe changes refactor how dialogs are displayed and managed within the Qt-based UI. Calls to the function that presented modal dialogs and automatically deleted them upon closure have been replaced with a new asynchronous approach. This update is applied across multiple components, including address management, coin sending, transaction editing, and wallet interactions. In addition, the Bitcoin application now conditionally triggers shutdown detection only when no modal dialog is active. The refactoring also includes renaming several QAction objects for consistency and clarity in the Bitcoin GUI and removing some platform-specific tray icon activation code. In one instance related to wallet unlocking, the dialog is now stack-allocated and executed synchronously. Overall, these modifications aim to improve the control flow and user interaction experience with a more uniform dialog management approach across the application. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (24)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
LGTM but check comment about fForMixingOnly
| coinJoinCoinsMenuAction = new QAction(QString("&%1").arg(strCoinJoinName), this); | ||
| coinJoinCoinsMenuAction->setStatusTip(tr("Send %1 funds to a Dash address").arg(strCoinJoinName)); | ||
| coinJoinCoinsMenuAction->setToolTip(coinJoinCoinsMenuAction->statusTip()); | ||
| coinJoinCoinsAction = new QAction(QString("&%1").arg(strCoinJoinName), this); |
There was a problem hiding this comment.
nit: maybe split this renaming to own commit?
There was a problem hiding this comment.
The original commit bitcoin-core/gui@58e1603 dropped *MenuActions and kept other actions. But we don't have other actions, we already use one action where bitcoin uses two (before this backport). If I'd rename these actions beforehand, then the backported commit would not do what it say it does ("drop members") cause there would be nothing to drop 🤷♂️
src/qt/walletview.cpp
Outdated
| auto dlg = new AskPassphraseDialog(AskPassphraseDialog::Unlock, this); | ||
| dlg->setModel(walletModel); | ||
| GUIUtil::ShowModalDialogAndDeleteOnClose(dlg); | ||
| AskPassphraseDialog dlg(AskPassphraseDialog::Unlock, this); |
There was a problem hiding this comment.
It seems as gui#336 introduced one extra bug: there's missing usage of flag fForMixingOnly
This line should be:
AskPassphraseDialog dlg(fForMixingOnly ? AskPassphraseDialog::UnlockMixing : AskPassphraseDialog::Unlock, this);
5 participants
Issue being fixed or feature implemented
bitcoin-core/gui#336 backported in #6569 broke unlock dialog
What was done?
backport bitcoin-core/gui#509 to fix this
How Has This Been Tested?
Breaking Changes
Checklist: