Skip to content

Conversation

@pandablue0809
Copy link
Member

ihomp

This comment was marked as outdated.

@Anna15170221
Copy link
Contributor

@ihomp @pandablue0809
what if we delete the word "offer" and change "for" to arrow?
somerthing like this but with XRP icon and cancel button
Screenshot 2025-10-01 at 20 42 40

@ihomp
Copy link
Member

ihomp commented Oct 10, 2025

@ihomp @pandablue0809 what if we delete the word "offer" and change "for" to arrow? somerthing like this but with XRP icon and cancel button Screenshot 2025-10-01 at 20 42 40

-> instead of for is good..
but we still need to indicate that buying 37600 - and paying for it max 0.96 BTC, so the worse rate we will have is 39.193 XRP for 1 btc, should be better in reality.

Buying means the user wants to buy exactly 37600 xrp - and he agrees to the worse rate of 39.193 for 1 btc, in reality it will be a better rate usually, he will get exactly 37600 xrp but will pay less than 0.96 xrp.

when selling - it's exact amount that user sells, doesn't now how much it will it get, but agrees for the minimum amoumt it will get, in reality it will get more than in the offer.

@pandablue0809 pandablue0809 requested a review from ihomp October 10, 2025 21:07
@ihomp
Copy link
Member

ihomp commented Oct 15, 2025

The code looks unoptimized.

type of offer.TakerGets === 'string'
divide / multiply

we shouldn't check if it's a native currency or iou
we shouldn't multiply by 1000000 etc

we already have lot's of functions to deal with amounts and formating in the utils.
we need to re-use them

On mobile:
the action button is not understable without the word actions
so, its better to move it to the last line, like

Action: Cancel the offer.

XRP doesn't need an icon, it can be just text: XRP

It's better to form offers like, so it will be understandable that the received/spent amount can be different than in the offer.

Selling 10 XRP, wants a minimum of 40 USD
Buying 40 USD can pay a maximum of 10 XRP

ihomp

This comment was marked as outdated.

ihomp

This comment was marked as resolved.

@pandablue0809
Copy link
Member Author

@ihomp
i fixed it.
plz review again. 🙏

@pandablue0809 pandablue0809 requested a review from ihomp October 31, 2025 09:24
<span>1 {payCurrency} = </span>
<span className="no-brake">
{typeof offer.TakerGets === 'string'
? niceNumber(divide(1, offer.quality * 1000000), 0, null, 2)
Copy link
Member

@ihomp ihomp Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use amountFormat for amount formating for bith native currencies and IOUs.

Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use amountFormat for amount formating for both native currencies and IOUs.

<span>1 {getCurrency} = </span>
<span className="no-brake">
{typeof offer.TakerGets === 'string'
? niceNumber(multiply(offer.quality, 1000000), 0, null, 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use amountFormat for amount formating for bith native currencies and IOUs.

@Bithomp Bithomp deleted a comment from pandablue0809 Nov 2, 2025
@Bithomp Bithomp deleted a comment from pandablue0809 Nov 2, 2025
@Bithomp Bithomp deleted a comment from Anna15170221 Nov 2, 2025
@Bithomp Bithomp deleted a comment from pandablue0809 Nov 2, 2025
@ihomp
Copy link
Member

ihomp commented Nov 2, 2025

we still need to have offer ids on the desktop, right now we have them only on the mobile

@pandablue0809
Copy link
Member Author

@ihomp
i fixed it.
plz review again 🙏

@pandablue0809 pandablue0809 requested a review from ihomp November 6, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants