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 send network change update to websites #3211

Merged
merged 2 commits into from
Sep 29, 2021

Conversation

andrepimenta
Copy link
Member

Description

Reflect network change on websites. This PR changes some of the logic about when we send a network update the websites.

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #???

@andrepimenta andrepimenta requested a review from a team as a code owner September 29, 2021 15:18
@andrepimenta andrepimenta added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Sep 29, 2021
@andrepimenta andrepimenta changed the title Account for network loading Fix send network change update to websites Sep 29, 2021
Copy link
Contributor

@rickycodes rickycodes left a comment

Choose a reason for hiding this comment

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

code changes look good

@@ -118,13 +119,18 @@ export class BackgroundBridge extends EventEmitter {
const publicState = this.getProviderNetworkState(memState);
Copy link
Contributor

@rickycodes rickycodes Sep 29, 2021

Choose a reason for hiding this comment

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

nit: we could destructure here

Suggested change
const publicState = this.getProviderNetworkState(memState);
const { chainId, networkVersion } = this.getProviderNetworkState(memState);

and then just refer to those instead

@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Sep 29, 2021
@Cal-L Cal-L merged commit 59145a7 into develop Sep 29, 2021
@Cal-L Cal-L deleted the fix/send-network-change-update branch September 29, 2021 19:22
@cortisiko cortisiko mentioned this pull request Sep 29, 2021
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants