-
Notifications
You must be signed in to change notification settings - Fork 7
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: Settings page, User agreement, Chat UI improvements, Dialogs #179
Conversation
I think that is already in place.
Ignore for now the reputation based limits. It has changed and we will release very soon.
Have you published all libs? |
@nostrbuddha I'm using this bisq2 commit from where I generated the jars and I run both the seed and the http-server
|
.../src/commonMain/kotlin/network/bisq/mobile/presentation/ui/components/layout/ScrollLayout.kt
Show resolved
Hide resolved
.../presentation/src/iosMain/kotlin/network/bisq/mobile/presentation/ui/helpers/TimeProvider.kt
Show resolved
Hide resolved
...in/kotlin/network/bisq/mobile/presentation/ui/uicases/take_offer/TakeOfferReviewPresenter.kt
Show resolved
Hide resolved
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.
Much expected PR :) - great job @nostrbuddha !
We've got several different features included here, so I'll do the review separately:
First addressing your question on validations, yes the implemented settings example looks great and it would be awesome to use that type of component in all the validations related to the Trading flow. Only comment is that if I change the language the validation stays in the original language (won't auto refresh)
Now:
Android Node
- all working as described, including the code examples
- I fixed a small navigation back on the new Dialog (please review 🙏 )
- Re the agreement, is working great even if the user has already done the onboarding.
- i18n change applies immediately in the settings screen and works nice. Some issues are: the combo box shouldn't show languages we don't have i18n for
- minor UI issues on the settings screens (spacing, looks too compressed. This can be left for polishing)
** xClients **
- Settings: Henrik commented he believes the settings APIs are there, could you please check to see if we can merge this complete?
- i18n: it would be necessary to completely remove all the lyricist stuff and apply the new i18n everywhere. Do you think we could do that in this PR?
Regarding the stuff that is incomplete (xClients settings & i18n application):
- i18n Applications #150 -> shall we do it in this PR? Otherwise let's remove all options but english from the combo box to not give false expectations in the settings.
- xClients settings, would you like to finish this in this PR ?
Please let me know and we'll move from there 🙏
The idea behind it is to merge working complete things ideally or at least be very conscious on what we merge that we commit to finish soon or if not leave a coherent implementation (no confusing functionality)
Cheers!
|
makes sense buddha, I'll wait for your next update then to do a 2nd pass! |
One more comment for the bisq settings @nostrbuddha , the "use animations" toggle would be useful actually because we have the avatar animation and also some default KMP animations we could disable if the user un-toggle the it (we could default to true). |
…oted message on click
…rvice) that wraps this Repository
…otifications removed
…ontent with an invisible overlay box that consumes all clicks and interactions
…de, LanguageAPIGateway (tested in androidClient)
…issue on iOS with new useAnimation setting
Buddha needed to add a bisq2 WS API to accomplish the bisq2 settings for the profile, so in order to test bisq2 http-api project instance needs to be used from here -> https://github.com/nostrbuddha/bisq2/tree/lang_ws |
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.
All the "General Settings" stuff needs more work. We agreed with @nostrbuddha to comment that out for now until it gets fixed and merge this PR since the rest is working nicely.
Also since now we are doing fully fledged features instead of just UI PRs let's try to do PRs that target only one feature at the time (can send multiple at the time if you will)
Thanks! 🙏
* - Seller reputation warning dialog * - User agreement * - agreementAccepted - Moved to Settings model from User model * - TakeOffer Review: Progress Dialog and Success Dialog * - Chat Improvements: Current time; Quoted message trim; Scroll to quoted message on click * - Settings screen UI 1/2 * - fetch i18n_codes from Bisq2.LanguageRepository (Coulndt find/No service) that wraps this Repository * - Settings screen UI 2/3 * - User Agreement flow uses settingsServiceFacade, rather than settingsRepository * - Settings - Backend integration 1 * - searchable dropdown for supported languages; useAnimations, tradeNotifications removed * - Multi select for Dropdown with Chip control * - langugeCode, supportedLanguages in androidNode * - Settings page - Remaining controls wired functionally in androidNode * - update languagePairs from LanguageServiceFacade, when app language changes * - TextField validation - Implemented in SettingsScreen * - Text field validation for TrustedNodeSetupScreen.RemoteBisqURL * - isInteractive handled in Scaffolds, to block the entire scaffold content with an invisible overlay box that consumes all clicks and interactions * - show PoW control only in androidNode; println, deadcode cleanup * - Settings Page functionality for xClients; With LanguageServiceFacade, LanguageAPIGateway (tested in androidClient) * - TextField validations 1/2 * - TextField validations 2/2 * - useAnimations - to be accompanied with changes in bisq2 repo * - Cleanup * - fix pod files references (bug from previous PR) + fix compilation issue on iOS with new useAnimation setting * - hiding new general settings as they need more work (agreed with buddha) --------- Co-authored-by: Rodrigo Varela <[email protected]>
Here is my PR after a long delay!
BisqTextField
and implemented it in GeneralSettingsPage and TrustedNodeSetupScreen.PS:
Couldn't test CreateOffer/TakeOffer after rebase and pulling from bisq2 latest