[Qt] Add out-of-sync modal info layer #8371

pull jonasschnelli wants to merge 7 commits into bitcoin:master from jonasschnelli:2016/07/UI-out-of-sync changing 17 files +708 −29
  1. jonasschnelli commented at 1:57 PM on July 19, 2016: contributor

    Adresses #8060 and #7235

    This change will slide in a semi transparent modal info layer in out-of-sync state resulting in a more prominent warning. Users can hide the modal info layer by pressing on "Hide". Clicking on the out-of-sync warning icons will re-display the modal info layer.

    In the modal info layer, the user can get three new values:

    • amount of blocks left
    • estimated time left until in-sync
    • progress increase per hour

    <img width="948" alt="bildschirmfoto 2016-07-19 um 15 53 16" src="https://cloud.githubusercontent.com/assets/178464/16952302/ed02ca74-4dc8-11e6-95de-e6309f1bd8c5.png">

  2. jonasschnelli added the label GUI on Jul 19, 2016
  3. instagibbs commented at 1:35 AM on July 20, 2016: member

    "Progress increase per hour" is a bit weird sounding to me, maybe "Sync progress per hour"?

  4. laanwj commented at 8:40 AM on July 20, 2016: member

    Looks very nice!

  5. laanwj commented at 9:04 AM on July 28, 2016: member
    • Looks like the font color should not depend on the theme? overlay
    • Maybe add some more text after "...but this process has not completed yet", to make it more concrete "this means that recent transactions will not be visible, and the balance will not be up-to-date until this process has completed."
  6. jonasschnelli force-pushed on Jul 28, 2016
  7. jonasschnelli commented at 9:45 AM on July 28, 2016: contributor

    Added the second info text and made sure, the font will be blackish (force push).

  8. jonasschnelli added this to the milestone 0.14 on Jul 28, 2016
  9. fanquake commented at 3:32 AM on August 8, 2016: member

    Testing this, and clicking hide doesn't seem to do anything? Also, If your opening a close-to, or recently synced wallet, should the modal disappear automatically once it reaches 100%?

    OS X screenshots: modal full-wallet

    Also, this looks like a new compiler warning.

      CXX      qt/qt_libbitcoinqt_a-bitcoingui.o
    qt/bitcoingui.cpp:118:5: warning: field 'spinnerFrame' will be initialized after field 'modalOverlay' [-Wreorder]
        spinnerFrame(0),
        ^
    1 warning generated.
    
  10. jonasschnelli commented at 12:31 PM on August 12, 2016: contributor

    @fanquake: It should hide automatically if you are in sync,.. but only, If you haven't opened it manually (by pressing the warning icons). Not sure if it should also hide in that case, but probably.

  11. Joukehofman commented at 9:35 AM on August 19, 2016: none

    Look great! Maybe add to the warning text that it's also not possible to send bitcoins?

  12. jonasschnelli force-pushed on Aug 26, 2016
  13. jonasschnelli commented at 7:49 AM on August 26, 2016: contributor

    Fixed warning. Added a short text passage "Spending bitcoins is not possible during that phase!".

  14. sipa commented at 7:52 AM on August 26, 2016: member

    Does the "This process is not complete yet" disappear when 100% is reached, but the window is still open?

  15. [Refactor] refactor function that forms human readable text out of a timeoffset 0904c3cda4
  16. [Qt] make Out-Of-Sync warning icon clickable bd44a04dc3
  17. [Qt] Always pass the numBlocksChanged signal for headers tip changed a001f18802
  18. [Qt] ClientModel add method to get the height of the header chain e47052f6b5
  19. jonasschnelli force-pushed on Aug 26, 2016
  20. jonasschnelli commented at 7:54 AM on August 26, 2016: contributor

    @sipa: yes. It will (https://github.com/bitcoin/bitcoin/pull/8371/files#diff-0db7dd184df07a48c307ccc182021a68R776). But only if you not have manually opened the "window" by clicking on the alert icons (then it will stay intentionally).

  21. sipa commented at 8:00 AM on August 26, 2016: member

    No, I mean that if you have manually clicked it, and the window does stay open, does the "This process has not completes yet" text also stay? Because that is very confusing...

  22. jonasschnelli commented at 8:02 AM on August 26, 2016: contributor

    Ah. I get your point. Maybe force closing then would make sense (or changing the text in the upper section [more complicate]).

  23. sipa commented at 8:04 AM on August 26, 2016: member

    I think force closing is fine.

  24. MarcoFalke commented at 8:05 AM on August 26, 2016: member

    I think force closing is fine.

    Agree

  25. jonasschnelli force-pushed on Aug 26, 2016
  26. [Qt] add out-of-sync modal info layer e3245b43d5
  27. jonasschnelli force-pushed on Aug 26, 2016
  28. jonasschnelli commented at 9:36 AM on August 26, 2016: contributor

    Updated so that the layer/window does force close when the we detect in-sync state.

  29. sipa commented at 12:15 PM on August 26, 2016: member

    The "number of blocks left" goes up and down for a while (presumably because the headers aren't processed yet?)... if the tip header is too far in the past, it may be better to say "unknown" there.

  30. fanquake commented at 12:37 PM on August 26, 2016: member

    Updated OS X screenshot, the hiding issue seems to be fixed now. modal

  31. luke-jr commented at 8:21 AM on September 10, 2016: member

    Looks like the font color should not depend on the theme?

    Rather, the background colour should depend on the theme also? Why is this overriding the theme?

  32. flack commented at 9:37 AM on September 12, 2016: contributor

    Shouldn't this dialog also have a headline of some sort (e.g. "Wallet out of sync")?

    Because if I look at the screenshot, I only see the progress data, and the text starts with "The displayed information may be out of date", but in fact, it's not the currently displayed data (progress information) that is out of date, but the stuff in the wallet, which is obscured by the overlay.

  33. jonasschnelli commented at 9:41 AM on September 12, 2016: contributor

    @luke-jr: I think creating nice visualizations is difficult with the presumption to only use theme based colors. @flack: Indeed. The term "the displayed information... " can look like its referred to the data below (in the same overlay). I have plans to overhaul this PR soon.

  34. [Qt] only update "amount of blocks left" when the header chain is in-sync d8b062ef5e
  35. jonasschnelli force-pushed on Sep 13, 2016
  36. jonasschnelli commented at 4:06 PM on September 13, 2016: contributor

    Updated with recommendation from @sipa to hide the remaining blocks during headers-first during IBD.

    <img width="971" alt="bildschirmfoto 2016-09-13 um 18 01 34" src="https://cloud.githubusercontent.com/assets/178464/18481542/ce43cc92-79dc-11e6-9ae0-69dab3ac9969.png">

  37. in src/qt/modaloverlay.cpp:None in d8b062ef5e outdated
     112 | +
     113 | +        // show expected remaining time
     114 | +        ui->expectedTimeLeft->setText(GUIUtil::formateNiceTimeOffset(remainingMSecs/1000.0));
     115 | +
     116 | +        // keep maximal 5000 samples
     117 | +        static int maxSamples = 5000;
    


    MarcoFalke commented at 8:12 AM on September 21, 2016:

    nit

    static const int MAX_SAMPLES = 5000;
    
  38. in src/qt/modaloverlay.cpp:None in d8b062ef5e outdated
      84 | +    blockProcessTime.push_front(qMakePair(currentDate.currentMSecsSinceEpoch(), nVerificationProgress));
      85 | +
      86 | +    // show progress speed if we have more then one sample
      87 | +    if (blockProcessTime.size() >= 2)
      88 | +    {
      89 | +        // try to get the window from the last 500 seconds or at least 10 samples
    


    MarcoFalke commented at 8:12 AM on September 21, 2016:

    Where does it say 10 in the source code?

  39. in src/qt/walletframe.h:None in d8b062ef5e outdated
      37 | @@ -38,6 +38,10 @@ class WalletFrame : public QFrame
      38 |  
      39 |      void showOutOfSyncWarning(bool fShow);
      40 |  
      41 | +Q_SIGNALS:
      42 | +    /** Notify that the user has requested more information about the out-of-sync warning */
      43 | +    void requestedOfSyncWarningInfo();
    


    MarcoFalke commented at 8:14 AM on September 21, 2016:

    nit: I don't think this is proper English. Idk, get rid of the Of?

  40. in src/qt/bitcoingui.cpp:None in d8b062ef5e outdated
     722 | +        {
     723 | +            /* use clientmodels getHeaderTipHeight and getHeaderTipTime because the NotifyHeaderTip signal does not fire when updating the best header */
     724 | +            modalOverlay->setKnownBestHeight(clientModel->getHeaderTipHeight(), QDateTime::fromTime_t(clientModel->getHeaderTipTime()));
     725 | +        }
     726 | +        else
     727 | +            modalOverlay->tipUpdate(count, blockDate, nVerificationProgress);
    


    MarcoFalke commented at 8:16 AM on September 21, 2016:

    style nit: brackets are "required" for the else when they are used for the if.

  41. MarcoFalke approved
  42. MarcoFalke commented at 8:20 AM on September 21, 2016: member

    Tested-only ACK. Found a few style nits. (You can fix them in a separate fixup commit)

    ACK d8b062ef5eea3addff00a09bad1dab162452b4b5

  43. MarcoFalke commented at 8:22 AM on September 21, 2016: member

    Looks good to merge

    screenshot from 2016-09-21 10-20-23

  44. jonasschnelli commented at 8:30 AM on September 21, 2016: contributor

    Fixed nits reported by @MarcoFalke. @MarcoFalke: can you re-ack?

  45. luke-jr commented at 8:33 AM on September 21, 2016: member

    @jonasschnelli On the contrary, violating the theme contradicts the goal of nice visualisations.

  46. jonasschnelli commented at 8:35 AM on September 21, 2016: contributor

    @luke-jr: I think adding a black overlay with white text does not directly violates the theme. It just will result in a more controllable look and feel.

  47. luke-jr commented at 8:40 AM on September 21, 2016: member

    Software controlling look-and-feel means the user's chosen look-and-feel is being ignored. Presumably the user has chosen them because they like it, so violating them simply won't be nice.

  48. jonasschnelli commented at 8:46 AM on September 21, 2016: contributor

    I think themes are there for a reason. Using the theme colors might result in loosing the optical effect which is there for a reason. The back background with the white info layer should imply that the informations within the layer are on a different level of importance.

    But happy if someone likes to try migrating this to the theme colors.

  49. luke-jr commented at 8:59 AM on September 21, 2016: member

    (BTW, I don't mean to imply theme concerns need to interfere in merging this. By all means, feel free. We can work on theming later.)

  50. gmaxwell commented at 11:57 PM on September 22, 2016: contributor

    Too much text, esp too much bolded text. I automatically don't read the top part (though I did in the original version). Also "Spending bitcoins is not possible during that phase!" is not true.

  51. sipa commented at 12:01 AM on September 23, 2016: member

    Well you can't spend funds your client hasn't seen the receival of.

  52. gmaxwell commented at 12:10 AM on September 23, 2016: contributor

    @sipa Yes, but you can spend anything you can see... This is a common point of confusion already. And in the common case where you had bitcoin off for the last month, but received no payment since then, you're good to go for sending.

  53. jonasschnelli commented at 6:12 AM on September 23, 2016: contributor

    @gmaxwell: would you be willing providing a better text?

    I agree that "spending Bitcoins is not possible" is not true and can lead to confusion, especially if one catches up just a couple of days (the info-layer will also be shown then).

  54. in src/qt/forms/modaloverlay.ui:None in 080a877854 outdated
     147 | +                 <weight>75</weight>
     148 | +                 <bold>true</bold>
     149 | +                </font>
     150 | +               </property>
     151 | +               <property name="text">
     152 | +                <string>This means that recent transactions will not be visible, and the balance will not be up-to-date until this process has completed. Spending bitcoins is not possible during that phase!</string>
    


    MarcoFalke commented at 9:22 AM on September 23, 2016:

    Maybe

    s/possible/recommended/
    

    ?


    jonasschnelli commented at 10:31 AM on September 23, 2016:

    Or what about [...]Spending bitcoins may not be possible[...]?


    MarcoFalke commented at 11:59 AM on September 23, 2016:

    Fine, I think either is better.

  55. jonasschnelli force-pushed on Sep 23, 2016
  56. jonasschnelli commented at 1:33 PM on September 23, 2016: contributor

    Force pushed a text- and bold-section-change. This is how it looks now: <img width="962" alt="bildschirmfoto 2016-09-23 um 15 31 38" src="https://cloud.githubusercontent.com/assets/178464/18787405/0b1c40c2-81a3-11e6-9cfc-6abb5ba6b764.png">

  57. MarcoFalke commented at 2:06 PM on September 23, 2016: member

    Are you sure you pushed the text change?

  58. [Qt] modalinfolayer: removed unused comments, renamed signal, code style overhaul 08827df3ec
  59. jonasschnelli force-pushed on Sep 23, 2016
  60. jonasschnelli commented at 2:07 PM on September 23, 2016: contributor

    Pushed now..

  61. MarcoFalke commented at 2:33 PM on September 23, 2016: member

    ACK 08827df3ecce925928dc3bedcdef63bfca290300

  62. jonasschnelli merged this on Sep 23, 2016
  63. jonasschnelli closed this on Sep 23, 2016

  64. jonasschnelli referenced this in commit 24f72e9f3f on Sep 23, 2016
  65. in src/qt/clientmodel.cpp:None in 08827df3ec
     249 | @@ -234,7 +250,7 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, const CB
     250 |      int64_t& nLastUpdateNotification = fHeader ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification;
     251 |  
     252 |      // if we are in-sync, update the UI regardless of last update time
     253 | -    if (!initialSync || now - nLastUpdateNotification > MODEL_UPDATE_DELAY) {
     254 | +    if (fHeader || !initialSync || now - nLastUpdateNotification > MODEL_UPDATE_DELAY) {
    


    MarcoFalke commented at 11:03 PM on September 25, 2016:

    Why is this needed?

    Oh, nvm. It is needed so a header update is not accidentally lost. Still, this makes the gui unresponsive when -reindex is used...

  66. Xekyo commented at 2:02 PM on September 26, 2016: member

    Instead of "this process" and "that phase" I would repeat "synchronization":

    The displayed information may be out of date. Your wallet automatically synchronizes with the Bitcoin network while connected, but synchronization has not completed yet. Recent transactions will not be visible until your wallet catches up to the network. Your balance will update as transactions are processed. Spending bitcoins may not be possible until synchronization has finished.

  67. luke-jr referenced this in commit cb80e00094 on Oct 20, 2016
  68. luke-jr referenced this in commit 8e325b460f on Oct 20, 2016
  69. luke-jr referenced this in commit 4f4d6410c7 on Oct 20, 2016
  70. luke-jr referenced this in commit 88c4ec6119 on Oct 20, 2016
  71. luke-jr referenced this in commit 4d7ccc532d on Oct 20, 2016
  72. luke-jr referenced this in commit d1bc374e75 on Oct 20, 2016
  73. luke-jr referenced this in commit 752f57a238 on Oct 20, 2016
  74. codablock referenced this in commit e19bf62795 on Sep 7, 2017
  75. UdjinM6 referenced this in commit 9707ca5cea on Sep 9, 2017
  76. 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: 2026-04-13 18:15 UTC

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