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

move activation logic into token rates controller #8744

Merged
merged 1 commit into from
Jun 5, 2020

Conversation

brad-decker
Copy link
Contributor

I had previously witnessed an issue where token rates were not fetched until 3 minutes (current default interval in the token rates controller) after the user unlocked their account. I updated this by moving to a start/stop system instead of creating an interval on construction. This allows an immediate fetch of token rates when the user unlocks and then kicking off an interval that ticks every 3 minutes. When the account is locked the interval is cleared.

@brad-decker brad-decker requested a review from a team as a code owner June 4, 2020 19:26
@brad-decker brad-decker marked this pull request as draft June 4, 2020 19:26
@brad-decker brad-decker force-pushed the update-token-rates-controller branch from 50daa82 to a7f7f5a Compare June 4, 2020 20:54
@metamaskbot
Copy link
Collaborator

Builds ready [a7f7f5a]
Page Load Metrics (1054 ± 89 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaintNaNNaNNaNNaNNaN
domContentLoaded5261337105018488
load5281338105418589
domInteractive5261337105018488

@brad-decker brad-decker marked this pull request as ready for review June 4, 2020 21:37
@brad-decker brad-decker force-pushed the update-token-rates-controller branch from a7f7f5a to dffb1a1 Compare June 5, 2020 18:00
@metamaskbot
Copy link
Collaborator

Builds ready [dffb1a1]
Page Load Metrics (716 ± 77 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33127462010
domContentLoaded366102871316278
load367103071616177
domInteractive365102871316278

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.

LGTM!

@brad-decker brad-decker merged commit 456684e into develop Jun 5, 2020
@brad-decker brad-decker deleted the update-token-rates-controller branch June 5, 2020 18:36
Gudahtt added a commit that referenced this pull request Jun 10, 2020
* origin/develop: (35 commits)
  Delete unused InfuraController & tests (#8773)
  Permissions: Do not display HTTP/HTTPS URL schemes for unique hosts (#8768)
  Refactor confirm approve page (#8757)
  blocklisted -> blocked
  Update app/scripts/contentscript.js
  blacklist -> blocklist; whitelist -> safelist
  replace blacklist with blocklist
  Delete unused transaction history test state (#8769)
  fix-formatting-of-gif (#8767)
  Order accounts on connect page (#8762)
  add gif for loading dev build (#8766)
  Bump websocket-extensions from 0.1.3 to 0.1.4 (#8759)
  Fix prop type mismatch (#8754)
  use grid template to position list item (#8753)
  Fix account menu entry for imported accounts (#8747)
  Fix permissions connect close and redirect behavior (#8751)
  Refactor `TokenBalance` component (#8752)
  Fix 'Remove account' in Account Options menu (#8748)
  move activation logic into token rates controller (#8744)
  asset outdated warning inline on full screen (#8734)
  ...
Gudahtt added a commit that referenced this pull request Jun 10, 2020
The `TokenRatesController` was accidentally broken in #8744, when the
logic for starting and stopping polling was moved from the `isActive`
property to start/stop functions.

A reference to the now-obsolete `isActive` property was accidentally
left behind, resulting in no exchange rate updates.
Gudahtt added a commit that referenced this pull request Jun 10, 2020
The `TokenRatesController` was accidentally broken in #8744, when the
logic for starting and stopping polling was moved from the `isActive`
property to start/stop functions.

A reference to the now-obsolete `isActive` property was accidentally
left behind, resulting in no exchange rate updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants