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

jonasschnelli commented at 1:57 PM on July 19, 2016: contributor

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

jonasschnelli added the label GUI on Jul 19, 2016

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"?

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

Looks very nice!

laanwj commented at 9:04 AM on July 28, 2016: member

Looks like the font color should not depend on the theme?
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."

jonasschnelli force-pushed on Jul 28, 2016

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).

jonasschnelli added this to the milestone 0.14 on Jul 28, 2016

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.

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.

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?

jonasschnelli force-pushed on Aug 26, 2016

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!".

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?

[Refactor] refactor function that forms human readable text out of a timeoffset 0904c3cda4

[Qt] make Out-Of-Sync warning icon clickable bd44a04dc3

[Qt] Always pass the numBlocksChanged signal for headers tip changed a001f18802

[Qt] ClientModel add method to get the height of the header chain e47052f6b5

jonasschnelli force-pushed on Aug 26, 2016

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).

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...

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]).

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

I think force closing is fine.

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

I think force closing is fine.

Agree

jonasschnelli force-pushed on Aug 26, 2016

[Qt] add out-of-sync modal info layer e3245b43d5

jonasschnelli force-pushed on Aug 26, 2016

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.

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.

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

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

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?

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.

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.

[Qt] only update "amount of blocks left" when the header chain is in-sync d8b062ef5e

jonasschnelli force-pushed on Sep 13, 2016

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.

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;

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?

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?

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.

MarcoFalke approved

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

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

Looks good to merge

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

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

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

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.

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.

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.

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.

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.)

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.

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.

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.

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).

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.

jonasschnelli force-pushed on Sep 23, 2016

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">

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

Are you sure you pushed the text change?

[Qt] modalinfolayer: removed unused comments, renamed signal, code style overhaul 08827df3ec

jonasschnelli force-pushed on Sep 23, 2016

jonasschnelli commented at 2:07 PM on September 23, 2016: contributor

Pushed now..

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

ACK 08827df3ecce925928dc3bedcdef63bfca290300

jonasschnelli merged this on Sep 23, 2016

jonasschnelli closed this on Sep 23, 2016

jonasschnelli referenced this in commit 24f72e9f3f on Sep 23, 2016

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...

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.

luke-jr referenced this in commit cb80e00094 on Oct 20, 2016

luke-jr referenced this in commit 8e325b460f on Oct 20, 2016

luke-jr referenced this in commit 4f4d6410c7 on Oct 20, 2016

luke-jr referenced this in commit 88c4ec6119 on Oct 20, 2016

luke-jr referenced this in commit 4d7ccc532d on Oct 20, 2016

luke-jr referenced this in commit d1bc374e75 on Oct 20, 2016

luke-jr referenced this in commit 752f57a238 on Oct 20, 2016

codablock referenced this in commit e19bf62795 on Sep 7, 2017

UdjinM6 referenced this in commit 9707ca5cea on Sep 9, 2017

MarcoFalke locked this on Sep 8, 2021