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

Recovery wallet auto complete #810

Merged

Conversation

lontivero
Copy link
Collaborator

This PR is for improving the recovery wallet mnemonic suggestions, making the suggested words clickable.

image

@lontivero lontivero force-pushed the Features/Recovery-Wallet-AutoComplete branch from 42aa826 to 5dc35a2 Compare November 5, 2018 13:13
@lontivero
Copy link
Collaborator Author

@danwalmsley could you give me a hand, please? This code works okay but there are a couple of thing it would be good if you could review them:

  1. After autocompleting the mnemonic texbox by clicking on any suggestion the textbox caret doesn't move to the end of the text, keeping in the middle of the word. I've found a quite awful workaround by implementing a PutCursorAtTheEndTextBoxBehavior behavior to do that but it only works okay if I use a delay. This doesn't look correct to me but I didn't find a better way.

  2. After changing my first implementation by one that fit better with the mvvm pattern, i couldn't decide what is the best pattern to notify the parent viewmodel about a child selection event.

Copy link

@danwalmsley danwalmsley left a comment

Choose a reason for hiding this comment

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

Would just like you to consider if the Autocomplete box control is more suitable, if so it might allow better viewmodel code.

@@ -50,11 +52,10 @@ private void HandleAutoUpdate()
var text = textBox.Text;
var enteredWordList = text.Split(' ', StringSplitOptions.RemoveEmptyEntries);
var lastWorld = enteredWordList.LastOrDefault();

Choose a reason for hiding this comment

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

typo lastWord :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, you're right. I cannot listen the national inflation forecast report and program at the same time.

using System.Linq;
using System.Reactive.Disposables;
using System.Reactive.Linq;
using WalletWasabi.Gui.Tabs.WalletManager;

Choose a reason for hiding this comment

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

Iv never used it but Avalonia has an inbuilt auto completing textbox control, this might perhaps be more suitable?

https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Controls/AutoCompleteBox.cs

It is also in the control catalog if you want to evaluate the way it works.

@lontivero lontivero changed the title WIP - Features/recovery wallet auto complete Features/recovery wallet auto complete Nov 6, 2018
@lontivero lontivero changed the title Features/recovery wallet auto complete Recovery wallet auto complete Nov 6, 2018
@lontivero
Copy link
Collaborator Author

Update This can be merged now. Thanks @danwalmsley

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.

None yet

3 participants