feat: passphrase support#292
Merged
Merged
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Storybook setup .storybook/main.ts, .storybook/preview.ts, package.json, tailwind.config.js, ui/globals.css |
Adds Storybook configuration, scripts, dependencies, Tailwind scanning, and global stylesheet directives. |
Shared page components ui/general/PageHeader.tsx, ui/full-page/*Page.tsx, ui/popup/add-wallet/AddWalletPage.tsx, related .stories.tsx, components/layouts/FullscreenLayout.tsx |
Adds reusable page components and stories, including header, welcome, password, import, and wallet selection pages. |
Screen wiring and navigation components/onboarding/Welcome.tsx, components/onboarding/SetupPassword.tsx, components/onboarding/ChooseImport.tsx, components/screens/AddWallet.tsx, lib/utils.ts |
Replaces inline layouts with shared pages and introduces full-page navigation from the popup. |
Fullscreen layout tweak components/layouts/FullscreenLayout.tsx |
Changes the layout wrapper height class from h-screen to min-h-screen. |
Passphrase import flow
| Layer / File(s) | Summary |
|---|---|
Passphrase derivation plumbing types/WalletSecret.ts, lib/wallet/account-factory.ts, lib/ethereum/wallet/account-factory.ts, hooks/wallet/useWalletImporter.ts, lib/service/handlers/kaspa/utils.ts, lib/service/handlers/evm/utils.ts |
Adds an optional passphrase field and passes it through mnemonic import, seed derivation, stored wallet secrets, and signer creation. |
Passphrase UI and onboarding flow ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx, ui/full-page/import-passphrase/ImportPassphrasePage.tsx, related .stories.tsx, components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx, components/screens/Onboarding.tsx, entrypoints/popup/router.tsx |
Adds the recovery phrase and passphrase screens, the coordinating onboarding screen, and the new popup route. |
| Estimated code review effort: 4 (Complex) | ~60 minutes |
Sequence Diagram(s)
sequenceDiagram
participant User
participant ImportRecoveryPhraseWithPassphrase
participant ImportRecoveryPhrasePage
participant ImportPassphrasePage
participant useWalletImporter
participant AccountFactory
participant Keyring
User->>ImportRecoveryPhrasePage: submit recovery words
ImportRecoveryPhraseWithPassphrase->>ImportRecoveryPhraseWithPassphrase: validate mnemonic and store it
ImportRecoveryPhraseWithPassphrase->>ImportPassphrasePage: render passphrase step
User->>ImportPassphrasePage: submit passphrase
ImportRecoveryPhraseWithPassphrase->>Keyring: initKeyring(password)
ImportRecoveryPhraseWithPassphrase->>useWalletImporter: importWalletByMnemonic(id, mnemonic, passphrase)
useWalletImporter->>AccountFactory: createFromMnemonic(mnemonic, index, passphrase)
useWalletImporter->>Keyring: addWalletSecret(..., passphrase)
Possibly related PRs
- forbole/kastle#236: Both PRs change the mnemonic account derivation call chain in
lib/ethereum/wallet/account-factory.tsand its callers.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly matches the main change: adding passphrase support across wallet import and creation flows. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feat/passphrase
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces UI building blocks and onboarding flows to support importing a wallet from a recovery phrase with an additional BIP-39 passphrase, while also adding Storybook setup for developing the new UI components.
Changes:
- Add a new “Recovery phrase with passphrase” import flow (phrase step → passphrase step) and wire it into onboarding + popup routing.
- Refactor onboarding/popup screens to use new reusable UI pages (Welcome, Create Password, Import Wallet, Add Wallet, Import Phrase/Passphrase).
- Add Storybook + Tailwind UI scanning to support UI component development.
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/popup/add-wallet/AddWalletPage.tsx | New popup “Add Wallet” UI page with standard + advanced options. |
| ui/popup/add-wallet/AddWalletPage.stories.tsx | Storybook stories for the popup add-wallet page. |
| ui/globals.css | Add Tailwind base/components/utilities for Storybook/UI pages. |
| ui/general/PageHeader.tsx | New shared header component for full-page/popup pages. |
| ui/general/PageHeader.stories.tsx | Storybook stories for PageHeader variants. |
| ui/full-page/welcome/WelcomePage.tsx | New full-page welcome UI. |
| ui/full-page/welcome/WelcomePage.stories.tsx | Storybook stories for WelcomePage. |
| ui/full-page/import-wallet/ImportWalletPage.tsx | New full-page “import method chooser” UI including advanced methods. |
| ui/full-page/import-wallet/ImportWalletPage.stories.tsx | Storybook stories for ImportWalletPage. |
| ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx | New recovery phrase entry UI (12/24 words, paste-all, hide/show, modal). |
| ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.stories.tsx | Storybook stories for ImportRecoveryPhrasePage. |
| ui/full-page/import-passphrase/ImportPassphrasePage.tsx | New passphrase entry UI (hide/show + modal). |
| ui/full-page/import-passphrase/ImportPassphrasePage.stories.tsx | Storybook stories for ImportPassphrasePage. |
| ui/full-page/create-password/CreatePasswordPage.tsx | New create-password UI (strength meter + terms checkbox). |
| ui/full-page/create-password/CreatePasswordPage.stories.tsx | Storybook stories for CreatePasswordPage. |
| types/WalletSecret.ts | Extend wallet secret shape to optionally include a passphrase. |
| tailwind.config.js | Include ui/**/*.{ts,tsx} in Tailwind content scanning. |
| package.json | Add Storybook scripts and Storybook dev dependencies. |
| package-lock.json | Lockfile updates for Storybook dependencies. |
| lib/wallet/account-factory.ts | Add optional passphrase support to mnemonic-to-seed derivation (Kaspa wallets). |
| lib/utils.ts | Add openFullPage() helper for opening fullscreen popup routes in a new tab. |
| lib/service/handlers/kaspa/utils.ts | Pass stored passphrase into mnemonic signer derivation (Kaspa). |
| lib/service/handlers/evm/utils.ts | Pass stored passphrase into mnemonic signer derivation (EVM). |
| lib/ethereum/wallet/account-factory.ts | Add optional passphrase support to mnemonic-to-seed derivation (EVM wallets). |
| hooks/wallet/useWalletImporter.ts | Store optional passphrase alongside mnemonic and use it for wallet derivation. |
| entrypoints/popup/router.tsx | Add route for the new “import recovery phrase with passphrase” screen. |
| components/screens/Onboarding.tsx | Add onboarding step for passphrase import flow. |
| components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx | New two-step import screen (phrase → passphrase) wired into onboarding. |
| components/screens/AddWallet.tsx | Replace inline popup UI with AddWalletPage + advanced passphrase option; open full-page routes via helper. |
| components/onboarding/Welcome.tsx | Refactor onboarding welcome to use WelcomePage UI component. |
| components/onboarding/SetupPassword.tsx | Refactor setup password screen to use CreatePasswordPage UI component. |
| components/onboarding/ChooseImport.tsx | Refactor import-method chooser to use ImportWalletPage UI component (incl. advanced passphrase method). |
| components/layouts/FullscreenLayout.tsx | Adjust layout to min-h-screen to better support tall content. |
| .storybook/preview.ts | Storybook preview setup (icons, globals.css, custom viewports). |
| .storybook/main.ts | Storybook Vite config + alias for @ imports. |
Comment on lines
+27
to
+32
| <button | ||
| className="flex size-[46px] items-center justify-center rounded-lg text-white hover:bg-daintree-800" | ||
| onClick={onBack} | ||
| > | ||
| <i className="hn hn-angle-left text-xl" /> | ||
| </button> |
Comment on lines
+47
to
+52
| <button | ||
| className="flex size-[46px] items-center justify-center rounded-lg text-white hover:bg-daintree-800" | ||
| onClick={onClose} | ||
| > | ||
| <i className="hn hn-times text-xl" /> | ||
| </button> |
Comment on lines
+57
to
+60
| { | ||
| label: "Recovery phrase", | ||
| description: "Use a 12- or 24-word recovery phrase, or a private key.", | ||
| }, |
Comment on lines
+16
to
+20
| isLoading: { control: "boolean" }, | ||
| error: { control: "text" }, | ||
| onBack: { action: "back" }, | ||
| onPassphraseInfo: { action: "passphraseInfo" }, | ||
| onSubmit: { action: "submit" }, |
| const [isHidden, setIsHidden] = useState(true); | ||
| const [showModal, setShowModal] = useState(false); | ||
|
|
||
| const disabled = !passphrase || isLoading; |
Comment on lines
+17
to
+23
| function PassphraseInfoModal({ onClose }: { onClose: () => void }) { | ||
| return ( | ||
| <div className="absolute inset-0 z-10 flex items-center justify-center bg-black/60 p-6"> | ||
| <div className="relative w-full max-w-[390px] rounded-2xl bg-icy-blue-950 p-10"> | ||
| <button | ||
| className="absolute right-2.5 top-2.5 flex size-[38px] items-center justify-center rounded-lg text-daintree-400 hover:bg-daintree-800" | ||
| onClick={onClose} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
ui/globals.css (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint misconfigured for Tailwind directives.
Stylelint is parsing this file with SCSS syntax (
scss/at-rule-no-unknown), flagging standard@tailwinddirectives as errors. This is a config gap, not a code issue — add@tailwind(and@apply,@layer, etc.) to the rule'signoreAtRulesor configurecustomSyntax: "postcss"/postcss-scssexceptions in.stylelintrcso Tailwind directives aren't misreported as errors in CI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/globals.css` around lines 1 - 3, Stylelint is treating Tailwind directives in the globals stylesheet as invalid SCSS at-rules, so update the Stylelint configuration rather than the CSS itself. In the Stylelint config that governs ui/globals.css, either add Tailwind directives like `@tailwind`, `@apply`, and `@layer` to the scss/at-rule-no-unknown ignoreAtRules list or switch the stylesheet parsing setup to the appropriate PostCSS syntax so these directives are recognized correctly. Focus the change on the Stylelint rule/configuration used for globals.css.Source: Linters/SAST tools
lib/utils.ts (1)
99-103: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnhandled rejection on
browser.tabs.create.
browser.tabs.createreturns a promise that can reject; it's not awaited or caught here, so failures fail silently (or surface as an unhandled rejection).🔧 Proposed fix
-export function openFullPage(path: `/${string}`) { +export async function openFullPage(path: `/${string}`) { const url = new URL(browser.runtime.getURL("/popup.html")); url.hash = path; - browser.tabs.create({ url: url.toString() }); + await browser.tabs.create({ url: url.toString() }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/utils.ts` around lines 99 - 103, The openFullPage helper ignores the promise returned by browser.tabs.create, which can lead to unhandled rejections. Update openFullPage to await the tabs creation or explicitly handle the returned promise with try/catch or a catch handler, and make sure any failure from browser.tabs.create is surfaced or logged appropriately while keeping the URL construction logic in place.ui/general/PageHeader.tsx (1)
26-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIcon-only buttons lack accessible names.
Back/close buttons render only an
<i>icon with noaria-label, so screen readers announce nothing meaningful.♿ Proposed fix
<button className="flex size-[46px] items-center justify-center rounded-lg text-white hover:bg-daintree-800" onClick={onBack} + aria-label="Back" >Also applies to: 46-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/general/PageHeader.tsx` around lines 26 - 33, The icon-only back/close buttons in PageHeader have no accessible name, so add an explicit accessible label to the button elements rendered by the showBack and close-button branches. Update the relevant button markup in PageHeader so the button exposed by onBack and the close action includes a meaningful aria-label (or equivalent accessible text) while keeping the visual icon unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/onboarding/ChooseImport.tsx`:
- Around line 47-53: Guard the `onCreateWallet` flow in `ChooseImport.tsx`
against repeated clicks and failures: add a loading/disabled guard around the
`keyringInitialize` → `createNewWallet` → `emitWalletCreated` → `navigate`
sequence in the `onCreateWallet` handler, and wrap it in `try/catch/finally` so
rejected promises surface an error via `internalToast` and the loading state
resets. Use the same pattern already present in
`ImportRecoveryPhraseWithPassphrase.tsx`, and consider extracting this shared
wallet-creation logic for reuse with `SetupPassword.tsx` so both call sites
benefit from the same protection.
In `@components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx`:
- Around line 27-43: Move the mnemonic validation out of handlePassphraseSubmit
and into handlePhraseSubmit so invalid recovery phrases are caught immediately
after phrase entry. In ImportRecoveryPhraseWithPassphrase, validate with new
Mnemonic(words.join(" ")) before calling setStep("passphrase"), and only advance
to the passphrase step when the phrase is valid; keep the existing
invalid-phrase error handling tied to the phrase submission flow rather than the
passphrase submission flow.
- Around line 32-68: The retry path in handlePassphraseSubmit is re-running
keyringInitialize() even when the keyring is already set up, which causes
subsequent importWalletByMnemonic attempts to fail immediately. Update the
ImportRecoveryPhraseWithPassphrase flow to guard the
onboardingForm/keyringInitialize step so it only initializes once, or skip it
when the keyring is already initialized, then continue the import and navigation
logic as before.
In `@ui/full-page/create-password/CreatePasswordPage.tsx`:
- Around line 18-32: CreatePasswordPage’s submit flow currently calls onSubmit
without awaiting it, disabling, or handling rejections, which can allow
duplicate submissions and silent failures. Update the submit logic in
CreatePasswordPage to track an in-flight/loading state around onSubmit, disable
the submit button while the async handler is pending, and catch errors so they
can be surfaced through the existing error prop or local error state. Use the
CreatePasswordPage component and its submit button handler as the main
locations, and ensure callers like SetupPassword.tsx still work with an async
onSubmit.
In `@ui/full-page/import-passphrase/ImportPassphrasePage.tsx`:
- Around line 17-73: The PassphraseInfoModal component is duplicated in multiple
import pages and should be shared to prevent drift. Extract the existing
PassphraseInfoModal from ImportPassphrasePage into a reusable component under a
common location such as ui/general, then import and use it from both
ImportPassphrasePage and ImportRecoveryPhrasePage. Keep the public API the same
by preserving the onClose prop and the current modal behavior.
In `@ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx`:
- Around line 163-166: The visibility toggle icon mapping in
ImportRecoveryPhrasePage is inconsistent with ImportPassphrasePage, where
isHidden should correspond to the hidden-state icon and the opposite state to
the visible icon. Update the icon class logic in the toggle JSX so the symbol
selected for isHidden matches the convention used by the equivalent toggle in
ImportPassphrasePage, and keep the label text in sync with that state.
- Around line 102-111: The empty catch in handlePasteAll is tripping lint and
hides paste failures. Update the clipboard read/parse flow in
ImportRecoveryPhrasePage so the catch block is not empty: either log or surface
the error, and add user feedback when the pasted text does not contain 12 or 24
words. Keep the validation logic in handlePasteAll and the phrase state updates
(setPhraseLength, setWords) unchanged except for handling the invalid/failed
paste case.
---
Nitpick comments:
In `@lib/utils.ts`:
- Around line 99-103: The openFullPage helper ignores the promise returned by
browser.tabs.create, which can lead to unhandled rejections. Update openFullPage
to await the tabs creation or explicitly handle the returned promise with
try/catch or a catch handler, and make sure any failure from browser.tabs.create
is surfaced or logged appropriately while keeping the URL construction logic in
place.
In `@ui/general/PageHeader.tsx`:
- Around line 26-33: The icon-only back/close buttons in PageHeader have no
accessible name, so add an explicit accessible label to the button elements
rendered by the showBack and close-button branches. Update the relevant button
markup in PageHeader so the button exposed by onBack and the close action
includes a meaningful aria-label (or equivalent accessible text) while keeping
the visual icon unchanged.
In `@ui/globals.css`:
- Around line 1-3: Stylelint is treating Tailwind directives in the globals
stylesheet as invalid SCSS at-rules, so update the Stylelint configuration
rather than the CSS itself. In the Stylelint config that governs ui/globals.css,
either add Tailwind directives like `@tailwind`, `@apply`, and `@layer` to the
scss/at-rule-no-unknown ignoreAtRules list or switch the stylesheet parsing
setup to the appropriate PostCSS syntax so these directives are recognized
correctly. Focus the change on the Stylelint rule/configuration used for
globals.css.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5b90b695-3138-4079-9a6e-290e1897f241
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
.storybook/main.ts.storybook/preview.tscomponents/layouts/FullscreenLayout.tsxcomponents/onboarding/ChooseImport.tsxcomponents/onboarding/SetupPassword.tsxcomponents/onboarding/Welcome.tsxcomponents/screens/AddWallet.tsxcomponents/screens/Onboarding.tsxcomponents/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsxentrypoints/popup/router.tsxhooks/wallet/useWalletImporter.tslib/ethereum/wallet/account-factory.tslib/service/handlers/evm/utils.tslib/service/handlers/kaspa/utils.tslib/utils.tslib/wallet/account-factory.tspackage.jsontailwind.config.jstypes/WalletSecret.tsui/full-page/create-password/CreatePasswordPage.stories.tsxui/full-page/create-password/CreatePasswordPage.tsxui/full-page/import-passphrase/ImportPassphrasePage.stories.tsxui/full-page/import-passphrase/ImportPassphrasePage.tsxui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.stories.tsxui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsxui/full-page/import-wallet/ImportWalletPage.stories.tsxui/full-page/import-wallet/ImportWalletPage.tsxui/full-page/welcome/WelcomePage.stories.tsxui/full-page/welcome/WelcomePage.tsxui/general/PageHeader.stories.tsxui/general/PageHeader.tsxui/globals.cssui/popup/add-wallet/AddWalletPage.stories.tsxui/popup/add-wallet/AddWalletPage.tsx
Comment on lines
+18
to
+32
| export interface CreatePasswordPageProps { | ||
| title?: string; | ||
| subtitle?: string; | ||
| passwordLabel?: string; | ||
| confirmPasswordLabel?: string; | ||
| passwordPlaceholder?: string; | ||
| confirmPasswordPlaceholder?: string; | ||
| hint?: string; | ||
| termsUrl?: string; | ||
| privacyUrl?: string; | ||
| buttonLabel?: string; | ||
| error?: string; | ||
| onBack?: () => void; | ||
| onSubmit?: (password: string) => void; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard async onSubmit with a loading state and error handling.
The submit button doesn't await or catch onSubmit, and isn't disabled while the callback is in flight. Consumers like SetupPassword.tsx pass an async handler that calls keyringInitialize/createNewWallet; a double-click can fire duplicate wallet-creation calls, and a rejected promise will fail silently with no user feedback.
🛠️ Proposed fix
export interface CreatePasswordPageProps {
title?: string;
subtitle?: string;
passwordLabel?: string;
confirmPasswordLabel?: string;
passwordPlaceholder?: string;
confirmPasswordPlaceholder?: string;
hint?: string;
termsUrl?: string;
privacyUrl?: string;
buttonLabel?: string;
error?: string;
onBack?: () => void;
- onSubmit?: (password: string) => void;
+ onSubmit?: (password: string) => void | Promise<void>;
}
export default function CreatePasswordPage({
...
}: CreatePasswordPageProps) {
const [password, setPassword] = useState("");
const [confirm, setConfirm] = useState("");
const [showPassword, setShowPassword] = useState(false);
const [showConfirm, setShowConfirm] = useState(false);
const [termsChecked, setTermsChecked] = useState(false);
+ const [submitting, setSubmitting] = useState(false);
const strength = calcStrength(password);
const mismatch = confirm.length > 0 && password !== confirm;
- const disabled = !password || password !== confirm || !termsChecked;
+ const disabled =
+ !password || password !== confirm || !termsChecked || submitting; <button
className={`...`}
disabled={disabled}
- onClick={() => !disabled && onSubmit?.(password)}
+ onClick={async () => {
+ if (disabled) return;
+ try {
+ setSubmitting(true);
+ await onSubmit?.(password);
+ } finally {
+ setSubmitting(false);
+ }
+ }}
type="button"
>Also applies to: 49-53, 190-201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/full-page/create-password/CreatePasswordPage.tsx` around lines 18 - 32,
CreatePasswordPage’s submit flow currently calls onSubmit without awaiting it,
disabling, or handling rejections, which can allow duplicate submissions and
silent failures. Update the submit logic in CreatePasswordPage to track an
in-flight/loading state around onSubmit, disable the submit button while the
async handler is pending, and catch errors so they can be surfaced through the
existing error prop or local error state. Use the CreatePasswordPage component
and its submit button handler as the main locations, and ensure callers like
SetupPassword.tsx still work with an async onSubmit.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx (1)
68-77: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPaste-all still fails silently on invalid input.
The empty-catch lint issue is fixed, but per the earlier review, pasting text that isn't 12/24 words (or a denied clipboard-read) still leaves the user with no feedback — nothing happens and no error is surfaced.
💡 Proposed fix
const handlePasteAll = async () => { const text = await navigator.clipboard.readText().catch(() => null); - if (!text) return; + if (!text) { + onErrorClear?.(); + return; + } const parsed = text.trim().split(/\s+/); if (parsed.length === 12 || parsed.length === 24) { onErrorClear?.(); setPhraseLength(parsed.length as PhraseLength); setWords([...parsed, ...Array(24 - parsed.length).fill("")]); + } else { + // surface feedback, e.g. via a local toast or passed-in error setter } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx` around lines 68 - 77, The handlePasteAll flow in ImportRecoveryPhrasePage still ignores invalid pasted content and clipboard-read failures without telling the user. Update the clipboard read and parsing path so that when navigator.clipboard.readText fails or the pasted text is not 12/24 words, the user gets a visible error via the existing onError/onErrorClear handling instead of silently returning. Keep the success path in handlePasteAll unchanged for valid phrases, but add a clear error message for the invalid/denied cases.
🧹 Nitpick comments (1)
ui/general/PassphraseInfoModal.tsx (1)
6-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd dialog semantics and keyboard support.
The overlay lacks
role="dialog"/aria-modal, has no Escape-key handler, and the icon-only close button (Line 9-15) has noaria-label. Screen-reader and keyboard-only users have no accessible way to identify or dismiss the modal other than tabbing to the visible buttons.♿ Proposed fix
- <div className="absolute inset-0 z-10 flex items-center justify-center bg-black/60 p-6"> - <div className="relative w-full max-w-[390px] rounded-2xl bg-icy-blue-950 p-10"> + <div + className="absolute inset-0 z-10 flex items-center justify-center bg-black/60 p-6" + onKeyDown={(e) => e.key === "Escape" && onClose()} + > + <div + role="dialog" + aria-modal="true" + aria-label="What is a passphrase?" + className="relative w-full max-w-[390px] rounded-2xl bg-icy-blue-950 p-10" + > <button className="absolute right-2.5 top-2.5 flex size-[38px] items-center justify-center rounded-lg text-daintree-400 hover:bg-daintree-800" onClick={onClose} + aria-label="Close" type="button" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/general/PassphraseInfoModal.tsx` around lines 6 - 16, The PassphraseInfoModal overlay needs proper dialog accessibility and keyboard dismissal. Update the modal container in PassphraseInfoModal to use dialog semantics such as role="dialog" and aria-modal, add an Escape-key handler to close it, and give the icon-only close button an explicit aria-label. Keep using the existing onClose handler so both keyboard and screen-reader users can reliably dismiss the modal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx`:
- Line 22: The keyring initialization guard in
ImportRecoveryPhraseWithPassphrase is only component-local, so remounting the
screen resets it and can call keyringInitialize() again after Keyring is already
initialized. Update the initialization flow in this screen to check
getKeyringStatus().isInitialized before invoking keyringInitialize(), or handle
the “Keyring is already initialized” error as a harmless no-op, and keep the
logic tied to the existing keyringReady/ref-based flow.
---
Duplicate comments:
In `@ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsx`:
- Around line 68-77: The handlePasteAll flow in ImportRecoveryPhrasePage still
ignores invalid pasted content and clipboard-read failures without telling the
user. Update the clipboard read and parsing path so that when
navigator.clipboard.readText fails or the pasted text is not 12/24 words, the
user gets a visible error via the existing onError/onErrorClear handling instead
of silently returning. Keep the success path in handlePasteAll unchanged for
valid phrases, but add a clear error message for the invalid/denied cases.
---
Nitpick comments:
In `@ui/general/PassphraseInfoModal.tsx`:
- Around line 6-16: The PassphraseInfoModal overlay needs proper dialog
accessibility and keyboard dismissal. Update the modal container in
PassphraseInfoModal to use dialog semantics such as role="dialog" and
aria-modal, add an Escape-key handler to close it, and give the icon-only close
button an explicit aria-label. Keep using the existing onClose handler so both
keyboard and screen-reader users can reliably dismiss the modal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: aa9b5b84-3e1d-43e8-9fc6-4a0f952066f8
📒 Files selected for processing (8)
components/onboarding/ChooseImport.tsxcomponents/onboarding/SetupPassword.tsxcomponents/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsxui/full-page/create-password/CreatePasswordPage.tsxui/full-page/import-passphrase/ImportPassphrasePage.tsxui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.stories.tsxui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.tsxui/general/PassphraseInfoModal.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- ui/full-page/import-recovery-phrase/ImportRecoveryPhrasePage.stories.tsx
- ui/full-page/import-passphrase/ImportPassphrasePage.tsx
- components/onboarding/ChooseImport.tsx
- components/onboarding/SetupPassword.tsx
- ui/full-page/create-password/CreatePasswordPage.tsx
| const { importWalletByMnemonic } = useWalletImporter(); | ||
| const onboardingForm = useFormContext<OnboardingData>(); | ||
|
|
||
| const keyringReady = useRef(false); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx --view expanded || true
printf '\n== relevant file excerpt ==\n'
cat -n components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx | sed -n '1,220p'
printf '\n== keyring references ==\n'
rg -n "keyringInitialize|useKeyring|initialize\(|already initialized|setUp|isInitialized|keyringReady" components lib hooks -S || true
printf '\n== onboarding step flow references ==\n'
rg -n 'setValue\("step"|step"\s*:\s*"choose"|step\s*===\s*"choose"|onboardingForm' components -S || trueRepository: forbole/kastle
Length of output: 15166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== onboarding screen ==\n'
cat -n components/screens/Onboarding.tsx | sed -n '1,220p'
printf '\n== choose import screen ==\n'
cat -n components/onboarding/ChooseImport.tsx | sed -n '1,220p'
printf '\n== useKeyring hook ==\n'
cat -n hooks/useKeyring.ts | sed -n '1,220p'
printf '\n== keyring initialize handler ==\n'
cat -n lib/service/handlers/keyring-initialize.ts | sed -n '1,120p'
printf '\n== keyring manager initialize ==\n'
cat -n lib/keyring-manager.ts | sed -n '1,120p'Repository: forbole/kastle
Length of output: 14331
Avoid re-initializing the keyring after this screen remounts. Around lines 47-49, keyringReady is component-local, so backing out to step="choose" unmounts this screen and resets the ref. Re-entering the flow will call keyringInitialize() again even if the keyring already exists, and Keyring.initialize() throws "Keyring is already initialized". Check getKeyringStatus().isInitialized before initializing, or treat that error as a no-op.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/screens/full-pages/ImportRecoveryPhraseWithPassphrase.tsx` at line
22, The keyring initialization guard in ImportRecoveryPhraseWithPassphrase is
only component-local, so remounting the screen resets it and can call
keyringInitialize() again after Keyring is already initialized. Update the
initialization flow in this screen to check getKeyringStatus().isInitialized
before invoking keyringInitialize(), or handle the “Keyring is already
initialized” error as a harmless no-op, and keep the logic tied to the existing
keyringReady/ref-based flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Before Submission
Description
Additional Notes
By submitting this pull request, I confirm that:
project's
license terms
Summary by CodeRabbit