Small moonpay fixes #1164
Small moonpay fixes #1164
Conversation
| this.setState({ options }); | ||
| } | ||
| this.setState({ options }); | ||
| this.maybeProceed(); |
There was a problem hiding this comment.
It is unclear when preact updates the state after this.setState({ options }), use the 2nd argument of setState that is a callback when the state has updated: this.setState({ options }, this.maybeProceed);
or
this.setState({ options }, () => this.maybeProceed());
There was a problem hiding this comment.
Good catch. The preact docs suck though, there is nothing about setState?
https://preactjs.com/guide/v10/components
https://preactjs.com/guide/v10/api-reference
There was a problem hiding this comment.
| @@ -882,7 +882,7 @@ func (handlers *Handlers) getExchangeMoonpayBuySupported(r *http.Request) (inter | |||
| break | |||
| } | |||
| } | |||
| return acct != nil && exchanges.IsMoonpaySupported(acct.Coin().Code()), nil | |||
| return acct != nil && !acct.Offline() && exchanges.IsMoonpaySupported(acct.Coin().Code()), nil | |||
There was a problem hiding this comment.
Doesn't feel semantically correct. The endpoint is supposed to respond whether account's coin is supported. I don't believe you need an account to be "online" in order to get a valid response from IsMoonpaySupported.
There was a problem hiding this comment.
You don't have to have an online account to call IsMoonpaySupported(), but you need an online account to be able to use the account in moonpay. this is a quick fix / hack to not list offline accounts.
There was a problem hiding this comment.
I have an idea for a general fix after this:
A higher order component that basically does what account.tsx already does: initialize an account, show spinner while syncing, show 'offline' if offline, etc. Factor that out of account.tsx, apply it there and in moonpay.tsx (and possibly even in the portfolio).
Will add the TODO
| @@ -81,7 +81,7 @@ class Moonpay extends Component<Props, State> { | |||
| { height }: State, | |||
| ) { | |||
| const account = this.getAccount(); | |||
| if (!account) { | |||
| if (!account || moonpay.url === '' || moonpay.address === '') { | |||
There was a problem hiding this comment.
I think only the moonpay.url should be enough? The address is more like an informational field. I think we could even remove it. Initially, I was planning to show the address in the UI during the onramp flow but never got to it...
|
Sorry, forgot: typo in commit 3716f60: "this would only happen if he disclaimer ..." |
Offline accounts have a connection issue.
Previously this would only happen if he disclaimer was already dismissed. The dropdown would still be shown after you dismiss the disclaimers.
This case should never happen, but if it did, the moonpay iframe would inline the whole BitBoxApp again (iframe src="").
Otherwise the function can panic when accessing unused addresses.
|
@x1ddos added another commit to fix the init, PTAL. |
| @@ -900,6 +900,11 @@ func (handlers *Handlers) getExchangeMoonpayBuy(r *http.Request) (interface{}, e | |||
| if acct == nil { | |||
| return nil, fmt.Errorf("unknown account code %q", acctCode) | |||
| } | |||
|
|
|||
| if err := acct.Initialize(); err != nil { | |||
There was a problem hiding this comment.
Oh, I get it now. Before, it was in account handlers which I guess was initialized. But now in the main handlers, it might not have been initialized yet. Does that sounds right?
There was a problem hiding this comment.
(by "it" I meant getExchangeMoonpayBuy handler method)
There was a problem hiding this comment.
it was also not initialized before either.
it was accidentally initialized by the portfolio (chart.go), but if you click fast enough, it will not have gotten to Litecoin yet ;)
Still could not find the 'undefined' issue though...