Skip to content
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

qt, wallet: Revamp SendConfirmationDialog #15886

Merged
merged 3 commits into from
Jun 6, 2019

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Apr 24, 2019

Fix #15667

With this PR:
Screenshot from 2019-04-24 23-47-30
Screenshot from 2019-04-24 23-47-40

The property-based API has been used. Added support for the
`informativeText` and `detailedText` properties.
@DrahtBot DrahtBot added the GUI label Apr 24, 2019
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 2ee756f, TIL detailedText!

This is enough to fix #15667 and in a follow the detailed text can be improved. For instance, there's no point printing "from wallet" for each recipient I think.

@jonasschnelli
Copy link
Contributor

Nice work!
I also wasn't aware of QMessageBox::setDetailedText().
utACK 2ee756f

@jonasschnelli
Copy link
Contributor

jonasschnelli commented May 18, 2019

Tested a bit. I think the "show details..." approach should only appear if we have multiple recipients. But I'm open to be convinced otherwise.

@hebasto
Copy link
Member Author

hebasto commented May 18, 2019

@promag @jonasschnelli
Thank you for your reviews.

I think the "show details..." approach should only appear if we have multiple recipients.

Nice. Done.

@hebasto hebasto force-pushed the 20190424-send-confirmation-dialog branch from 65f3a7c to 5d85328 Compare May 18, 2019 19:17
@hebasto hebasto force-pushed the 20190424-send-confirmation-dialog branch from 5d85328 to 78f9b51 Compare May 18, 2019 19:22
@laanwj
Copy link
Member

laanwj commented Jun 6, 2019

code review ACK 78f9b51

@laanwj laanwj merged commit 78f9b51 into bitcoin:master Jun 6, 2019
laanwj added a commit that referenced this pull request Jun 6, 2019
78f9b51 Do not show list for the only recipient. (Hennadii Stepanov)
2ee756f Show recipient list as detailedText of QMessageBox (Hennadii Stepanov)
654e419 Make SendConfirmationDialog fully fledged (Hennadii Stepanov)

Pull request description:

  Fix #15667

  With this PR:
  ![Screenshot from 2019-04-24 23-47-30](https://user-images.githubusercontent.com/32963518/56692672-63400b00-66eb-11e9-87f6-15957c6e81f7.png)
  ![Screenshot from 2019-04-24 23-47-40](https://user-images.githubusercontent.com/32963518/56692681-663afb80-66eb-11e9-8b04-8a342026ada6.png)

ACKs for commit 78f9b5:
  laanwj:
    code review ACK 78f9b51

Tree-SHA512: f868d78d01b0898aff2277fa3a7e8c6f936acbbcfa8a0323cddcd9daba4a998030c667bd803ae67c2b9179ed8082a48a67568e9ba3c8d14e3a2d88d93ada94fa
@hebasto hebasto deleted the 20190424-send-confirmation-dialog branch June 6, 2019 11:40
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 6, 2019
78f9b51 Do not show list for the only recipient. (Hennadii Stepanov)
2ee756f Show recipient list as detailedText of QMessageBox (Hennadii Stepanov)
654e419 Make SendConfirmationDialog fully fledged (Hennadii Stepanov)

Pull request description:

  Fix bitcoin#15667

  With this PR:
  ![Screenshot from 2019-04-24 23-47-30](https://user-images.githubusercontent.com/32963518/56692672-63400b00-66eb-11e9-87f6-15957c6e81f7.png)
  ![Screenshot from 2019-04-24 23-47-40](https://user-images.githubusercontent.com/32963518/56692681-663afb80-66eb-11e9-8b04-8a342026ada6.png)

ACKs for commit 78f9b5:
  laanwj:
    code review ACK 78f9b51

Tree-SHA512: f868d78d01b0898aff2277fa3a7e8c6f936acbbcfa8a0323cddcd9daba4a998030c667bd803ae67c2b9179ed8082a48a67568e9ba3c8d14e3a2d88d93ada94fa
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 1, 2020
Summary:
The property-based API has been used. Added support for the
`informativeText` and `detailedText` properties.

bitcoin/bitcoin@654e419

---

Partial backport of Core [[bitcoin/bitcoin#15886 | PR15886]]

Test Plan:
  ninja

run bitcoin-qt in regtest mode and check things out

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7289
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 1, 2020
Summary:
bitcoin/bitcoin@2ee756f

---

Depends on 7289

Partial backport of Core [[bitcoin/bitcoin#15886 | PR15886]]

Test Plan:
  ninja

send coins with bitcoin-qt -regtest and check it behaves as expected

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7290
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 1, 2020
Summary:
bitcoin/bitcoin@78f9b51

---

Depends on D7290

Concludes backport of Core [[bitcoin/bitcoin#15886 | PR15886]]

Test Plan:
  ninja

send coins with bitcoin-qt -regtest and check there's no more detailed option for single recipient

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D7291
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Oct 23, 2021
78f9b51 Do not show list for the only recipient. (Hennadii Stepanov)
2ee756f Show recipient list as detailedText of QMessageBox (Hennadii Stepanov)
654e419 Make SendConfirmationDialog fully fledged (Hennadii Stepanov)

Pull request description:

  Fix bitcoin#15667

  With this PR:
  ![Screenshot from 2019-04-24 23-47-30](https://user-images.githubusercontent.com/32963518/56692672-63400b00-66eb-11e9-87f6-15957c6e81f7.png)
  ![Screenshot from 2019-04-24 23-47-40](https://user-images.githubusercontent.com/32963518/56692681-663afb80-66eb-11e9-8b04-8a342026ada6.png)

ACKs for commit 78f9b5:
  laanwj:
    code review ACK 78f9b51

Tree-SHA512: f868d78d01b0898aff2277fa3a7e8c6f936acbbcfa8a0323cddcd9daba4a998030c667bd803ae67c2b9179ed8082a48a67568e9ba3c8d14e3a2d88d93ada94fa
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Oct 26, 2021
78f9b51 Do not show list for the only recipient. (Hennadii Stepanov)
2ee756f Show recipient list as detailedText of QMessageBox (Hennadii Stepanov)
654e419 Make SendConfirmationDialog fully fledged (Hennadii Stepanov)

Pull request description:

  Fix bitcoin#15667

  With this PR:
  ![Screenshot from 2019-04-24 23-47-30](https://user-images.githubusercontent.com/32963518/56692672-63400b00-66eb-11e9-87f6-15957c6e81f7.png)
  ![Screenshot from 2019-04-24 23-47-40](https://user-images.githubusercontent.com/32963518/56692681-663afb80-66eb-11e9-8b04-8a342026ada6.png)

ACKs for commit 78f9b5:
  laanwj:
    code review ACK 78f9b51

Tree-SHA512: f868d78d01b0898aff2277fa3a7e8c6f936acbbcfa8a0323cddcd9daba4a998030c667bd803ae67c2b9179ed8082a48a67568e9ba3c8d14e3a2d88d93ada94fa
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transaction confirmation prompt too big - buttons not clickable
5 participants