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

Don't show ledger live option in advanced settings if using firefox #12494

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Oct 27, 2021

Addresses an issue found during QA of v10.4.0

Ledger Live doesn't work with firefox, so this option should not be selected in the advanced tab of settings when on firefox. This is how it works on prod.

@danjm danjm requested a review from a team as a code owner October 27, 2021 13:29
@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 [9f04aef]
Page Load Metrics (462 ± 65 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8787544015775
domContentLoaded34487644813866
load36087646213665
domInteractive34487644813866

highlights:

storybook

NiranjanaBinoy
NiranjanaBinoy previously approved these changes Oct 27, 2021
Copy link
Contributor

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

@darkwing
Copy link
Contributor

Do we want to go a step further and only allow U2f if it's Firefox? Mozilla refuses to implement WebHID: mozilla/standards-positions#459

@danjm
Copy link
Contributor Author

danjm commented Oct 27, 2021

@Gudahtt
Copy link
Member

Gudahtt commented Oct 27, 2021

It does seem silly to include a dropdown with only one entry. I hope we'll have more options in the future, but removing it makes sense for now.

@danjm
Copy link
Contributor Author

danjm commented Oct 27, 2021

As of the latest commit, the dropdown is just hidden when using firefox

@metamaskbot
Copy link
Collaborator

Builds ready [84575f0]
Page Load Metrics (292 ± 9 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint276362294199
domContentLoaded267348284199
load276360292199
domInteractive267348284199

highlights:

storybook

Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

nit: only if you have to push again, can we group the constant and helper imports together in the top of the file?

@danjm danjm merged commit 1fa4b5c into develop Nov 2, 2021
@danjm danjm deleted the ledger-live-setting-firefox branch November 2, 2021 07:24
@github-actions github-actions bot locked and limited conversation to collaborators Nov 2, 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.

6 participants