gui: Ensure transaction send error is always visible #16694

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:rebased_squashed_14956 changing 1 files +2 −1

fanquake commented at 11:42 AM on August 23, 2019: member

Rebased and squashed #14956.

If sending to multiple recipients and one of the recipient fields is malformed, the highlighted field may not be visible due to being scrolled out of view. This results in a confusing lack of error feedback.

Avoid this problem by ensuring the first field containing an error is scrolled into view when Send is clicked.

You can see the behavior here: https://imgur.com/a/QZG5TQc

How to test: Add a few recipients and give any of them an invalid address or amount. Scroll the invalid recipient out of view and hit Send. With this change, the GUI will scroll to show the invalid recipient, with master it will not, "hiding" the error.

gui: Ensure tx send error highlight is visible

If sending to multiple recipients and one of the recipient fields is malformed,
the highlighted field may not be visible due to being scrolled out of view,
leading to a confusing lack of error feedback when clicking Send. To avoid this
problem ensure the first field containing an error is scrolled into view
when Send is clicked.

a4765bd77f

fanquake added the label GUI on Aug 23, 2019
jonasschnelli commented at 12:39 PM on August 23, 2019: contributor

Screenshots would be great for this.
fanquake commented at 12:46 PM on August 23, 2019: member

@jonasschnelli It'll be hard to convey the change in behavior using screenshots, given they are a movement in the GUI, but I can try and take a video.

However testing the changes is trivial. Just add a few recipients, and give any of them an invalid address or amount. Scroll the invalid recipient out of view and hit Send. With this change, the GUI will scroll to show the invalid recipient, with master it will not, "hiding" the error.
jonatack commented at 12:53 PM on August 23, 2019: member

Hm, just tested the opposite. 5 recipients, then make first one have an invalid address or amount of 0, scroll down, click send, and the invalid one remained above and out of view e.g. no scrolling happened.

Linux Debian, and not working for me. Maybe it will for someone else?
fanquake commented at 12:55 PM on August 23, 2019: member

Updated PR body with a link to a video showing the scrolling behavior.
hebasto commented at 1:08 PM on August 23, 2019: member

Concept ACK

in src/qt/sendcoinsdialog.cpp:233 in a4765bd77f

 229 | @@ -230,8 +230,9 @@ void SendCoinsDialog::on_sendButton_clicked()
 230 |              {
 231 |                  recipients.append(entry->getValue());
 232 |              }
 233 | -            else
 234 | +            else if (valid)

hebasto commented at 2:14 PM on August 23, 2019:

nit: formatting

            else if (valid) {

fanquake commented at 12:52 AM on August 24, 2019:

This matches the style of the surrounding code and has two ACKs, so I'm going to leave it as is.

hebasto approved
hebasto commented at 2:15 PM on August 23, 2019: member

ACK a4765bd77f9bc5c4d5c866ee6940807522741941, tested on Debian 9.9 with system Qt 5.7.1. @jonatack

Hm, just tested the opposite. 5 recipients, then make first one have an invalid address or amount of 0, scroll down, click send, and the invalid one remained above and out of view e.g. no scrolling happened.

Scrolling to the error works for me. @jonatack what Debian and Qt version do you use?
jonatack commented at 8:47 PM on August 23, 2019: member

Tested ACK with Qt 5.13.0 on macOS.

FWIW quite a few Qt deprecation warnings during the build; #16701 makes sense.

Will try again on Debian and watch the build... :eyes:
jonatack commented at 9:32 PM on August 23, 2019: member

Tested ACK a4765bd77f9bc5c4d5c866ee6940807522741941 on Linux Debian with Qt 5.11.3. Change is that I had added an inadvertent typo in my make bash alias; fixed.
fanquake referenced this in commit d86a906a88 on Aug 24, 2019
fanquake merged this on Aug 24, 2019
fanquake closed this on Aug 24, 2019
fanquake deleted the branch on Aug 24, 2019
jasonbcox referenced this in commit ac424d9593 on Oct 14, 2020
PastaPastaPasta referenced this in commit 33fd172c53 on Jun 27, 2021
PastaPastaPasta referenced this in commit 81204fcb78 on Jun 28, 2021
PastaPastaPasta referenced this in commit d8f5b1e167 on Jun 29, 2021
PastaPastaPasta referenced this in commit ed0722bfd5 on Jul 1, 2021
PastaPastaPasta referenced this in commit 448d64dd6b on Jul 1, 2021
PastaPastaPasta referenced this in commit 66efe7bcb0 on Jul 12, 2021
PastaPastaPasta referenced this in commit ae01429338 on Jul 13, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#14956 [qt] Ensure tx send error highlight is visible