web-wallet: Overhaul landing page

[nortonandreev]

Resolves #1534

[nortonandreev]

Wondering if it won't be better to just have the landing page under the / route now?

[ascartabelli]

Leaving some notes as there are some problems.

• these changes should have been made in three separate PRs

• pressing enter in the unlock textbox doesn't always unlock the wallet:
  in some cases it seems that it just fires a blur event on the textbox

• if I set up a password, enter my wallet, go to settings and reset the wallet

  • login info is not deleted: new bug? regression we had from before?
  • the system sees existing login info, unlock screen asks for password,
    but by entering the password I'm redirected to the terms & privacy policy in the restore flow.
    By accepting the terms I see my password in clear as the first word of the mnemonic (see screenshot)
    with no possibility to advance further.

• if I enter a new mnemonic and with an existing wallet, and I accept to proceed by restoring another wallet,
  I'm brought to the "enter mnemonic phrase" step of the restore flow, but the mnemonic is now in clear
  while just seconds ago it was hidden.
  - should we skip this step?
  - should we show it, but obfuscated by default with an option to show it? Should we consider always obfuscating it by default (or maybe only after a paste) as others are doing? (this is for a new issue anyway)

• didn't do a full review of the code yet, as there are some problems and we're waiting for @ZER0's input,
  but I think FieldButtonGroup is quite a confusing name for that component.
  Given that for now the component is specific to the application, I would go for something along the lines of
  "UnlockTexbox", or anyway something mentioning that is the control for unlocking.

• at this point it's probably better to move the TermsOfService in the app components?

• it's fine having a redirect(301, "/") in the setup route right now, but maybe we should consider
  trapping all 404s in client routing instead and make the same redirect to the home? (in a future issue)
  What do you think?

[nortonandreev]

Leaving some notes as there are some problems.

• these changes should have been made in three separate PRs

We could have done, but I also see the work around the login page as a whole. For the rest of the allowing a sync from a custom block height epic, I will work with Matteo to organize the features in a way that makes sense to deliver them.

From yesterday's discussion, seems like we could do the individual parts separately – I would have personally used an feature branch to capture the whole initiative (with smaller branches containing the individual tasks), so to have the whole thing delivered at once.

• pressing enter in the unlock textbox doesn't always unlock the wallet:
  in some cases it seems that it just fires a blur event on the textbox

Will investigate. I think I saw some issues with Chrome, which were not present in Safari.

• if I set up a password, enter my wallet, go to settings and reset the wallet

  • login info is not deleted: new bug? regression we had from before?
  • the system sees existing login info, unlock screen asks for password,
    but by entering the password I'm redirected to the terms & privacy policy in the restore flow.
    By accepting the terms I see my password in clear as the first word of the mnemonic (see screenshot)
    with no possibility to advance further.

I haven't changed anything in the settings page, but I also did not manually test this flow. This is a good catch, I will check what is going on. Also, seeing your password as the first word of the mnemonic, obviously, shouldn't happen, will fix this.

• if I enter a new mnemonic and with an existing wallet, and I accept to proceed by restoring another wallet,
  I'm brought to the "enter mnemonic phrase" step of the restore flow, but the mnemonic is now in clear
  while just seconds ago it was hidden.
  • should we skip this step?
  • should we show it, but obfuscated by default with an option to show it? Should we consider always obfuscating it by default (or maybe only after a paste) as others are doing? (this is for a new issue anyway)

As you said, let's open a new issue and involve the others in the discussion.

• didn't do a full review of the code yet, as there are some problems and we're waiting for @ZER0's input

I have pinged Matteo on Discord to have a look. I assume it will be after the consensus update has been rolled out.

but I think FieldButtonGroup is quite a confusing name for that component.
Given that for now the component is specific to the application, I would go for something along the lines of
"UnlockTexbox", or anyway something mentioning that is the control for unlocking.

It is not specific to the application, the same component will be used in the Explorer too, thus the generic component name. Another option could be TextButtonCombo?

• at this point it's probably better to move the TermsOfService in the app components?

I'm fine with that.

• it's fine having a redirect(301, "/") in the setup route right now, but maybe we should consider
  trapping all 404s in client routing instead and make the same redirect to the home? (in a future issue)
  What do you think?

I was just thinking from SEO perspective – if a search engine has indexed a route that we have gotten rid of, I think we should respond with the appropriate code – at least initially when we release the change.

[ascartabelli]

Wondering if it won't be better to just have the landing page under the / route now?

I think so, yes.

(cc @kieranhall @deuch13 )

[deuch13]

Wondering if it won't be better to just have the landing page under the / route now?

I think so, yes.

(cc @kieranhall @deuch13 )

I agree with this, as I said in my earlier comment I would go even further and have both login and restore flow under the same route.

[ascartabelli]

but I think FieldButtonGroup is quite a confusing name for that component.
Given that for now the component is specific to the application, I would go for something along the lines of
"UnlockTexbox", or anyway something mentioning that is the control for unlocking.

It is not specific to the application, the same component will be used in the Explorer too, thus the generic component name. Another option could be TextButtonCombo?

I thought it was under $lib/components but, anyway, it deserves a better name.
Not sure about your suggestion, but right now nothing good comes to mind: maybe @kieranhall and @deuch13 can barge in and help.

• it's fine having a redirect(301, "/") in the setup route right now, but maybe we should consider
  trapping all 404s in client routing instead and make the same redirect to the home? (in a future issue)
  What do you think?

I was just thinking from SEO perspective – if a search engine has indexed a route that we have gotten rid of, I think we should respond with the appropriate code – at least initially when we release the change.

Yes, let's leave it like you did as right now we don't have the option to modify things server side, and this way the route exists even for the server.