Add Ledger hardware wallet support

[brunobar79]

Fixes #717 and #4593

UI/UX wise it's almost identical to the Trezor implementation, with the main difference that instead of connecting to the device via popup (Trezor connect) I've implemented a 2 way communication through an iframe in the background page, and the iframe interacts to the device sending messages back and forth to the extension. More info here

Another important thing is that we're supporting 2 different HD paths for ledger.
They have updated the previous one (which MEW / MyCrypto still uses) so by supporting both we're allowing users to use their legacy accounts too.

@cjeria @bdresser please take a look at the design and provide feedback:

video: http://recordit.co/zVcKUIPUYY

[bdresser]

Awesome work @brunobar79 this is good stuff. Glad we're supporting both HD paths, that will save support a giant headache :)

UI looks smooth, only small suggestions:

• We could swap the two helper paragraphs below HD path: start with "These are the accounts available..." then have "If you don't see your existing Ledger address..." It reads a little smoother that way, but I also realize the instructions are closest to the element they're talking about... your call.
• Rather than two full-width buttons (Connect with Trezor / Connect with Ledger) it might look nice with two side-by-side, I'll let @cjeria make the call on this one.
• When you successfully create a new account, is the default title LEDGER #1 ? Could we drop the # since other accounts are just numbered (Account 1, Account 2). Sorry my OCD made me say something 😬

Want me to reach out to Ledger about an affiliate link to add in with the Trezor one?

[brunobar79]

@bdresser That's great feedback!

RE: order of the paragraphs, I've played around and all the combinations had pro and cons like you said. I don't wanna say here's the list of your accounts and then show something else. Maybe we can move the paragraph "If you don't see your existing Ledger address(es) ..." under the account list.
connect. WDYT?

I like the side by side idea for the buttons. We could use the original Ledger and Trezor logos and remove the words "Connect to"

Good idea about removing the # on the account label. Just did that.

RE: Affiliate link, do it!

[metamaskbot]

Builds ready [fdf202e]: mascara, chrome, firefox, edge, opera

Comment was adequetly addressed.

[ghost]

@brunobar79 , nice work! Can you please add a "Fixes #4593" reference to the PR?

[cjeria]

The first connect screen is looking busier now with the added connect button. I agree with @bdresser maybe place the button side by side and add a very clear connect button like this:

On the Unlock screen, here's what I suggest we do:

Also remove the "#" symbol from the account names.

[metamaskbot]

Builds ready [b369560]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [51e4a6d]: mascara, chrome, firefox, edge, opera

[danfinlay]

Reviewed as a team, looks good!

[MicahZoltu]

Is this chrome only, or also FF and Edge?

[gre]

great work!

[gnarco]

Hi

I just bought a second ledger to to some test and connect it to Metamask.

Work great until I request on a dApps to sign a transaction.
It goes to metamask ask for confirmation, wait, and then the transaction appear in a "wait list".

I'm not on the main net but on a custom one. Perhaps it's the issue ?

Also, perhaps I'm in the wrong place for chatting about this, don't hesitate to redirect me :)

[eccentricexit]

Looks like this is still a thing and may not get fixed:
https://www.reddit.com/r/ledgerwallet/comments/gcdx1f/any_hope_for_eip712_support/

[Ktt155]

Should i creatva ledger wallet ?