Skip to content

Fix UI sync#16612

Merged
jpuri merged 2 commits intomv3-loginfrom
mv3-login-fix
Nov 21, 2022
Merged

Fix UI sync#16612
jpuri merged 2 commits intomv3-loginfrom
mv3-login-fix

Conversation

@jpuri
Copy link
Copy Markdown
Contributor

@jpuri jpuri commented Nov 21, 2022

PR addresses comment: #15558 (comment)

@jpuri jpuri requested review from Gudahtt and darkwing November 21, 2022 18:04
@github-actions
Copy link
Copy Markdown
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.

this.on('startUISync', startUISync);
if (this.startUISync) {
this.emit('startUISync');
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In case event listeners were not created when this.emit('startUISync') was called, code above will take care to send event to UI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: this might make more sense reformatted as something like

if (this.startUISync) {
  startUISync()
} else {
  this.on('startUISync', startUISync);
}

e.g. call sync now, otherwise schedule for later. As we don't need the event listener at all if the UI sync is already prepared.

@jpuri jpuri marked this pull request as ready for review November 21, 2022 18:28
@jpuri jpuri requested a review from a team as a code owner November 21, 2022 18:28
Copy link
Copy Markdown
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.

LGTM!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [6acfd0b]
Page Load Metrics (1957 ± 128 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint92167107199
domContentLoaded157526971947271130
load157526971957267128
domInteractive157526971947271130
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 23035 bytes
  • ui: 6296 bytes
  • common: 483 bytes

@jpuri jpuri merged this pull request into mv3-login Nov 21, 2022
@jpuri jpuri deleted the mv3-login-fix branch November 21, 2022 19:10
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2022
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.

3 participants