Page MenuHomePhabricator

[] Improve network upgrade banner

Authored by johnkuney on Tue, Nov 14, 22:15.


Group Reviewers
Restricted Project
rABC09e6fb2a379c: [] Improve network upgrade banner

The countdown for the network upgrade banner needed improvement. Im adding it as a seperate component here
It is following the same logic as the countdown on, and using the API

The basic idea is it fetches the api on load

  • if it fails it doesnt load the banner
  • if we get a response it sets the timestamp for the countdown, and get the blocksUntilUpgrade
  • once the timestamp is reached there is another method to start polling the API every 60s to get the blocksUntilUpgrade
  • Once this value is < 0 the network upgrade is considerd finished, the final text is shown and the polling stops
Test Plan

preview the site and check it looks good

testing the actual functionality is a bit more invloved as not sure I have time to add tests, but it can be done manually
by running npm run dev

  • add const TESTING_TIMESTAMP = + 10000; somewhere
  • replace setTimestamp(data.upgradeTimeStamp * 1000) with setTimestamp(TESTING_TIMESTAMP); in the useEffect
  • replace if ( newBlocksUntilUpgrade !== blocksUntilUpgrade && newBlocksUntilUpgrade !== null ) { setBlocksUntilUpgrade(newBlocksUntilUpgrade); }

with setBlocksUntilUpgrade(blocksUntilUpgrade - 1); in the polling useEffect

  • update the polling interval to a lower number if you like

This should run through every stage of the countdown and stop polling once its complete

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

bytesofman added a subscriber: bytesofman.
bytesofman added inline comments.
62 ↗(On Diff #43084)

add comments whereever you are using this suppressHydrationWarning prop ... seems unique to NextJS? I had to look this up, it's not immediately clear what's going on here

96 ↗(On Diff #43084)

Are and timestamp in the same units here? should be less than timestamp, if the upgrade is in the future.

Maybe this is just happening to work bc the server is giving a timestamp in seconds and returns milliseconds .. so you happen to be testing "did I get a response from the server"

This revision now requires changes to proceed.Tue, Nov 14, 23:41
62 ↗(On Diff #43084)

For sure, it is in the nextjs docs, but I think actually a React prop. But yeah added a comment with link to next doc

96 ↗(On Diff #43084)

So they are both in ms, note when the timestamp is set the api result is * 1000

Shouldn't it be greater than? if its more ms passed than it's later right?

add comment for supress hydrationprop

96 ↗(On Diff #43084)

oic, in this case you are just looking to do the blocks and not the timer. got it.

bytesofman added inline comments.
116 ↗(On Diff #43085)

if the server can handle a tighter interval, imo worth it. Could easily have a flurry of sub-minute blocks, bad UX for countdown watchers.

Say, 10s.

117 ↗(On Diff #43085)
This revision now requires changes to proceed.Wed, Nov 15, 00:27

changing polling interval to 10

This revision is now accepted and ready to land.Wed, Nov 15, 00:39
This revision was automatically updated to reflect the committed changes.