Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if ledger was successfully able to establish transport on confirm screen mount #12535

Merged
merged 4 commits into from
Nov 4, 2021

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Oct 29, 2021

For Ledger users who connect with WebHID, on some operating systems the connection of the ledger device to other software can prevent the WebHID transport from successfully being created. This PR adds a message to the user if that state is detected.

Depends on MetaMask/eth-ledger-bridge-keyring#125 and MetaMask/eth-ledger-bridge-keyring#126

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [d3f0b42]
Page Load Metrics (330 ± 31 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint2865193316431
domContentLoaded2765083216431
load2855173306531
domInteractive2765083216431

highlights:

storybook

@@ -1549,6 +1553,11 @@ export default class MetamaskController extends EventEmitter {
return keyring;
}

async attemptLedgerTransportCreation() {
const keyring = this.getKeyringForDevice('ledger');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an await:

const keyring = await this.getKeyringForDevice('ledger');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is addressed since my latest rebase

@danjm danjm force-pushed the ledger-connection-failure-warning branch from d3f0b42 to 4d04e74 Compare November 2, 2021 07:23
@metamaskbot
Copy link
Collaborator

Builds ready [4d04e74]
Page Load Metrics (343 ± 40 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint2855613448440
domContentLoaded2755473338340
load2855553438240
domInteractive2755473338340

highlights:

storybook

@danjm danjm force-pushed the ledger-connection-failure-warning branch from a6239ea to 4a0d6bf Compare November 2, 2021 08:10
@danjm danjm marked this pull request as ready for review November 4, 2021 13:30
@danjm danjm requested a review from a team as a code owner November 4, 2021 13:30
@danjm danjm requested a review from NiranjanaBinoy November 4, 2021 13:30
@danjm danjm force-pushed the ledger-connection-failure-warning branch from e849811 to e40d909 Compare November 4, 2021 13:37
@metamaskbot
Copy link
Collaborator

Builds ready [e40d909]
Page Load Metrics (581 ± 68 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint36887658013967
domContentLoaded35587957414670
load36687958114168
domInteractive35487857414670

highlights:

storybook

@danjm danjm merged commit 843bb6e into develop Nov 4, 2021
@danjm danjm deleted the ledger-connection-failure-warning branch November 4, 2021 18:19
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants