Ui: Trade flow UI and improvements in Take Offer #90

nostrbuddha · 2024-12-04T11:53:30Z

Take offer screens: Select Amount, Payment Method, Review Screens - Improved
Trade flow UI, along with necessary components
Offer card UI

Pushing this, so functional integration can start happening.

This PR covers 90% of the Trade flow UI. Some pending items are

Trade completed popup
Request mediation popup
Link buttons to Wallet guide, Trade guide, etc.,
Floating chat icon

…sted in small screen devices; BisqTextField improvements

…BisqSlider; SvgImage; AmountSelector

- Take offer screens - Added Interfaces - OfferCard proper UI - Layout spacing improvements - RootNavGraph -> Optimised routes definition and improved popBack nav animation - i18n Improvements

rodvar · 2024-12-04T23:00:33Z

rebased to fix compilation issue on ios

rodvar

@nostrbuddha this is starting to feel like we are in the real app :D

First comment is that its been a pleasure to test this one, working great on all 3 apps.

Besides my code comments, here are some functional things I would suggest to work on before merging this one:

"Close Trade" button at the end of the workflow should close the whole trading workflow process and go back to the original tab where all started which is buy/sell (eventually we can redirect to my trades when we have support for viewing completed trades)
Trade Amount Slider should:
- be fixed if the offer is for a fixed price
- allow values between min/max if the offer has a min max for trade
- show only 8 decimals (for BTC, min trade is 1 SAT) or 11 decimals (for lightning mSAT is allowed) accordingly to the trade
there is a glitch in the transition animation where sometimes it would come from the top left. Couldn't figure out how to consistently reproduce it I'll create a separate card for it if I find it.

Hope this makes sense please ask otherwise. It is ok if you prefer to leave some of this stuff above for later on, just please add TODOs or create GH issues for it 🙏

There are also some UX stuff we could improve, like title text spacing for example, but we can do that later when we have the trading fully implemented.

...mmonMain/kotlin/network/bisq/mobile/presentation/ui/components/atoms/CircularLoadingImage.kt

...presentation/src/commonMain/kotlin/network/bisq/mobile/presentation/di/PresentationModule.kt

...ntation/src/commonMain/kotlin/network/bisq/mobile/presentation/ui/components/atoms/Slider.kt

...rc/commonMain/kotlin/network/bisq/mobile/presentation/ui/components/layout/StaticScaffold.kt

...ommonMain/kotlin/network/bisq/mobile/presentation/ui/components/molecules/DirectionToggle.kt

...network/bisq/mobile/presentation/ui/components/organisms/trades/TradeFlow01AccountDetails.kt

...otlin/network/bisq/mobile/presentation/ui/uicases/offers/takeOffer/PaymentMethodPresenter.kt

.../kotlin/network/bisq/mobile/presentation/ui/uicases/offers/takeOffer/ReviewTradePresenter.kt

rodvar · 2024-12-05T00:11:00Z

...c/commonMain/kotlin/network/bisq/mobile/presentation/ui/uicases/trades/TradeFlowPresenter.kt

+    }
+}
+
+// TODO: Should do Interface for this?


probably yes since this handles the wholw trade flow. This presenter is very important, it will be interacting with services and/or repositories to manage the real world data and views should be consuming what they need through the interface that this presenter will implement (see other existing examples but I'm sure you know what I'm talking about already :) )

Yeah. Got it

HenrikJannsen · 2024-12-05T15:41:04Z

Small comments:

... when we have support for viewing completed trades)

After a trade was closed we delete all data. The user can export trade data if needed, but we intentionally dont keep history to avoid that your privacy gets weakened by your trade peers who keep all the trades and in case that computer falls into the wrong hands your persona data is revealed as well (like name, bank account,...).

...for lightning mSAT is allowed

In Bisq 2 we do not support mSAT. Those tiny amounts would be irrelevant in fiat terms. we also round the Fiat amount to avoid decimals and the BTC amount gets then converted to that.

nostrbuddha · 2024-12-05T22:18:48Z

Thanks for the review @rodvar :-)

"Close Trade"

Doing this now

Trade Amount Slider should be fixed if the offer is for a fixed price

For an offer with fixed trade amount, we will skip this screen, similar to the flow in desktop app.

show only 8 decimals (for BTC, min trade is 1 SAT) or 11 decimals (for lightning mSAT is allowed) accordingly to the trade

For this, created a design for btc/sats display component in Figma, as cbeams suggested. Was planning to code this component next PR. Is that okay?

There is a glitch in the transition animation where sometimes it would come from the top left. Couldn't figure out how to consistently reproduce it I'll create a separate card for it if I find it.

Actually this happened in previous code, where transitions weren't properly defined for all routes. I thought that was fixed in this PR. Spent sometime now to try replicate this, but not happening!

There are also some UX stuff we could improve, like title text spacing for example, but we can do that later when we have the trading fully implemented.

Yeah sure. Once functionalities are done, I will do a last iteration of final UI polish.

rodvar · 2024-12-05T22:35:00Z

Small comments:

... when we have support for viewing completed trades)

After a trade was closed we delete all data. The user can export trade data if needed, but we intentionally dont keep history to avoid that your privacy gets weakened by your trade peers who keep all the trades and in case that computer falls into the wrong hands your persona data is revealed as well (like name, bank account,...).

...for lightning mSAT is allowed

In Bisq 2 we do not support mSAT. Those tiny amounts would be irrelevant in fiat terms. we also round the Fiat amount to avoid decimals and the BTC amount gets then converted to that.

thanks Henrik @HenrikJannsen !!

rodvar · 2024-12-05T22:36:45Z

Thanks for the review @rodvar :-)

"Close Trade"

Doing this now

Trade Amount Slider should be fixed if the offer is for a fixed price

For an offer with fixed trade amount, we will skip this screen, similar to the flow in desktop app.

show only 8 decimals (for BTC, min trade is 1 SAT) or 11 decimals (for lightning mSAT is allowed) accordingly to the trade

For this, created a design for btc/sats display component in Figma, as cbeams suggested. Was planning to code this component next PR. Is that okay?
> There is a glitch in the transition animation where sometimes it would come from the top left. Couldn't figure out how to consistently reproduce it I'll create a separate card for it if I find it.
Actually this happened in previous code, where transitions weren't properly defined for all routes. I thought that was fixed in this PR. Spent sometime now to try replicate this, but not happening!

There are also some UX stuff we could improve, like title text spacing for example, but we can do that later when we have the trading fully implemented.

Yeah sure. Once functionalities are done, I will do a last iteration of final UI polish.

Awesome @nostrbuddha - yep the BTC/SAT component should go on a different PR, but let's keep it lo prio for now until we have all the trading fully working, do you agree?

rodvar · 2024-12-05T22:45:02Z

Small comments:

... when we have support for viewing completed trades)

After a trade was closed we delete all data. The user can export trade data if needed, but we intentionally dont keep history to avoid that your privacy gets weakened by your trade peers who keep all the trades and in case that computer falls into the wrong hands your persona data is revealed as well (like name, bank account,...).

...for lightning mSAT is allowed

In Bisq 2 we do not support mSAT. Those tiny amounts would be irrelevant in fiat terms. we also round the Fiat amount to avoid decimals and the BTC amount gets then converted to that.

thanks Henrik @HenrikJannsen !!

@HenrikJannsen @nostrbuddha maybe we can have statistics like # trades done between what currencies, and things like that without compromising any sensitive info. Anyways, that's out of the way for now 👍
your comment on mSAT is spot on so we can keep 8 decimals for all 👍

- Close trade button logic - Fixed missed i18n keys - Removed number from Trade flow composables - Removed confusing comments

rodvar

@nostrbuddha Just checked the last push, this is pretty much ready to merge just the slider limit on the decimals is missing.

I think we'll merge it as is and do that fix later on the "polishing" times

nostrbuddha added 8 commits December 5, 2024 09:59

- Nav: Back button generalization

aa494bf

- layout improvements: consistent, better padding across screens; te…

a569cf2

…sted in small screen devices; BisqTextField improvements

- Takeoffer.TradeAmount 1/2; TakeOffer Scaffold and TopBar updates; …

57819b4

…BisqSlider; SvgImage; AmountSelector

Take offer screens UI 1/2; i18n keys - modularized

097d98b

- Offer card UI and some Layout Improvements

53cc6d0

Layout / UI Improvements

f798190

- Take offer screens - Added Interfaces - OfferCard proper UI - Layout spacing improvements - RootNavGraph -> Optimised routes definition and improved popBack nav animation - i18n Improvements

Trade flow screen UI 1/2

5992c4b

- i18n update and layout/margin improvements in Trade flow

550586e

rodvar force-pushed the ui4 branch from 053cfcc to 550586e Compare December 4, 2024 22:59

rodvar requested changes Dec 5, 2024

View reviewed changes

PR Review commits, Trade close dialog

76c5ddd

- Close trade button logic - Fixed missed i18n keys - Removed number from Trade flow composables - Removed confusing comments

rodvar approved these changes Dec 5, 2024

View reviewed changes

rodvar merged commit 849875d into bisq-network:main Dec 5, 2024
1 check passed

nostrbuddha mentioned this pull request Dec 10, 2024

For Cycle 65 bisq-network/compensation#1684

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ui: Trade flow UI and improvements in Take Offer #90

Ui: Trade flow UI and improvements in Take Offer #90

nostrbuddha commented Dec 4, 2024 •

edited

Loading

rodvar commented Dec 4, 2024

rodvar left a comment

rodvar Dec 5, 2024

nostrbuddha Dec 5, 2024

HenrikJannsen commented Dec 5, 2024

nostrbuddha commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar left a comment

Ui: Trade flow UI and improvements in Take Offer #90

Ui: Trade flow UI and improvements in Take Offer #90

Conversation

nostrbuddha commented Dec 4, 2024 • edited Loading

rodvar commented Dec 4, 2024

rodvar left a comment

Choose a reason for hiding this comment

rodvar Dec 5, 2024

Choose a reason for hiding this comment

nostrbuddha Dec 5, 2024

Choose a reason for hiding this comment

HenrikJannsen commented Dec 5, 2024

nostrbuddha commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar commented Dec 5, 2024

rodvar left a comment

Choose a reason for hiding this comment

nostrbuddha commented Dec 4, 2024 •

edited

Loading