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

Copy to clipboard warning #3488

Merged
merged 7 commits into from
Jun 22, 2023

Conversation

kkosiorowska
Copy link
Contributor

@kkosiorowska kkosiorowska commented Jun 19, 2023

Closes #3468

What

When users try to copy a recovery phrase/private key we should warn them that it's not safe to do so.

UI

Screen.Recording.2023-06-21.at.10.43.32.mov

Testing

  • Try to copy a recovery phrase/private key, warning should warn that it's not safe to do.
  • Check the "Don’t ask me again" option. The warning should not be displayed again.

Latest build: extension-builds-3488 (as of Thu, 22 Jun 2023 06:03:38 GMT).

@kkosiorowska
Copy link
Contributor Author

@VladUXUI Please do some design reviews.

Please confirm which background color the checkbox should have if it is not checked.

Screenshot 2023-06-19 at 16 12 51

@VladUXUI
Copy link
Contributor

@kkosiorowska this doesn't look like the correct checkmark.
We have a component for this already created i think it's called .checkbox_box (but i might be wrong)

@kkosiorowska
Copy link
Contributor Author

@kkosiorowska this doesn't look like the correct checkmark. We have a component for this already created i think it's called .checkbox_box (but i might be wrong)

I have checked the code. I think you mean the one from the ledger accounts import page. I'll fix it 😀

Screenshot 2023-06-20 at 12 20 40

Comment on lines 31 to 34
const [showWarning, setShowWarning] = useLocalStorage(
MODAL_COPY_WARNING_ID,
getLocalStorageItem(MODAL_COPY_WARNING_ID, "true")
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can (and should) use the new DismissableItem interface in preferences introduced last week. See #3464 for usage!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jagodarybacka
Copy link
Contributor

Should we add the warning to copy recovery phrase button during onboarding (new wallet onboarding)?

@kkosiorowska
Copy link
Contributor Author

Should we add the warning to copy recovery phrase button during onboarding (new wallet onboarding)?

@VladUXUI Could you clear up any doubts?

@Shadowfiend
Copy link
Contributor

Let's not block this merge on that regardless, fwiw.

)

const [isOpen, setIsOpen] = useState(false)
const [isConfirmed, setIsConfirmed] = useState(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's name this what it is, shouldNotDisplayAgain or isDontAskAgainChecked or something like that. Right now this is the “copy to clipboard” component, so “is confirmed” implies you're confirming the copy to clipboard—which is actually happening differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kkosiorowska kkosiorowska self-assigned this Jun 22, 2023
Copy link
Contributor

@jagodarybacka jagodarybacka left a comment

Choose a reason for hiding this comment

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

Looks good let's 🚢

@jagodarybacka jagodarybacka merged commit c05f338 into keyring-with-pk Jun 22, 2023
@jagodarybacka jagodarybacka deleted the copy-sensitive-material-warning branch June 22, 2023 08:38
kkosiorowska pushed a commit that referenced this pull request Jun 26, 2023
### What

Compiled refactors of the Checkbox component from
#3455 and
#3488 as these are not going
to be merged to `main` soon.

- new styles
- allows checkbox to be disabled
- allows checkbox to show error message
- allows to display label on both sides of the checkbox

On this branch (main) checkbox is not used anywhere right now.

Latest build:
[extension-builds-3495](https://github.com/tahowallet/extension/suites/13815587027/artifacts/766623220)
(as of Fri, 23 Jun 2023 11:09:22 GMT).
@kkosiorowska kkosiorowska mentioned this pull request Jul 13, 2023
kkosiorowska pushed a commit that referenced this pull request Jul 14, 2023
## What's Changed
* Add private key onboarding flow by @jagodarybacka in
#3119
* Private key JSON import by @jagodarybacka in
#3177
* Allow export of private keys and mnemonics by @jagodarybacka in
#3248
* Export private key form by @jagodarybacka in
#3255
* Unlock screen for the account backup by @kkosiorowska in
#3257
* Show mnemonic menu by @jagodarybacka in
#3259
* Fix background blur issue by @jagodarybacka in
#3265
* Account backup UI fixes by @jagodarybacka in
#3270
* Fix unhiding removed accounts by @jagodarybacka in
#3282
* New error for incorrectly decrypted JSON file by @jagodarybacka in
#3293
* Export private keys from HD wallet addresses by @jagodarybacka in
#3253
* Refactor keyring redux slice to remove `importing` field by
@jagodarybacka in #3309
* 📚 Accounts backup by @kkosiorowska in
#3252
* Catch Enter keypress on Unlock screen by @jagodarybacka in
#3355
* Rename `keyring` to `internal signer` and other improvements by
@jagodarybacka in #3331
* 🗝 QA - Accounts backup and private key import by @jagodarybacka in
#3266
* Remove private key signers if they are replaced by accounts from HD
wallet by @jagodarybacka in
#3377
* RFB 4: One-Off Keyring Design by @Shadowfiend in
#3372
* Copy to clipboard warning by @kkosiorowska in
#3488
* Allow setting custom auto-lock timer by @hyphenized in
#3477
* Use Argon2 for encrypted vaults by @jagodarybacka in
#3502
* 👑 Private keys import and accounts backup by @jagodarybacka in
#3089
* Untrusted assets should not block the addition of custom tokens by
@kkosiorowska in #3491
* Flip updated dApp connections flag by @Shadowfiend in
#3492
* v0.41.0 by @Shadowfiend in
#3531
* Switch to a given network if adding a network that is already added.
by @0xDaedalus in #3154
* Remove waiting for Loading Doggo component in E2E tests by
@jagodarybacka in #3541
* Squeeze content to better fit on Swaps page by @jagodarybacka in
#3542
* Refactor of terms for verified/unverified assets by @kkosiorowska in
#3528
* Fix ChainList styling by @fulldecent in
#3547
* Update release checklist by @jagodarybacka in
#3548
* Fix custom asset price fetching by @hyphenized in
#3508
* Sticky Defaults: Make Taho-as-default replace MetaMask in almost all
cases by @Shadowfiend in
#3546

## New Contributors
* @fulldecent made their first contribution in
#3547

**Full Changelog**:
v0.41.0...v0.42.0

Latest build:
[extension-builds-3549](https://github.com/tahowallet/extension/suites/14268975651/artifacts/801826435)
(as of Thu, 13 Jul 2023 09:51:56 GMT).
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.

4 participants