Qt: Delay user confirmation of send #8012

pull Tyler-Hardin wants to merge 1 commits into bitcoin:master from Tyler-Hardin:send-delay changing 2 files +71 −4
  1. Tyler-Hardin commented at 11:34 pm on May 5, 2016: contributor

    I made a subclass of QMessageBox that disables the send button in exec() and starts a timer that calls a slot to re-enable it after a configurable delay.

    I went with 3s for the delay.

    This PR attempts to implement this feature request: #785.

  2. jonasschnelli added the label GUI on May 6, 2016
  3. in src/qt/sendcoinsdialog.cpp: in 2190e07930 outdated
    311@@ -311,10 +312,10 @@ void SendCoinsDialog::on_sendButton_clicked()
    312     questionString.append(QString("<span style='font-size:10pt;font-weight:normal;'><br />(=%2)</span>")
    313         .arg(alternativeUnits.join(" " + tr("or") + "<br />")));
    314 
    315-    QMessageBox::StandardButton retval = QMessageBox::question(this, tr("Confirm send coins"),
    316-        questionString.arg(formatted.join("<br />")),
    317-        QMessageBox::Yes | QMessageBox::Cancel,
    318-        QMessageBox::Cancel);
    319+    SendConfirmationDialog confirmationDialog(tr("Confirm send coins"),
    320+        questionString.arg(formatted.join("<br />")), 3000, this);
    


    jonasschnelli commented at 8:04 am on May 6, 2016:
    Maybe use a constant for the timeout (3000)?
  4. laanwj commented at 8:06 am on May 6, 2016: member
    Concept ACK 2190e07
  5. in src/qt/sendcoinsdialog.cpp: in 2190e07930 outdated
    839+}
    840+
    841+int SendConfirmationDialog::exec()
    842+{
    843+    button(QMessageBox::Yes)->setEnabled(false);
    844+    QTimer::singleShot(msecDelay, this, SLOT(enableSendButton()));
    


    jonasschnelli commented at 8:07 am on May 6, 2016:
    What happens if you close the QMessageBox before the QTimer fires? I have tested it and it works here… but wondering if it would be better to explicit invalidate the timer in the destructor.

    Tyler-Hardin commented at 8:59 am on May 6, 2016:

    @jonasschnelli, I don’t think it’s possible to invalidate a singleShot. There’s never any timer exposed for me to invalidate. It functions like the static QMessageBox::info/warning/etc functions.

    I would expect that Qt planned for the possibility of the connected object disappearing, seeing as it could happen often. A quick google turns up that signals are automatically disconnected: http://stackoverflow.com/questions/10570857/are-signals-in-qt-automatically-disconnected-when-one-of-the-class-is-deleted


    jonasschnelli commented at 9:08 am on May 6, 2016:
    Okay. Fair enough. Thanks for the clarification.
  6. jonasschnelli commented at 8:09 am on May 6, 2016: contributor

    Somehow I like this but I could imagine confused users during the 3 second timeout (“why the heck is ‘Ok’ button disabled?!”). What about displaying the remaining seconds? Overkill?

    Anyways: this is great work! Thanks.

  7. jonasschnelli commented at 8:10 am on May 6, 2016: contributor
    Needs squashing (at least remove last merge commit).
  8. MarcoFalke commented at 8:20 am on May 6, 2016: member

    displaying the remaining seconds?

    Maybe just a clock icon?

  9. jonasschnelli commented at 8:20 am on May 6, 2016: contributor

    Maybe just a clock icon?

    Good idea!

  10. Tyler-Hardin commented at 9:01 am on May 6, 2016: contributor
    @jonasschnelli @MarcoFalke what about a wait cursor? That would be the easiest to implement by far. Only 2 lines.
  11. jonasschnelli commented at 9:18 am on May 6, 2016: contributor
    @Tyler-Hardin Not sure if wait cursors are still make since today (haven’t seem them on my OSX system a while ago IIRC). But it would be better then noting.
  12. MarcoFalke commented at 9:25 am on May 6, 2016: member
    Agree with @jonasschnelli. Today, changing the mouse icon is not a thing to do anymore. Instead, the spinner/clock icon is displayed in place (inside the button). This makes clear what exactly is blocking the ui.
  13. paveljanik commented at 9:38 am on May 6, 2016: contributor
    Concept ACK Visual indication of the “sleep” would be nice.
  14. Tyler-Hardin force-pushed on May 6, 2016
  15. Tyler-Hardin commented at 10:51 am on May 6, 2016: contributor

    Squashed.

    And I added an animated clock on the yes button.

  16. jonasschnelli commented at 11:04 am on May 6, 2016: contributor
    Nice. Looks good!
  17. in src/qt/sendcoinsdialog.cpp: in 59e23c63a1 outdated
    853+
    854+    animateStage = 0;
    855+    QTimer::singleShot(msecDelay / 4, this, SLOT(animate()));
    856+    QTimer::singleShot(msecDelay / 2, this, SLOT(animate()));
    857+    QTimer::singleShot(3 * msecDelay / 4, this, SLOT(animate()));
    858+    QTimer::singleShot(msecDelay, this, SLOT(enableSendButton()));
    


    jonasschnelli commented at 11:06 am on May 6, 2016:
    nit: use a repetitive Timer with interval msecDelay/4.

    Tyler-Hardin commented at 11:32 am on May 6, 2016:
    Fixed.
  18. jonasschnelli commented at 11:06 am on May 6, 2016: contributor
    Tested ACK 59e23c63a14d275b6b787256b53f782f0911e8a1 We could argue about the icon.
  19. paveljanik commented at 11:07 am on May 6, 2016: contributor

    Can you hide the icon at the end of animation? Or what is the UI style for this?

    But anyway, looks good 👍

  20. MarcoFalke commented at 11:11 am on May 6, 2016: member

    Maybe just steal some MIT spinner from elsewhere? https://github.com/snowwlex/QtWaitingSpinner

    Putting such a spinner in a /util dir could make it easier to re-use later?

  21. Tyler-Hardin commented at 11:26 am on May 6, 2016: contributor
    @MarcoFalke that would look good, but I have no idea where to start in terms of getting that integrated into the build process. @paveljanik I’ll change it. I had left it in because the button shrinks when it goes away. No style. (I’m guessing you’re hinting that I could prevent the shrink with a well designed style. I don’t have any idea where to start with that either.)
  22. Tyler-Hardin force-pushed on May 6, 2016
  23. jonasschnelli commented at 11:40 am on May 6, 2016: contributor
    Lets not bloat the codebase with a progress indicator (should be provided by Qt/OS IMO). We should try to keep the scope of this PR and add graphical improvements over a different PR (if required).
  24. fcbga commented at 11:54 pm on May 6, 2016: none

    Firefox used to have a similar feature in a dialog that asked the user to confirm they accepted the installation of a plugin from the internet. The “Install” button text was replaced with a timer that counted down while the button was disabled, making it obvious to the user something was going to happen without the need of any other visual indicator.

    firefox

  25. laanwj commented at 9:02 am on May 7, 2016: member

    Good porint @fcbga . Yes I think a countdown on the button is fine. No strong need for an icon.

    Reusing the transaction confirmation icons as a progress indicator is inventive, but it may also be confusing.

  26. Qt: Delay user confirmation of send
    I made a subclass of QMessageBox that disables the send button in
    exec() and starts a timer that calls a slot to re-enable it after a
    configurable delay.
    
    It also has a countdown in the send/yes button while it is disabled
    to hint to the user why the send button is disabled (and that it is
    actually supposed to be disabled).
    3902a291ab
  27. Tyler-Hardin force-pushed on May 10, 2016
  28. Tyler-Hardin commented at 2:29 am on May 10, 2016: contributor
    @laanwj, done. Changed to have a text countdown.
  29. laanwj commented at 8:29 am on May 10, 2016: member
    Works for me, tested (but not deeply code-reviewed) ACK 3902a29
  30. jonasschnelli commented at 8:33 am on May 10, 2016: contributor
    Tested ACK 3902a291abecad4da25d496f0e3e2b45f43a7947
  31. jonasschnelli merged this on May 10, 2016
  32. jonasschnelli closed this on May 10, 2016

  33. jonasschnelli referenced this in commit b33824b76c on May 10, 2016
  34. MarcoFalke commented at 9:23 am on May 10, 2016: member

    Looks great. Thanks for sticking with this.

    Tested post-merge ACK

  35. codablock referenced this in commit 72ebac8a5b on Sep 16, 2017
  36. codablock referenced this in commit b98562ed7b on Sep 19, 2017
  37. codablock referenced this in commit 262e857db9 on Dec 21, 2017
  38. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-05 04:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me