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(*): add custom password prompt for KMD and Mnemonic #322

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

PatrickDinh
Copy link
Contributor

@PatrickDinh PatrickDinh commented Dec 3, 2024

There are scenarios where customising window.prompt are needed:

  • There are environments where window.prompt isn't supported, for example, Tauri app built for macOS
  • a better UI to display the message is required

To implement this change:

  • add options to customise the prompt to KMD and Mnemonic wallets
  • when the option isn't set, fallback to the default window.prompt

There are scenarios where customsing window.prompt are needed:
- window.prompt isn't supported, for example, on macOS WebKit
- a better UI to display the message
To implement this change, I have:
- add options to customise the prompt to KMD and Mnemonic wallets
- when the option isn't set, fallback to the default window.prompt
Copy link
Collaborator

@drichar drichar left a comment

Choose a reason for hiding this comment

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

The only change I would like to see is to have the promptForPassword property be optional. Then it would be a non-breaking change and anyone currently using KMD and Mnemonic wallets in their projects would not need to do anything to continue with the default behavior (using window.prompt).

Other than that it looks great.

Copy link
Collaborator

@drichar drichar left a comment

Choose a reason for hiding this comment

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

Ah I apologize! All the options are already optional due to Partial 😅

So yeah this looks good to merge. Thank you again for the contribution @PatrickDinh! I'll ship the next release with this feature.

@drichar drichar changed the title feat(*): support customising prompt for KMD password and Mnemonic phrase feat(*): add custom password prompt for KMD and Mnemonic Dec 5, 2024
@drichar drichar merged commit 0cfbc6d into TxnLab:main Dec 5, 2024
1 check passed
drichar added a commit that referenced this pull request Dec 5, 2024
Two tests that were added upstream in PR #322 instantiate wallets without the
`networks` property, introduced on the current `v4` branch. This change adds
the `networks` property to `MnemonicWallet` and `KmdWallet`, ensuring proper
initialization and tests passing.
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.

2 participants