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 recover wallet cursor position being stuck at end of input field #1667

Merged

Conversation

Projects
None yet
3 participants
@varsnotwars
Copy link
Contributor

commented Jun 29, 2019

Removed unnecessary subscription that always set the cursor position to the end of the list.

Fixes: #1429

@varsnotwars varsnotwars changed the title Remove not needed subscription that set cursor to itself Fix recover wallet cursor position being stuck at end of input field Jun 29, 2019

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented Jun 30, 2019

I don't think it's good. Either @lontivero or @molnard was hacking around a larger bug with this.

@varsnotwars

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

I wasn't able to replicate anything buggy after removing this subscription. @lontivero, @molnard could you advise, please 🙏

@danwalmsley

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

I think you might be looking for removing Mode=TwoWay from here?

https://github.com/zkSNACKs/WalletWasabi/blob/master/WalletWasabi.Gui/Tabs/WalletManager/RecoverWalletView.xaml#L15

instead of removing that.

@varsnotwars

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

I don't think it's good. Either @lontivero or @molnard was hacking around a larger bug with this.

@nopara73,

So looking at the MR that brought in this work I found this comment:

  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.

And here in the OnAddWord method you can see that the caret is set to the end of the mnemonic word list after a word is added:

So to me the subscription can be removed? Or line 245 can be removed as they both seem to be doing the same thing.

If not, I have updated the MR with the changes recommended by Dan.

Thanks @danwalmsley.

nopara73 added some commits Jul 3, 2019

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented Jul 3, 2019

Thanks @varsnotwars ACK for removing. In fact we use this code at 3 places: recovery, receive, send. They were quite inconsistent:

  • recovery and receive had subscription, send didn't
  • recovery and send had Mode=TwoWay, receive didn't
  • they also slightly differed in their implementation regarding null checks

@nopara73 nopara73 merged commit 8a51e00 into zkSNACKs:master Jul 3, 2019

1 of 4 checks passed

Wasabi.Linux in progress
Details
Wasabi.Osx in progress
Details
Wasabi.Windows in progress
Details
CodeFactor No issues found.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.