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

feat: new ui for mnemonic onboarding #2991

Merged
merged 30 commits into from
Feb 27, 2024
Merged

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Jan 12, 2024

Describe the changes you have made in this PR

ui improvements for mnemonic onboarding

Link this PR to an issue [optional]

Fixes #2932

Type of change

(Remove other not matching type)

  • feat: New feature (non-breaking change which adds functionality)

Checklist

  • Self-review of changed code
  • Manual testing
  • Added automated tests where applicable
  • Update Docs & Guides
  • For UI-related changes
  • Darkmode
  • Responsive layout

Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

When there is no key yet and I click on "Generate new Master Key" I don't see the 3 list items explaining the backup of the Master Key:

image

They are currently only being shown when visiting the backup page (after generating). I think it would be useful to add these 3 points there as well.

image

Most users probably will never visit the backup page (as they confirm the backup while generating the key). cc @stackingsaunter

@stackingsaunter
Copy link
Contributor

@pavanjoshi914 here you have 2x illustrations in png and svg

Master Key Dark
Master Key Dark
Master Key Light
Master Key Light

@pavanjoshi914
Copy link
Contributor Author

@reneaaron we still need the designs for generating the master key page. @stackingsaunter is working on it i think!

@stackingsaunter
Copy link
Contributor

CleanShot 2024-01-18 at 12 10 05@2x

This icon is a png, here's svg:

face-surprise

There's also a copy duplication:
CleanShot 2024-01-18 at 12 12 02@2x

@stackingsaunter
Copy link
Contributor

stackingsaunter commented Jan 18, 2024

@pavanjoshi914 cc @reneaaron

you meant this right? (when user deletes their master key)
I'd like settings to look like this

User, when after deleting Master Key

image

The generation itself would look the same (and could happen after users click Continue on explenation page?)

I posted designs in the file as well

Other screens:

User with MK

image

User with MK, but imported Nostr wallet

image

@pavanjoshi914
Copy link
Contributor Author

@reneaaron there is separate pr for default master key generation. with this user will only see back everytime and generate master key screen will be only seen when user delete existing key and want to generate new key. #2988

shall we keep generate screen same as backup screen @stackingsaunter ? we don't have desings for this screen yet

@stackingsaunter
Copy link
Contributor

In my opinion yes, what would you change? @pavanjoshi914
What;s your opinion @reneaaron ?

@pavanjoshi914
Copy link
Contributor Author

image
image

@stackingsaunter
Copy link
Contributor

I though the goal was to autogenerate the key, does it make sense to make it separate from UI then? In this PR there's no autogeneration cc @reneaaron @pavanjoshi914

Some feedback:

  • As we discussed on Slack, it's better to revert to white bg in text field everywhere (also in seed fields)
  • I think we can delete the recovery phrase explanation points in "Import" screen, and keep them only in generate/backup screens
  • This is minor, but it would be great if we can have those inputs somehow aligned. If it's too hard then I'd say don't worry about it:

In Figma I did it by defining the width of number in front of the field the same size, so for double digits, and centered the numbers:

  • This may be unrelated to this PR and could get another one but I'd not use emojis in the notification bars. And instead use icons:

Maybe for now you can just delete the emoji

@pavanjoshi914
Copy link
Contributor Author

@stackingsaunter autogeneration is here #2988 there was separate issue so i created separate pr

@reneaaron reneaaron added the next-release To be included in the next release label Feb 8, 2024
static/assets/images/face_surprise_dark.png Outdated Show resolved Hide resolved
static/assets/icons/popicons/eye-crossed.svg Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
src/app/components/mnemonic/MnemonicDescription/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

tACK

@reneaaron reneaaron merged commit d8efb71 into master Feb 27, 2024
7 checks passed
@reneaaron reneaaron deleted the master-key-ui-improvements branch February 27, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-release To be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Master Key back up and import flows
3 participants