Bip070 status monitoring #32
Conversation
Adding prop type and prelim txParam handling (badger-cash#30)
There was a problem hiding this comment.
Good progress on this, lots of small requests.
Largest thing is I think we need to re-think the price display especially in the BIP70 flows. Showing the price/value optionally there would be very useful, and keeping the price display optional and moving the BIP70 information elsewhere will keep the components concerns separated.
… from PriceDisplay, putting in own component
…tcoin.com invoices
There was a problem hiding this comment.
Broken: Display for sending SLP tokens not through BIP70
Missing: USD/Fiat prices for Price Display with BIP70 BCH requests.
Then just some smaller changes requested.
I think it could be valuable to do a once over on simplifying where the logic is handled, such that multiple flows can re-use the same simple logic, such as updating the token metadata.
It's confusing if there's 2 flows which are slightly different with special guards on them, ideally the updateTokenMetadata can be a simple flow which is the same regardless of BIP70 or not. Look at the tokenID => set the info.
| .add( | ||
| 'Default', | ||
| () => { | ||
| return <InvoiceTimer secondsRemaining={600} />; |
There was a problem hiding this comment.
Still seems broken, not seeing a knob or an example in the storybook anymore
NaN:NaN
| @@ -368,7 +505,7 @@ const BadgerBase = (Wrapped: React.AbstractComponent<any>) => { | |||
| this.startRepeatable(); | |||
| } | |||
|
|
|||
| if (tokenId !== prevTokenId) { | |||
| if (tokenId !== prevTokenId && !paymentRequestUrl) { | |||
There was a problem hiding this comment.
Can we do all the setupCoinMeta in the same way as before, and just set the relevant fields in the paymentRequest setup step?
This would allow the setupCoinMeta to stay simple, and handle the payment request added complexity locally in the payment request flows.
There was a problem hiding this comment.
The code must over-ride props if the invoice SLP type does not match the prop type. This could be handled by setting tokenId in both props and state; exporting the state version, and having BadgerButton and BadgerBadge only read these and not the props version. I think this would be more complicated than what we have right now. Right now, program logic is the same for cases with no paymentRequestUrl parameter. If paymentRequestUrl is set, then information from the invoice over-rides anything in props (should be this way to prevent anyone from using badger-react-components to display incorrect information about an invoice by entering misleading props).
Let me know thoughts on improving this. I don't think the current implementation is perfect, but I wanted to make sure that in the case of any invoice being displayed, the display information can come only from that invoice.
|
|
… props even for invoices without websocket support
pay.bitcoin.cominvoices only)PriceDisplaycaused by previous errors