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.

Sign up for GitHub

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

Jump to bottom

Handle suggested token resolved elsewhere #8910

Merged
merged 1 commit into from
Jul 3, 2020
Merged

Handle suggested token resolved elsewhere #8910

merged 1 commit into from
Jul 3, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 3, 2020

When a suggested token was resolved in a different window, the popup or notification UI could get stuck with an empty suggested token list, where either action would throw an error.

This case is now handled by either redirecting or closing the window, in the popup and notification cases respectively. This check is
performed on both component mount and update.

@Gudahtt
Handle suggested token resolved elsewhere
48654e6 
When a suggested token was resolved in a different window, the popup
or notification UI could get stuck with an empty suggested token list,
where either action would throw an error.

This case is now handled by either redirecting or closing the window,
in the popup and notification cases respectively. This check is
performed on both component mount and update.
@Gudahtt Gudahtt marked this pull request as ready for review July 3, 2020 16:01
@Gudahtt Gudahtt requested a review from a team as a code owner July 3, 2020 16:01
@metamaskbot
Copy link
Collaborator

Builds ready [48654e6]
Page Load Metrics (675 ± 68 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31108562813
domContentLoaded35590867414168
load35990967514168
domInteractive35590867314168

@Gudahtt Gudahtt mentioned this pull request Jul 3, 2020
Merged
whymarrh
whymarrh approved these changes Jul 3, 2020
View reviewed changes
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit b0014a9 into develop Jul 3, 2020
@Gudahtt Gudahtt deleted the handle-suggested-token-resolved-in-different-window branch July 3, 2020 16:16
@metamaskbot metamaskbot mentioned this pull request Jul 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brad-decker brad-decker brad-decker approved these changes

@whymarrh whymarrh whymarrh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Gudahtt @metamaskbot @whymarrh @brad-decker