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

Fix BatchRequest.execute() multiple window issue #10423

Merged
merged 3 commits into from
Feb 11, 2021

Conversation

jpatel888
Copy link
Contributor

@jpatel888 jpatel888 commented Feb 11, 2021

Fixes: #10422

Explanation: The issue describes the problem with a sandbox, but this PR fixes multiple windows being opened on BatchRequest.execute()

@jpatel888 jpatel888 requested a review from a team as a code owner February 11, 2021 21:13
@jpatel888 jpatel888 requested a review from darkwing February 11, 2021 21:13
@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2021

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.

@jpatel888
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I noticed this problem recently as well, and had been meaning to create an issue for it.

This solution looks good overall! I just had one suggested change.

app/scripts/background.js Outdated Show resolved Hide resolved
Gudahtt
Gudahtt previously approved these changes Feb 11, 2021
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LTGM, thanks!

Edit: Oh, prettier is angry about something. LGTM after running yarn lint:fix then 😅

@jpatel888
Copy link
Contributor Author

LTGM, thanks!

Edit: Oh, prettier is angry about something. LGTM after running yarn lint:fix then 😅

I figured it was too easy for there not to be something lol, just added a commit, lmk if there's anything else.

Thanks

@Gudahtt Gudahtt merged commit 38078d7 into MetaMask:develop Feb 11, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 11, 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.

MetaMask opens multiple windows for BatchRequest.execute()
2 participants