Offline support #195

TobiaszCudnik · 2019-01-28T16:31:39Z

This PR adds:

offline support (account, recent transactions)
connections status (top bar icon / text, responsive)

Fixes following issues:

Better offline / connection status in the UI Better offline / connection status in the UI #187
Show the last state when offline Show the last state when offline #185

Does not cover the following:

blocks/change doesnt refresh the involved transactions blocks/change doesnt refresh involved transactions #190

To demo the feature one needs to have a non-localhost server with HTTPS.

- listen to the browser's offline event

- refactored service worker to async/await

- persist the account data - refresh the data after going online - persist recent transactions

- persist recent transactions

- persist recent transactions - refresh when connecting

- support for mobile

During one of the Ledger code refactors the feature of pinging the Ledger device only when the UI needs the information was lost. This commit reimplements that feature. This is also needed for users of Yubikeys (for which the U2F protocol was created for), to not bombard their non-Ledger devices with U2F requests when it's not needed. See #174.

The LedgerChannel wasn't being closed properly in the ConfirmTransactionDialog logic, thus the background pinging of the device continued indefinitely. This has been fixed. In addition the LedgerChannel was opened for non-Ledger transactions. Now the channel is only openend for accounts of type LEDGER. See #174 for motivations behind these improvements.

- persist recent transactions

roosmaa

Would've been better if this had been broken in to multiple PRs as currently this branch contains various other changes that aren't directly related to offline support feature (file formatting changes, bug fixes, my changes from another PR). Makes reviewing and catching things harder.

src/index.tsx

src/stores/account.ts

roosmaa

The font on the connected indicator is wrong.

src/containers/wallet/ConnectionStatus.tsx

roosmaa · 2019-01-29T10:26:06Z

Found a bug somewhere with the dirty state probably:

Make sure that the cache local storage key is unset
Refresh wallet with 3 accounts, account 1 open
Open dev tools and toggle offline mode
Open account 2, see errors in console, the loading indicator doesn't go away
Go back to account 1 & disable offline mode
Open account 2, the loading indicator still running, but no actual request to load the transaction data is made
Open account 3, the account history is loaded successfully

roosmaa · 2019-01-29T10:42:57Z

Another bug - the account balance isn't persisted after an offline reload.

Setup a domain with the production service worker installed
Open wallet and let it load data for accounts
Enable offline mode in developer tools
Reload the page
Old transactions appear, but the balance is shown as zero.

Sub-bug 2 of this is that at step 5 the connection indicator claims "Connecting" even-though the browser is offline.

- fixed loading of account cache

- fixed transaction cache timestamp

- fixed "load more" button

- fixed fonts

TobiaszCudnik · 2019-01-29T15:49:32Z

Would've been better if this had been broken in to multiple PRs as currently this branch contains various other changes that aren't directly related to offline support feature (file formatting changes, bug fixes, my changes from another PR). Makes reviewing and catching things harder.

Somethings wrong with the github's diff, as it doesnt compare with current master. Eg this change isnt a part of the PR, along with others. Local diff shows the real situation.

Formatting is a part of precommit which seems to have been skipped for some files in recent commits. We should keep master formatted to avoid those.

account balance isn't persisted after an offline reload.

Fixed.

connection indicator claims "Connecting" even-though the browser is offline.

Browser limitation. Online/offline status is event based. Events fire only when it changes, thus doesnt fire when page is load from a service worker when offline.

TobiaszCudnik · 2019-01-29T15:54:26Z

To test the recent changes you need to clean the cache (at least localStorage.cache).

TobiaszCudnik · 2019-01-29T16:35:23Z

Browser limitation. Online/offline status is event based. Events fire only when it changes, thus doesnt fire when page is load from a service worker when offline.

This actually turns out not to be true and we can rely on navigator.onLine. Fixed.

roosmaa · 2019-01-29T21:50:06Z

Will do another review-pass & test run tomorrow morning to see if I spot anything else.

roosmaa

Looking good. Didn't find any obvious bugs during my testing either. Just one suggestion about importData() function signature changes (see comments). Up to you if you want to change this or merge straight away.

roosmaa · 2019-01-30T10:29:45Z

src/stores/wallet.ts

-    account.importData(parseAccountReponse(res, local));
-    account.loaded = true;
+    const res = await this.fetchAccountData(id);
+    account.importData(parseAccountReponse(res, local), true, true);


I know that intelliJ shows the argument names, but for the sake of non-intelliJ users it would be nice if the importData() function took an options object with named fields as the final parameter, instead of regular boolean arguments. E.g:

account.importData(parseAccountReponse(res, local), { saveCache: true, markAsLoaded: true });

Fixed, but an options object for 2 params seems like an overkill.

roosmaa · 2019-01-30T10:42:23Z

src/stores/transactions.ts

@@ -152,9 +212,11 @@ export class Transaction {
  amount: RawAmount;
  amountFee: RawAmount;
  fee: RawAmount;
+  /** TODO may be a problem when keeping the same transaction for 2 users */


You mean when both the sender account and recipient account are in the same wallet? That worked as expected while I was testing things.

Note for the future, related to #191.

TobiaszCudnik · 2019-01-30T15:02:17Z

I've reformatted the whole master and rebased this branch properly, so only the involved changes are visible in the diff.

TobiaszCudnik and others added 18 commits January 22, 2019 17:59

Better offline / connection status in the UI #187

f25e2a2

Connection to the websocket / fiat provider broken #183

9919f63

fix service worker registration regression from #164

151cc09

Better offline / connection status in the UI #187

4f8e007

Better offline / connection status in the UI #187

e7a5d9a

- listen to the browser's offline event

Show the last state when offline #185

2dc7053

- refactored service worker to async/await

removed yarn deploy-nightly

071ad78

Show the last state when offline #185

dbb3eb0

- persist the account data - refresh the data after going online - persist recent transactions

Show the last state when offline #185

a47c20f

- persist recent transactions

Vote transactions with empty votes break the wallet #189

66805ec

Show the last state when offline #185

910d8e5

- persist recent transactions - refresh when connecting

Better offline / connection status in the UI #187

c10d176

- support for mobile

precommit

a3280b4

fixed wrong message names in TxDetailsExpansionPanel.getSummary

b3c951d

Vote transactions with empty votes break the wallet #189

0b383bc

Show the last state when offline #185

d623bfd

- persist recent transactions

TobiaszCudnik requested a review from roosmaa January 28, 2019 16:31

lang IDs

02d264a

roosmaa reviewed Jan 29, 2019

View reviewed changes

src/index.tsx Outdated Show resolved Hide resolved

src/stores/account.ts Outdated Show resolved Hide resolved

src/stores/account.ts Outdated Show resolved Hide resolved

roosmaa suggested changes Jan 29, 2019

View reviewed changes

src/containers/wallet/ConnectionStatus.tsx Outdated Show resolved Hide resolved

src/containers/wallet/ConnectionStatus.tsx Outdated Show resolved Hide resolved

src/containers/wallet/ConnectionStatus.tsx Outdated Show resolved Hide resolved

TobiaszCudnik added 6 commits January 29, 2019 15:45

Show the last state when offline #185

1e13420

- fixed loading of account cache

Show the last state when offline #185

095162a

- fixed transaction cache timestamp

Show the last state when offline #185

4fb32ac

- fixed "load more" button

Show the last state when offline #185

7754191

- fixed fonts

linter

e3fd152

fixed isDirty #185

e74560f

show the disconnected icon when not online #164

c8e7a8e

roosmaa approved these changes Jan 30, 2019

View reviewed changes

TobiaszCudnik added 5 commits January 30, 2019 14:55

Merge branch 'master' into feature/offline-support

c569e6c

Merge branch 'master' into feature/offline-support

86b92e7

test for TransactionStore.loadCache

66856b9

better comment

fd13d9f

options object for importData

8fabd6a

fixes for account import params #185

c8a49e1

TobiaszCudnik mentioned this pull request Feb 1, 2019

Submit the app to Play Store as TWA #202

Open

Merge branch 'master' into feature/offline-support

3349e76

TobiaszCudnik merged commit 57c0e6b into master Feb 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offline support #195

Offline support #195

TobiaszCudnik commented Jan 28, 2019

roosmaa left a comment

roosmaa left a comment

roosmaa commented Jan 29, 2019 •

edited

roosmaa commented Jan 29, 2019 •

edited

TobiaszCudnik commented Jan 29, 2019

TobiaszCudnik commented Jan 29, 2019

TobiaszCudnik commented Jan 29, 2019

roosmaa commented Jan 29, 2019

roosmaa left a comment

roosmaa Jan 30, 2019

TobiaszCudnik Jan 30, 2019

roosmaa Jan 30, 2019

TobiaszCudnik Jan 30, 2019

TobiaszCudnik commented Jan 30, 2019 •

edited

Offline support #195

Offline support #195

Conversation

TobiaszCudnik commented Jan 28, 2019

roosmaa left a comment

Choose a reason for hiding this comment

roosmaa left a comment

Choose a reason for hiding this comment

roosmaa commented Jan 29, 2019 • edited

roosmaa commented Jan 29, 2019 • edited

TobiaszCudnik commented Jan 29, 2019

TobiaszCudnik commented Jan 29, 2019

TobiaszCudnik commented Jan 29, 2019

roosmaa commented Jan 29, 2019

roosmaa left a comment

Choose a reason for hiding this comment

roosmaa Jan 30, 2019

Choose a reason for hiding this comment

TobiaszCudnik Jan 30, 2019

Choose a reason for hiding this comment

roosmaa Jan 30, 2019

Choose a reason for hiding this comment

TobiaszCudnik Jan 30, 2019

Choose a reason for hiding this comment

TobiaszCudnik commented Jan 30, 2019 • edited

roosmaa commented Jan 29, 2019 •

edited

roosmaa commented Jan 29, 2019 •

edited

TobiaszCudnik commented Jan 30, 2019 •

edited