Bugfix: GUI: Restore SendConfirmationDialog button default to "Yes"#17463
Bugfix: GUI: Restore SendConfirmationDialog button default to "Yes"#17463luke-jr wants to merge 3 commits intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
I think it still does make some sense, as in 'send bumped transaction'. |
|
The prompt could be rephrased to make sense with "Send", but IMO it currently isn't. More than just button text is needed for #15987 so might as well just get the interface change over with all at once. |
|
Tested ACK d6adf3bc2db17ed3a17caa2ac5cf9095d70b53fb Before: With this PR:
|
|
Needs a small rebase due to #16944 getting merged. |
|
utACK; I'm looking to build on #17509 to which this is mentioned as a prereq, so if this is not too controversial I'd love to see it go in soon. It seems pretty small. |
luke-jr
force-pushed
the
gui_custom_sendyes
branch
from
d6adf3b to
3502340
Compare
|
Rebased |
|
As @gwillen points out inline, I'm able to drop this entire code block with no visible change. At least not on macOS with QT 5.13.2: // We need to ensure the buttons have Yes/No roles, or they'll get ordered weird
QAbstractButton * const cancel_button_obj = button(cancel_button);
removeButton(cancel_button_obj);
addButton(cancel_button_obj, QMessageBox::NoRole);
setEscapeButton(cancel_button_obj);
setDefaultButton(cancel_button);
Otherwise 350234028786a139b7faf08655f27d97c9ff66e8 looks good. |
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
Sorry but I really dislike the "super" constructor - I don't see how that's preferable over calling a couple of setters, which is more expressive and obvious.
It's also unfortunately that it needs to remove/add buttons to fix the order.
luke-jr
force-pushed
the
gui_custom_sendyes
branch
from
d00f5db to
b728f99
Compare
|
This needs a rebase now that #17509 (Save / Load PSBT is merged). |
|
Approach ACK. |
Both buttons can be replaced with other standard buttons
The SendConfirmationDialog is used for bumping the fee, where "Send" doesn't really make sense
|
Rebased |
luke-jr
force-pushed
the
gui_custom_sendyes
branch
from
b728f99 to
b460385
Compare
|
Let move this over to the GUI repo. |
…t to "Yes" 8775691 Bugfix: GUI: Restore SendConfirmationDialog button default to "Yes" (Luke Dashjr) Pull request description: The SendConfirmationDialog is used for bumping the fee, where "Send" doesn't really make sense Originally bitcoin/bitcoin#17463, but rewritten here much simpler based on other merged changes. ACKs for top commit: hebasto: ACK 8775691, tested on Linux Mint 20.1 (x86_64, Qt 5.12.8): Tree-SHA512: 3953cc9c09613c9a629def8b4dc061b537f148ddcb378430645602e0be0f3a9f1cff083aa685b94b2e9372300d02ec97e0d9ea89db6e3c6feec86795090f0f77
9 participants
The SendConfirmationDialog is used for bumping the fee, where "Send" doesn't really make sense
Customisation of the dialog is also needed for #16944 and #15987 so might as well use it for this too.