fix: Fixes #320. SUI-based modal overlays to prevent form field values reseting on sync.

[ltfschoen]

This is an alternative solution to #326 that was previously reviewed. This time we're creating an overlay that's based on a SUI Modal.

Simulate bad connection:

The code has been updated so that the health modal HealthModal is an overlay (based on a SUI modal Modal component) that doesn't cause the children in the RequireHealthOverlay component to be re-rendered such that any input field values that the user entered are reset before they've finished. This is very annoying if you have a bad connection and it keeps reseting just before you click 'Send'.

To test that it works, you can just start the app, and then switch off your wifi temporarily and the modal will appear.

Or you can achieve the same outcome a longer way by modifying the code to simulate a bad connection modal overlay. After we've done that we'll go to the Send Ether page, and every 6 seconds it will display the modal. To confirm that it doesn't reset the values just enter a valid recipient address or amount or some gas, and if it's still there after the next cycle of the modal appearing and disappearing then it's working.

Modify the code as follows:

• Hard-code a title and description in paritytech/fether/packages/fether-ui/src/Modal/Modal.js
• Modify componentDidUpdate() in RequireHealthOverlay.js to be the following so it shows the health modal every 6 seconds:

  componentDidUpdate () {
    const { visible } = this.state;	

     setTimeout(() => {	
      this.setState({ visible: !visible });	
    }, 6000);
    // this.updateVisibility();
  }

Simulate send transaction

The code hasn't really been changed apart from re-using the SUI Modal Modal component and moving the relevant code into a SentModal component.

You can just send a transaction as normal to check that it still works, but the back button that eventually appears may take a while (i.e. 1350 and counting confirmations so far on kovan for this tx i send a while ago https://kovan.etherscan.io/tx/0x7f247b129cd98ff52bcb8bdf06ed4ce32f68f174dd0c7624ea29e087f3558ae3)

• Modify paritytech/fether/packages/fether-ui/src/SentModal/SentModal.js and add some words instead of an empty string to these return values for title and description:

    if (!txStatus) {
      return 'Text to test that it shows a status';
    }

• Remove the following to show the back button

    if (confirmations < MIN_CONFIRMATIONS) {
      return;
    }

Simple create a transaction as normal and send it

[ltfschoen]

Question: On the "Send Ether" page we previously only made the bad connection modal take up ~80%
of the full screen (it didn't cover the navigation bar at the top. Is this "Send Ether" page an exception the full screen rule mentioned here #326 (comment). In this commit I've made it 100% full screen on that page. Should I just make it the same as it currently is (i.e. so the navigation bar is still shown including "Back" button, the "Send Ether" title and the "+" button)?

I think these other issues could be handled in this PR, but I don't full understand what the specific requirements for them are, and if they are still active issues:

• Syncing screen preventing to do anything else #51
• Enhance experience with bad connection #218

[Tbaut]

Here is the reason behind showing the navigation buttons.
When the node is syncing/doesn't have enough peers, users should be able to:

• Create/restore a new account
• See the list of current accounts
• Backup an account
• Manage tokens (wouldn't see them then but that's ok)

User should not:

• See the balance of their tokens (would be inaccurate)
• Send a token

Letting Fether being halfway usable when syncing or having a bad connection makes the UX so much better because users aren't "blocked" on a loading screen. This is particularly important for the first launch as Fether might require some time to sync, while users don't have accounts yet and will spend some time create/restore it. It's also very important in case the node can't connect/sync for some reason and all the user wants to do is take his account to another wallet (backup and leave).

[Tbaut]

Works great in my tests except for 1thing, the modal doesn't update

• when sending a token, the transaction confirmation is stuck at "sending your transaction" and never gets further although the console shows:

ether-react:sendStore Tx status updated. +5s 
{confirmed: {…}}
confirmed...

• when syncing, the percentage doesn't change either, the modal says for instance: "Syncing 3%" but it will not change until fully synced and not visible anymore

Nit: Could we center the message vertically, eg on the token view here or here, the account has several tokens listed (>3)

[ltfschoen]

@Tbaut regarding:

when syncing, the percentage doesn't change either, the modal says for instance: "Syncing 3%" but it will not change until fully synced and not visible anymore

I just tried this and found that the percentage does change in the modal.
it worked both when i was running a node that was started by Fether, and also one that I started separately beforehand. i tried removing this subfolder ~/Library/Application Support/io.parity.ethereum/chains_light/kovan and it synced from scratch and showed the percentage

[ltfschoen]

@Tbaut regarding:

Nit: Could we center the message vertically, eg on the token view here or here, the account has several tokens listed (>3)

I've pushed commits to try and address this so far, which is just an offset from the top of the screen depending on whether its a full screen modal or not, but it doesn't cater for >3 tokens.

As the height depends on the amount of tokens that are passed down via props, I thought I could just modify packages/fether-react/src/RequireHealthOverlay/RequireHealthOverlay.js, by adding:

...
import withTokens from '../utils/withTokens';
...
@withTokens
...

  <HealthModal
  ...
    tokenCount={this.props.tokensArray.length}

And then apply a different CSS class depending on the length.

But with that approach, as soon as I add just import withTokens from '../utils/withTokens'; and @withTokens, I get error:

Uncaught Error: You should not use <Route> or withRouter() outside a <Router>

The only other way that comes to mind at the moment is to detect when the window height changes using the window object and use DOM manipulation of relevant styles from within the React component so it appears in the center. And that's a no-go.

A simpler solution hasn't come to mind yet...

[ltfschoen]

@Tbaut regarding:

when sending a token, the transaction confirmation is stuck at "sending your transaction" and never gets further although the console shows: ...

I've pushed a commit that restores this functionality

[ltfschoen]

Why don't we show the status icon on:

• Create a new account (the first step page)
• Import account page
• Send Ether or Send THIBCoin transaction and password pages

[amaury1093]

Why don't we show the status icon on:
* Create a new account (the first step page)
* Import account page
* Send Ether or Send THIBCoin transaction and password pages

It actually should be on all pages. Maybe I've forgotten to add them.

[ltfschoen]

Why don't we show the status icon on:

• Create a new account (the first step page)
• Import account page
• Send Ether or Send THIBCoin transaction and password pages

It actually should be on all pages. Maybe I've forgotten to add them.

I think we should add that task in a different issue, as it involves a bit of mucking around with flexbox and different footer-nav heights depending on what page the user is on

[amaury1093]

I think we should add that task in a different issue, as it involves a bit of mucking around with flexbox and different footer-nav heights depending on what page the user is on

Agreed.

I just have one last grumble about the missing withoutLoading(), after that we can merge.

[ltfschoen]

I think we should add that task in a different issue, as it involves a bit of mucking around with flexbox and different footer-nav heights depending on what page the user is on

Agreed.

I just have one last grumble about the missing withoutLoading(), after that we can merge.

I used tap to see what was going on:

...
import { startWith, tap, withLatestFrom } from 'rxjs/operators';
import light from '@parity/light.js-react';

import { transactionCountOf$, withoutLoading } from '@parity/light.js';
import withAccountsInfo from '../utils/withAccountsInfo';

const WithAccount = compose(
...
  light({
    transactionCount: props =>
      transactionCountOf$(props.match.params.accountAddress).pipe(
        startWith(undefined),
        withoutLoading(),
        tap(transactionCount => console.log('transactionCount', transactionCount))
      )
  }),

[amaury1093]

This will make the app crash, right? there's no error catcher here if i'm not mistaken. Should we put these throws inside validateForm (or prevalidate) instead?

[ltfschoen]

Ok, I've pushed a commit moving them into validateForm function

[ltfschoen]

I think we should add that task in a different issue, as it involves a bit of mucking around with flexbox and different footer-nav heights depending on what page the user is on

Agreed.

I just have one last grumble about the missing withoutLoading(), after that we can merge.

I've pushed a commit that adds withoutLoading() back, in combination with using startWith(undefined)

[amaury1093]

Perfect!