Skip to content

chore: rename wallet vault keycard references to safe + update keycard copy/formatting#9242

Merged
s84krish merged 1 commit into
masterfrom
wcn-1193-rename-to-safes
Jul 13, 2026
Merged

chore: rename wallet vault keycard references to safe + update keycard copy/formatting#9242
s84krish merged 1 commit into
masterfrom
wcn-1193-rename-to-safes

Conversation

@s84krish

@s84krish s84krish commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

WCN-1193

Description

Renaming relevant keycard generation types/references as part of the Wallet Vaults -> Wallet Safes product rename. Also includes formatting and copy updates to reflect proper wording when keycard is a safe/wallet keycard.

  • Updates copy in keycard to indicate 'Safe password' instead of generic 'Wallet password' when it is a safe keycard
  • Moves the you will need to put all Parts together for the full key note underneath the QR codes

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

Generated Safe keycard and Wallet keycard to verify generation is working properly using mock data.

safe-keycard.pdf

wallet-keycard.pdf

@linear-code

linear-code Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

WCN-1193

@s84krish s84krish force-pushed the wcn-1193-rename-to-safes branch from a6a9712 to 54bf80a Compare July 13, 2026 20:47
@s84krish

Copy link
Copy Markdown
Contributor Author

@claude

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @s84krish's task —— View job


Code Review for Wallet Vault → Safe Renaming

Todo List:

  • Read and analyze all modified files
  • Check consistency of renaming pattern
  • Review copy/formatting updates
  • Verify test file changes
  • Provide comprehensive feedback

Analysis Complete ✅

This PR successfully implements the renaming from "Wallet Vault" → "Safe" across the key-card module. Here's my comprehensive review:

Strengths

Consistent Renaming Pattern

  • Types: VaultRootKeyTypeSafeRootKeyType, VaultKeycardRootsSafeKeycardRoots
  • Constants: VAULT_ROOT_ORDERSAFE_ROOT_ORDER
  • Functions: generateVaultQrDatagenerateSafeQrData, parseVaultKeycardBoxparseSafeKeycardBox
  • Test file: Properly renamed from vaultQrData.tssafeQrData.ts

Smart Copy Updates

  • Box D title now correctly shows "Safe password" vs "Wallet password" when entityNoun='safe'
  • Description text updated to use the appropriate entity noun (safe vs wallet)
  • Variable names cleaned up (e.g., encryptedWalletPasscodeencryptedPasscode)

Improved UX in keycard layout 🎯

  • The multi-part QR note "you will need to put all Parts together for the full key" is now properly positioned underneath the QR codes instead of floating elsewhere
  • This makes scanning instructions clearer and more intuitive
  • Better handling of pagination when QR codes span multiple pages

🔍 Code Quality Observations

Good Architecture Decisions

  • Uses the new KeycardEntity type for type safety instead of string literals
  • Maintains backward compatibility for regular wallet keycards (defaults to 'wallet')
  • Error messages updated with correct terminology ("Safe is missing the X root")

Enhanced drawKeycard Function

  • drawOnePageOfQrCodes now returns both nextIndex AND endY for better layout control
  • Proper calculation of qrColumnBottom for multi-part keys
  • Improved rowHeight calculation that accounts for both text and QR column heights

Parser Improvements

  • isEncryptedWalletPasswordSectionTitle() now correctly handles both "Encrypted Wallet Password" AND "Encrypted Safe Password" titles
  • Better PDF parsing for both wallet and safe keycards

Test Coverage

  • Test file comprehensively updated with new function names and terminology
  • Comments updated to reflect "safe" instead of "vault" terminology
  • All test descriptions remain relevant and accurate
  • Test passphrase changed from 'vault-keycard-round-trip' to 'safe-keycard-round-trip'

🚀 No Issues Found

This is a well-executed renaming with thoughtful attention to:

  • Type safety (new KeycardEntity union type)
  • User experience (improved QR note positioning)
  • Backward compatibility (wallet keycards still work as before)
  • Test completeness (comprehensive test updates)
  • Documentation (JSDoc comments updated consistently)

The code is ready for merge! 🎉

@s84krish s84krish marked this pull request as ready for review July 13, 2026 21:05
@s84krish s84krish requested review from a team as code owners July 13, 2026 21:05
@s84krish s84krish merged commit d3c7359 into master Jul 13, 2026
24 checks passed
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