Include password instructions under wallet settings #12207
Conversation
|
If anyone has an idea an how to make the styling better, feel free to directly push on the PR. |
Co-authored-by: Max Hillebrand <30683012+MaxHillebrand@users.noreply.github.com>
There was a problem hiding this comment.
In general I think we should change this feature completely, and it probably shouldn't be in the settings view, but meanwhile we can at least improve it.
For the PR I think we should have a separate text block for the password.
If you agree just apply my code suggestions below.
PR:
My suggestion:
|
I don't really understand what is the goal here, because If we want to mitigate the 13th word/passphrase issue, which is about highlighting that the password is needed for recovering the wallet, then this is not the right place to mention it. We tend to keep adding more and more stuff in order to make things better. In this example, I think the only problem is the title that says: Also the 13th word/passphrase issue should be really fixed elsewhere because I doubt the majority reads this. Somewhere during wallet creation. |
The goal is to educate our users as much as possible that both the recovery words and the passphrase/password are needed to recover their wallet.
Such a feature to verify only half of the backup was a mistake. Some users already don't know that the password is necessary to recover their funds and this feature as it is just adds more confusion. As I have mentioned above a better solution is to redesign this completely but that would take more time for sure, so at least adding a note about the password is better than doing nothing.
We should not make a separation between the recovery words and the password when verifying the wallet backup. |
|
Highly agree with every point made by @yahiheb Comment from the original PR:
If a user chooses a secure password like FGHr298ur23g8yXNIOAW$Fh98723 and saves locally to their device so they don't have to remember it, they will be shocked to find they cannot recover their funds using the 12 words they made sure to verify if the device holding their password is broken or stolen. |
Okay, than the actual fix would be to add the message that you proposed here, and also to ask for the PW. Just to be clear... The situation that we have here tends to happen time by time. Someone picks an issue and does a not proper/half work. Then either ask someone else to finish it or not even ask. This is not good. The other thing that is risky is picking a task from the UX board that is in the triage column. Those items are not even prioritized in terms of when to start the work on them, and moreover, those items do not even have an approved design, so obviously will raise a contradiction that will waste the time of many of us. Long story short... first the concept needs to be approved, then start to work and implement it completely. Now from my side checking for PW too and adding the extra message is ok. IF you guys are also happy with it, you can start implementing it. If you can't @Szpoti or @adamPetho can give a hand. |
|
Such UX issues might never get any priority to get fixed, so while waiting for a redesign of the feature that might take months or even years we can at least improve it a bit, this is the purpose of this small PR I guess. |
Co-authored-by: yahiheb <52379387+yahiheb@users.noreply.github.com>
Co-authored-by: yahiheb <52379387+yahiheb@users.noreply.github.com>
|
If everyone is satisfied, I will merge the PR this afternoon. Please scream if you think otherwise. |
There was a problem hiding this comment.
TBH I am not happy with this and I also was thinking about asking for PW when doing the recovery word verification, that doesn't make sense either as I stated here.
Reasoning:
- The feature was built to only verify the recovery words, and not the recovery words + password. As its button says: Verify Recovery Words.
- Mentioning that the password is needed for the backup is unnecessary here, because:
- The feature is only about verifying the recovery words.
- It does not solve the original issue, if you really want to make the users understand the functionality of password, then it should be explained where they create it!
- Not all users come here to be educated by this.
The actual fix would be to rephrase the title of the section. #12207 (comment) And then fix the actual issue around password #10408, which is already prioritized in 2.0.7.
By all of these, I am sure this PR just adds unnecessary text to somewhere it doesn't belong to.
I don't want to be a blocker so @MaxHillebrand please read my comment and put an end to this. @adamPetho you can merge after that if needed.
I replied to that comment already: #12207 (comment) A user should never be able to remember their password. If a password is short and simple enough to be remembered, then it is easy enough to be guessed. |
Fixes #11539