-
Notifications
You must be signed in to change notification settings - Fork 167
Implement loader - syncing chain screen #430
Conversation
tanx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I added some minor style adjustments and made a few comments
src/store.js
Outdated
| walletUnlocked: false, // Is the wallet unlocked | ||
| lndReady: false, // Is lnd process running | ||
| syncedToChain: false, // Is lnd synced to blockchain | ||
| percentSynced: 0, // Expects 0-100 range |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it wouldn't be easier to use a float here between 0 and 1. generateArc divides it by 100 anyway:
lightning-app/src/component/spinner.js
Lines 145 to 151 in 4bf9624
| const generateArc = (percentage, radius) => { | |
| if (percentage === 0) { | |
| percentage = 1; | |
| } else if (percentage === 100) { | |
| percentage = 99.999; | |
| } | |
| const a = percentage * 2 * Math.PI / 100; // angle (in radian) depends on percentage |
| LoadNetworkSpinner.propTypes = { | ||
| percentage: PropTypes.number.isRequired, | ||
| msg: PropTypes.string.isRequired, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why move the component out of the component library? Seems to me it just makes things more complicated if we decide to reuse the spinner somewhere else.
5cd9d9a to
d9c9791
Compare
tanx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Just made a few minor changes.
Closes #255