backend: show estimated conversions for unconfirmed ETH txs and remove skeleton when conversions are missing. #3235
+24
−18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bznein
force-pushed
the
amount
branch
2 times, most recently
from
c0ae07d to
23d80b8
Compare
bznein
changed the title
bznein
force-pushed
the
amount
branch
3 times, most recently
from
d41de3b to
382b229
Compare
Beerosagos
reviewed
Mar 17, 2025
Beerosagos
requested changes
Mar 17, 2025
| )} | ||
| <span className={styles.txUnit}>{' '}{conversionUnit}</span> | ||
| ) : null } | ||
| { (conversion || sendToSelf) ? <span className={styles.txUnit}>{' '}{conversionUnit}</span> : null } |
There was a problem hiding this comment.
I think that we can move this inside the previous statement, wdyt?
There was a problem hiding this comment.
Not sure I follow, the logic for the if is slightly different here than from anywhere else. can you clarify what you mean please? :)
There was a problem hiding this comment.
Yes that is different, but if we decide that instead of showing the skeleton we want to hide the whole line (doesn't make sense to me to hide only the fiat conversion and show the unit), I think that the logic used to decide when showing the unit should be the same of the amount, and hence we don't need separate if statements.
There was a problem hiding this comment.
Ok I think I understood, LMK if it seems ok now :)
thisconnect
reviewed
Mar 17, 2025
Unconfirmed ETH transactions do not have a timestamp associated (while for BTC we store the created timestamp in the DB). So instead we use estimated conversions.
bznein
force-pushed
the
amount
branch
2 times, most recently
from
81e4805 to
8f0c2e4
Compare
The skeleton is meant to represent that we are waiting for a backend response. In this case the conversion is not available becase the data is missing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ETH transactions do not have a "Created" timestamp we can't provide an estimated conversion based on that. We use real-time conversion instead.
We also remove the skeleton for the cases in which a conversion is still not available:
Before asking for reviews, here is a check list of the most common things you might need to consider: