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

Fix multiple click #1312

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Fix multiple click #1312

merged 5 commits into from
Nov 7, 2024

Conversation

aminsato
Copy link
Contributor

@aminsato aminsato commented Nov 6, 2024

Fixes #1178

@@ -514,10 +516,26 @@ internal class SendFormViewModel @Inject constructor(
)
} catch (e: InvalidTransactionDataException) {
showError(e.text)
} finally {
disableSendingForm()
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure that we should disable it here? what if this ends with error? looks like it will disable sending forever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I mean by ‍disableSendingForm‍‍‍‍ is that in this state, the form is not in the sending state. The finally section is always executed, even if an exception occurs. As a result, whether the operation completes successfully or an exception occurs, the form will be out of submission mode, and the user can submit it again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that's probably not the best naming then :) let's maybe call it showLoading and hideLoading for simplicity?

@aminsato aminsato requested a review from yvebe November 6, 2024 12:26
@@ -514,10 +516,26 @@ internal class SendFormViewModel @Inject constructor(
)
} catch (e: InvalidTransactionDataException) {
showError(e.text)
} finally {
disableSendingForm()
Copy link
Collaborator

Choose a reason for hiding this comment

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

that's probably not the best naming then :) let's maybe call it showLoading and hideLoading for simplicity?

Comment on lines 87 to 98
when (isBusy) {
true -> UiCirclesLoader(
color1 = textColor ?: appColor.oxfordBlue800
)

false -> content?.invoke() ?: Text(
text = text,
color = if (disabled == true) appColor.neutral800 else textColor
?: appColor.oxfordBlue800,
style = textStyle ?: Theme.montserrat.subtitle1
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's no need for when if conditions are true & false, just use if-else statement

@@ -28,7 +29,8 @@ internal fun MultiColorButton(
trailingIcon: Int? = null,
iconColor: Color? = null,
backgroundColor: Color? = null,
disabled: Boolean? = false,
isBusy: Boolean = false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
isBusy: Boolean = false,
isLoading: Boolean = false,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@aminsato aminsato requested a review from yvebe November 7, 2024 06:38
…screen

Added `launchSingleTop=true` to prevent multiple instances of the same screen from being opened. This change ensures that navigating to a destination that is already on top of the back stack will not create a new instance of that destination, thus preventing duplicate screens.
@aminsato aminsato requested a review from yvebe November 7, 2024 09:37
@yvebe yvebe merged commit e3cf740 into main Nov 7, 2024
1 check passed
@yvebe yvebe deleted the fix/multiple-click branch November 7, 2024 14:50
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.

[BUG] able to open UTXO verify screen multiple times
2 participants