-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix payment channels request #946
Conversation
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 good on both iOS and Android, QA Passed 👍
…tamask-mobile into fix-payment-channels-request
)} | ||
</View> | ||
</View> | ||
<Text style={styles.intro}>{strings('paymentRequest.is_requesting_you_to_pay')}</Text> | ||
<View style={styles.total}> | ||
<Text style={styles.totalPrice}> ${formattedAmount}</Text> | ||
<Text style={styles.totalPrice}>{formattedAmount} DAI</Text> |
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.
we have locale for this, strings('unit.dai')
i believe
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 really don't think there's a good reason to add DAI, ETH to locales since it doesn't have translation but if want to keep it consistent I'll do it.
}); | ||
|
||
PaymentChannelsClient.hub.on('payment::complete', () => { | ||
// show the success screen | ||
this.setState({ paymentChannelRequestCompleted: true }); | ||
// hide the modal and reset state | ||
setTimeout(() => { | ||
setTimeout(() => { |
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.
this is strange, why two settimeouts? you're calling one settimeout inside the other directly
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.
there are different timeouts
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.
You're right. My bad. Fixed it here: #957
paymentChannelRequestLoading: false, | ||
paymentChannelRequestInfo: {} | ||
}); | ||
setTimeout(() => { |
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.
did you do this on purpose? no delay time and you're setting the state two times in this callback
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 did it on purpose. The delay times are below (See here) The reason of it is that I need to give enough time the modal to close before changing the flag.
* fix * fixed payment channels requests * fixed payment channels requests * fix detail * increase timeout for hiding popup
This PR fixes and improves payment channels requests via deeplinks / qr codes.