Skip to content
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

UI: Various improvements and fixes #142

Merged
merged 16 commits into from
Jan 7, 2025
Merged

Conversation

nostrbuddha
Copy link
Contributor

@nostrbuddha nostrbuddha commented Dec 26, 2024

Not able to test in iOS. Running iOS Client results in Missing resources exception for any Drawable resources. Any iOS config changed?

@rodvar
Copy link
Collaborator

rodvar commented Dec 27, 2024

let's strip the QR-related feature into a separate PR to decide if we merge later 🙏

Copy link
Collaborator

@rodvar rodvar left a comment

Choose a reason for hiding this comment

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

Cool, here is my first review hope it helps 🙏

  1. The (+) button works on all apps, but it doesn't look like a circle its more like a rounded square, is that on purpose?
  2. iOS crash on "Create Offer" when triggered from my trades
  3. xClients crash when trying to take an offer now coming from "start first trade" in My trades
  4. see code comments
  5. for this PR, when a trade is completed you can navigate back each of the trade workflow screens. That shouldn't happen, the whole workflow should be out of the stack when the trade is finished. We can create an issue and deal with it later if you prefer (this issue has not been introduced by this PR)

As usual, let me know if you need any help!

(2)
kotlin.UninitializedPropertyAccessException: lateinit property createOfferModel has not been initialized
Uncaught Kotlin exception:     at 0   presentation                        0x10828e17b        kfun:kotlin.Throwable#<init>(kotlin.String?){} + 119 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Throwable.kt:28:44)
   at 1   presentation                        0x10828752f        kfun:kotlin.Exception#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:23:51)
   at 2   presentation                        0x10828774f        kfun:kotlin.RuntimeException#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:34:51)
   at 3   presentation                        0x10828861f        kfun:kotlin.UninitializedPropertyAccessException#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:154:44)
   at 4   presentation                        0x1082c490b        kfun:kotlin.native.internal#ThrowUninitializedPropertyAccessException(kotlin.String){}kotlin.Nothing + 371 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt:81:11)
   at 5   presentation                        0x107de471f        kfun:network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferPresenter#<get-createOfferModel>(){}network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferPresenter.CreateOfferModel + 207 
   at 6   presentation                        0x107dd2a27        kfun:network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferDirectionPresenter#onViewAttached(){} + 207 
   at 7   presentation                        0x107e8002f        kfun:network.bisq.mobile.presentation.ViewPresenter#onViewAttached(){}-trampoline + 91 
   at 8   presentation                        0x107d8f4e7        kfun:network.bisq.mobile.presentation.ui.helpers.RememberPresenterLifecycle$lambda$0#internal + 239 
(3)
initializedPropertyAccessException: lateinit property createOfferModel has not been initialized
Uncaught Kotlin exception:     at 0   presentation                        0x10828e17b        kfun:kotlin.Throwable#<init>(kotlin.String?){} + 119 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Throwable.kt:28:44)
   at 1   presentation                        0x10828752f        kfun:kotlin.Exception#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:23:51)
   at 2   presentation                        0x10828774f        kfun:kotlin.RuntimeException#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:34:51)
   at 3   presentation                        0x10828861f        kfun:kotlin.UninitializedPropertyAccessException#<init>(kotlin.String?){} + 115 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:154:44)
   at 4   presentation                        0x1082c490b        kfun:kotlin.native.internal#ThrowUninitializedPropertyAccessException(kotlin.String){}kotlin.Nothing + 371 (/opt/buildAgent/work/ed783494cd2364bc/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt:81:11)
   at 5   presentation                        0x107de471f        kfun:network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferPresenter#<get-createOfferModel>(){}network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferPresenter.CreateOfferModel + 207 
   at 6   presentation                        0x107dd2a27        kfun:network.bisq.mobile.presentation.ui.uicases.offer.create_offer.CreateOfferDirectionPresenter#onViewAttached(){} + 207 

…ent. Having some other resource loading issue with iOS, not able to test.
…'; moved all files from 'offers' to 'offer' and from 'trades' to 'trade'
… TradeFlow nav happens; Snackbar warnings in PaymentMethod screen, when payment method is not selected
 - AmountScreen - Amount selector - Placed vertically center;
 - PaymentMethodScreen - Make use of PaymentMethodCard as done in Createoffer flow;
 - ReviewScreen - BisqUIConstants;

Misc UI Improvements
 - CreateOfferPaymentMethodScreen - Adjust spacing;
 - BisqAmount select - Comment on usage
@rodvar
Copy link
Collaborator

rodvar commented Jan 6, 2025

rebased to main

Copy link
Collaborator

@rodvar rodvar left a comment

Choose a reason for hiding this comment

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

@nostrbuddha great job man. Setting aside minor UI-related issues (spacing, padding etc) I think the most relevant thing we need to tackle is navigation related.

When a workflow is exited (completed or cancelled) we don't want it to remain in the stack. In other words, if the user tries to navigate back it shouldn't get back to the workflow which is the current behaviour in this PR.

There is also a more general back issue related to tab navigation, we probably want to override default back behaviour and send user to home tab first, and if they press back again then we do OS default back behaviour (backgrounding the app).

Does this make sense? I'm happy to tackle the latter in a different PR, my question is if you want to address the workflow related navigation in this PR or we do it in the next one?

@nostrbuddha
Copy link
Contributor Author

I will do this here in this PR and push

@nostrbuddha
Copy link
Contributor Author

Hi Rodvar.

Hope this fixes all back navigation issues.

Note: In iOS, I am yet to write back handler logic. So tested only in androidNode and androidClient now. Will create a new issue to implement BackHandler in iOS

Copy link
Collaborator

@rodvar rodvar left a comment

Choose a reason for hiding this comment

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

excellent job it works beautifully on Android! for iOS, its still ok but we need to work on it for the real device "back gesture". I've added some sample code that might be it and created an issue as well.

on iOS I got to a state where I was stuck in "offers" screen and couldn't get out of it. I think I'll comment that on that back issue now so we don't loose track of it. Thanks Buddha!

@rodvar rodvar merged commit 22d3cc6 into bisq-network:main Jan 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants