-
Notifications
You must be signed in to change notification settings - Fork 492
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
[VDG] Close the Success dialog automatically #12312
[VDG] Close the Success dialog automatically #12312
Conversation
…se-the-dialog-automatically
…se-the-dialog-automatically
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.
cACK
The autoclosing should be also applied to the SuccessViewModel
.
And both view (SuccessView, AddedWalletPageView) should only contain an animating checkmark. Basically, exactly how is it in SuccessView (without the text).
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.
@wieslawsoltes Just found that |
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.
tACK
|
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.
tACK
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.
22a7418 the closure is too fast
I'm unable to read all the text which gives me bad UX
There shouldn't be text, just the checkmark. |
Remove text 284cfbe |
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.
first round of review
@@ -4,20 +4,15 @@ | |||
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" | |||
xmlns:c="using:WalletWasabi.Fluent.Controls" | |||
xmlns:vm="using:WalletWasabi.Fluent.ViewModels.Wallets.Send" | |||
xmlns:views="clr-namespace:WalletWasabi.Fluent.Views" | |||
mc:Ignorable="d" | |||
x:DataType="vm:SendSuccessViewModel" | |||
x:CompileBindings="True" | |||
x:Class="WalletWasabi.Fluent.Views.Wallets.Send.SendSuccessView"> | |||
<c:ContentArea Title="{Binding Title}" |
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.
Title can be removed
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.
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.
That commit doesn't remove the title from this view.
WalletWasabi.Fluent/ViewModels/Wallets/Send/SendSuccessViewModel.cs
Outdated
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.
Final review:
- Don't forget this: [VDG] Close the Success dialog automatically #12312 (comment)
- Can you create a variable for the delay time and use that everywhere?
- (or make the other dialogs to inherit from SuccesViewModel, and just have the delay there?) Up to you.
- Please increase the delay time to 1000s ([VDG] Close the Success dialog automatically #12312 (comment))
|
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.
tACK
Fixes #11568