From 6a253fd43b1f9c52e921ef528c04607f4ad1e547 Mon Sep 17 00:00:00 2001 From: thephez Date: Wed, 13 May 2026 16:49:47 -0400 Subject: [PATCH 1/3] refactor(dashnote): drop login-modal duplicates of settings controls The contract-ID editor and "Forget this device" button now live only in the Settings panel; the LoginModal's Advanced disclosure also hides for WIF input since identity-index is the only remaining knob. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../dashnote/src/components/LoginModal.tsx | 105 ++++++---------- .../dashnote/test/LoginModal.test.tsx | 115 +++++++++++++++--- example-apps/dashnote/test/e2e/auth.spec.ts | 37 ------ example-apps/dashnote/test/e2e/smoke.spec.ts | 21 +++- 4 files changed, 147 insertions(+), 131 deletions(-) diff --git a/example-apps/dashnote/src/components/LoginModal.tsx b/example-apps/dashnote/src/components/LoginModal.tsx index dd9bb7f..2b1012f 100644 --- a/example-apps/dashnote/src/components/LoginModal.tsx +++ b/example-apps/dashnote/src/components/LoginModal.tsx @@ -16,7 +16,6 @@ export function LoginModal({ open, onClose }: LoginModalProps) { const session = useSession(); const [secret, setSecret] = useState(""); const [identityIndex, setIdentityIndex] = useState("0"); - const [contractInput, setContractInput] = useState(session.contractId ?? ""); const [error, setError] = useState(null); const [submitting, setSubmitting] = useState(false); const [showAdvanced, setShowAdvanced] = useState(false); @@ -37,10 +36,6 @@ export function LoginModal({ open, onClose }: LoginModalProps) { wifPreview.status === "wrong-purpose" || wifPreview.status === "key-disabled"; - useEffect(() => { - setContractInput(session.contractId ?? ""); - }, [session.contractId]); - useEffect(() => { if (open) { setRememberMe(true); @@ -50,10 +45,6 @@ export function LoginModal({ open, onClose }: LoginModalProps) { } }, [open]); - function applyContractId() { - session.setContractId(contractInput.trim() || null); - } - async function handleLogin(event: FormEvent) { event.preventDefault(); setError(null); @@ -187,26 +178,17 @@ export function LoginModal({ open, onClose }: LoginModalProps) { )} - {session.rememberedIdentityId && ( + {session.rememberedIdentityId && !useDifferentIdentity && (
- {!useDifferentIdentity && ( - - )}
)} @@ -221,57 +203,38 @@ export function LoginModal({ open, onClose }: LoginModalProps) { Remember this identity on this device - + {!isWifInput && ( + <> + - {showAdvanced && ( -
- {!isWifInput && ( - + {showAdvanced && ( +
+ +
)} - -
- - Contract ID (optional) - - setContractInput(event.target.value)} - placeholder="Paste a Dashnote note contract ID to reuse" - className="rounded-md border border-line bg-bg px-3 py-2 font-mono text-[12px] text-ink outline-none transition focus:border-accent-dim" - /> - -
-
+ )} {error && ( diff --git a/example-apps/dashnote/test/LoginModal.test.tsx b/example-apps/dashnote/test/LoginModal.test.tsx index 87d3a3a..841ff9f 100644 --- a/example-apps/dashnote/test/LoginModal.test.tsx +++ b/example-apps/dashnote/test/LoginModal.test.tsx @@ -150,6 +150,98 @@ describe("LoginModal", () => { expect(button.disabled).toBe(true); }); + it("keeps the login button disabled for a whitespace-only secret", () => { + mockUseSession.mockReturnValue(makeSession()); + + render(); + + fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { + target: { value: " \t " }, + }); + + const button = screen.getByRole("button", { + name: /^login$/i, + }) as HTMLButtonElement; + expect(button.disabled).toBe(true); + }); + + it("disables the login button while a login is in flight", async () => { + // Resolve manually so we can observe the in-flight state. The label + // also flips to "Connecting…" while submitting is true. + let resolveLogin: (() => void) | undefined; + const login = vi.fn( + () => + new Promise((resolve) => { + resolveLogin = () => resolve(); + }), + ); + mockUseSession.mockReturnValue(makeSession({ login })); + + render(); + + fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { + target: { value: "phrase" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + + const connectingButton = (await screen.findByRole("button", { + name: /connecting/i, + })) as HTMLButtonElement; + expect(connectingButton.disabled).toBe(true); + + resolveLogin?.(); + await waitFor(() => { + expect(login).toHaveBeenCalled(); + }); + }); + + it("falls back to identityIndex=0 when the index field is non-numeric", async () => { + // Number.parseInt("abc", 10) is NaN; the handler must coerce that to 0 + // rather than passing NaN through to session.login. + const login = vi.fn().mockResolvedValue(undefined); + mockUseSession.mockReturnValue(makeSession({ login })); + + render(); + + fireEvent.click(screen.getByText(/advanced settings/i)); + fireEvent.change(screen.getByRole("spinbutton"), { + target: { value: "abc" }, + }); + fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { + target: { value: "phrase" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + + await waitFor(() => { + expect(login).toHaveBeenCalledWith("phrase", { + identityIndex: 0, + rememberMe: true, + }); + }); + }); + + it("preserves identityIndex and showAdvanced across modal reopens", () => { + // The open-effect resets secret/rememberMe/useDifferentIdentity/error, + // but NOT identityIndex or showAdvanced — verify that contract so a + // future refactor doesn't quietly change it. + mockUseSession.mockReturnValue(makeSession()); + + const { rerender } = render(); + + fireEvent.click(screen.getByText(/advanced settings/i)); + fireEvent.change(screen.getByRole("spinbutton"), { + target: { value: "7" }, + }); + + rerender(); + rerender(); + + // Disclosure is still open, value still 7. + expect((screen.getByRole("spinbutton") as HTMLInputElement).value).toBe( + "7", + ); + }); + it("uses identity index from advanced settings", async () => { const login = vi.fn().mockResolvedValue(undefined); mockUseSession.mockReturnValue(makeSession({ login })); @@ -341,23 +433,6 @@ describe("LoginModal", () => { expect(checkbox.checked).toBe(true); }); - it("calls forgetIdentity when Forget this device is clicked from the login form", () => { - const forgetIdentity = vi.fn(); - mockUseSession.mockReturnValue( - makeSession({ - rememberedIdentityId: "remembered-identity-id", - forgetIdentity, - }), - ); - - render(); - - fireEvent.click( - screen.getByRole("button", { name: /forget this device/i }), - ); - expect(forgetIdentity).toHaveBeenCalled(); - }); - it("hides the remembered identity panel when no identity is remembered", () => { mockUseSession.mockReturnValue(makeSession()); @@ -483,15 +558,17 @@ describe("LoginModal", () => { expect(screen.queryByRole("spinbutton")).not.toBeNull(); }); - it("hides the identity-index field when the input parses as a WIF", () => { + it("hides the Advanced settings disclosure when the input parses as a WIF", () => { + // WIF input has no DIP-13 derivation, so the only knob inside Advanced + // (Identity index) is irrelevant — the disclosure itself disappears. mockUseSession.mockReturnValue(makeSession()); render(); fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "cVHcfvcWNc7DvqaPCwM6Z3DqZ" }, }); - fireEvent.click(screen.getByText(/advanced settings/i)); + expect(screen.queryByText(/advanced settings/i)).toBeNull(); expect(screen.queryByRole("spinbutton")).toBeNull(); }); diff --git a/example-apps/dashnote/test/e2e/auth.spec.ts b/example-apps/dashnote/test/e2e/auth.spec.ts index a510f14..f5ba186 100644 --- a/example-apps/dashnote/test/e2e/auth.spec.ts +++ b/example-apps/dashnote/test/e2e/auth.spec.ts @@ -80,43 +80,6 @@ test("forget-this-device via the Settings panel drops back to readonly", async ( ).toBeHidden(); }); -test("forget-this-device via the LoginModal also clears the remembered identity", async ({ - page, -}) => { - await loginViaModal(page, { rememberMe: true }); - - // Log out so the modal can be opened with the rememberedIdentity panel - // visible. Logout-without-forget keeps the hint, dropping to browsing. - await openIdentityMenu(page); - await page.getByRole("menuitem", { name: /^log out$/i }).click(); - await expect( - page.getByText("Browsing (read-only)", { exact: true }), - ).toBeVisible({ timeout: 30_000 }); - - // Open the login modal — the "Forget this device" button is rendered - // beside "Use a different identity" when rememberedIdentityId is set. - // The IdentityCard in browsing state is a button with aria-haspopup="menu" - // (the "Switch identity" menuitem opens the login modal). - await openIdentityMenu(page); - await page.getByRole("menuitem", { name: /switch identity/i }).click(); - - const dialog = page.getByRole("dialog"); - await expect(dialog).toBeVisible(); - await dialog.getByRole("button", { name: /forget this device/i }).click(); - - // The form re-renders without the remembered panel. - await expect( - dialog.locator('[data-testid="remembered-identity-panel"]'), - ).toBeHidden(); - - // Close and reload — readonly state confirms the hint is gone. - await dialog.getByRole("button", { name: /^cancel$/i }).click(); - await page.reload(); - await expect( - page.getByText("Browsing (read-only)", { exact: true }), - ).toBeHidden(); -}); - test("Switch identity from the IdentityCard menu re-opens the login form", async ({ page, }) => { diff --git a/example-apps/dashnote/test/e2e/smoke.spec.ts b/example-apps/dashnote/test/e2e/smoke.spec.ts index 3add610..81ffebc 100644 --- a/example-apps/dashnote/test/e2e/smoke.spec.ts +++ b/example-apps/dashnote/test/e2e/smoke.spec.ts @@ -108,14 +108,27 @@ test.describe("login modal", () => { await expect(dialog.locator('input[type="number"]')).toBeVisible(); }); - test("Advanced settings exposes a contract-ID field", async ({ page }) => { + test("Advanced settings disclosure is hidden when input parses as a WIF", async ({ + page, + }) => { + // WIF input has no DIP-13 derivation, so identity-index is irrelevant + // and the whole Advanced disclosure should disappear. await (await navButton(page, /login$/i)).click(); const dialog = page.getByRole("dialog"); - await dialog.getByRole("button", { name: /advanced settings/i }).click(); - await expect(dialog.getByText(/contract id/i).first()).toBeVisible(); + + // Mnemonic-shaped input first → disclosure renders. + await dialog.getByPlaceholder(/mnemonic phrase|wif/i).fill("word word"); await expect( - dialog.getByPlaceholder(/dashnote note contract id/i), + dialog.getByRole("button", { name: /advanced settings/i }), ).toBeVisible(); + + // Switch to WIF-shaped input → disclosure disappears. + await dialog + .getByPlaceholder(/mnemonic phrase|wif/i) + .fill("cVHcfvcWNc7DvqaPCwM6Z3DqZ"); + await expect( + dialog.getByRole("button", { name: /advanced settings/i }), + ).toBeHidden(); }); }); From 83afec7e0abdc852831d05948d81e9830a82a815 Mon Sep 17 00:00:00 2001 From: thephez Date: Wed, 13 May 2026 17:23:58 -0400 Subject: [PATCH 2/3] refactor(dashnote): unify auth CTAs under "Sign in" wording Renames Login/Log in to Sign in across the IdentityCard, sidebar NavButton, LoginModal, and NotesWorkspace empty state. Hides the sidebar Sign in entry in authenticated and browsing modes (the IdentityCard menu handles those). Adds a primary Sign in button to the SettingsPanel unauth empty state matching the NotesWorkspace treatment. The e2e loginViaModal helper now routes through the IdentityCard menu when the session boots into browsing (rememberMe reload) since the sidebar entry is absent there. Co-Authored-By: Claude Opus 4.7 (1M context) --- example-apps/dashnote/public/favicon.svg | 3 ++ example-apps/dashnote/src/App.tsx | 4 ++- .../dashnote/src/components/AppShell.tsx | 4 +-- .../dashnote/src/components/IdentityCard.tsx | 8 ++--- .../dashnote/src/components/LoginModal.tsx | 4 +-- .../src/components/NotesWorkspace.tsx | 4 +-- .../dashnote/src/components/SettingsPanel.tsx | 19 ++++++++-- .../dashnote/test/IdentityCard.test.tsx | 27 +++++++++++--- .../dashnote/test/LoginModal.test.tsx | 30 ++++++++-------- .../dashnote/test/NotesWorkspace.test.tsx | 2 +- .../dashnote/test/SettingsPanel.test.tsx | 29 ++++++++------- example-apps/dashnote/test/e2e/auth.spec.ts | 4 ++- example-apps/dashnote/test/e2e/fixtures.ts | 35 ++++++++++++++----- example-apps/dashnote/test/e2e/smoke.spec.ts | 18 +++++----- 14 files changed, 125 insertions(+), 66 deletions(-) create mode 100644 example-apps/dashnote/public/favicon.svg diff --git a/example-apps/dashnote/public/favicon.svg b/example-apps/dashnote/public/favicon.svg new file mode 100644 index 0000000..00f248d --- /dev/null +++ b/example-apps/dashnote/public/favicon.svg @@ -0,0 +1,3 @@ + + + diff --git a/example-apps/dashnote/src/App.tsx b/example-apps/dashnote/src/App.tsx index 2f167e2..14726b6 100644 --- a/example-apps/dashnote/src/App.tsx +++ b/example-apps/dashnote/src/App.tsx @@ -98,7 +98,9 @@ function App() { /> )} {tab === "how-it-works" && } - {tab === "settings" && } + {tab === "settings" && ( + setLoginOpen(true)} /> + )} diff --git a/example-apps/dashnote/src/components/AppShell.tsx b/example-apps/dashnote/src/components/AppShell.tsx index e1eee78..618e454 100644 --- a/example-apps/dashnote/src/components/AppShell.tsx +++ b/example-apps/dashnote/src/components/AppShell.tsx @@ -116,9 +116,9 @@ export function AppShell({ closeDrawer(); }} /> - {status !== "authenticated" && ( + {status !== "authenticated" && status !== "browsing" && ( { diff --git a/example-apps/dashnote/src/components/IdentityCard.tsx b/example-apps/dashnote/src/components/IdentityCard.tsx index eee938d..8833c9e 100644 --- a/example-apps/dashnote/src/components/IdentityCard.tsx +++ b/example-apps/dashnote/src/components/IdentityCard.tsx @@ -69,7 +69,7 @@ export function IdentityCard({ onClick={onLoginClick} className="w-full rounded-md bg-accent px-3 py-2 text-[12px] font-semibold text-bg transition hover:bg-accent-dim" > - Login + Sign in
- Switch identity + {isAuthed ? "Switch identity" : "Sign in"} {isAuthed && (
); } diff --git a/example-apps/dashnote/test/IdentityCard.test.tsx b/example-apps/dashnote/test/IdentityCard.test.tsx index e9d66eb..d658464 100644 --- a/example-apps/dashnote/test/IdentityCard.test.tsx +++ b/example-apps/dashnote/test/IdentityCard.test.tsx @@ -91,13 +91,13 @@ describe("IdentityCard", () => { expect(screen.getByText(TRUNCATED_ID)).toBeTruthy(); }); - it("hides the identity layout and shows a Login button when not connected", () => { + it("hides the identity layout and shows a Sign in button when not connected", () => { renderCard({ status: "idle", identityId: IDENTITY_ID, dpnsName: "alice", }); - expect(screen.getByRole("button", { name: /login/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /^sign in$/i })).toBeTruthy(); // The identity layout (name + truncated id) must not render in the // disconnected state, even when an id and name are passed in. expect(screen.queryByText("@alice")).toBeNull(); @@ -106,8 +106,8 @@ describe("IdentityCard", () => { // Regression: when the card was unified into a single menu trigger, readonly // (connected-but-not-signed-in) silently lost its one-click path to the - // login modal — the menu offered Settings and Switch identity but no direct - // Login. The card must call onLoginClick on click and render no menu. + // login modal — the menu offered Settings and Sign in but no direct Sign in + // affordance. The card must call onLoginClick on click and render no menu. it("calls onLoginClick on click when readonly, without opening a menu", () => { const onLoginClick = vi.fn(); renderCard({ @@ -159,6 +159,25 @@ describe("IdentityCard", () => { expect(onLoginClick).toHaveBeenCalled(); }); + // In browsing mode the user has no signed-in key to switch *from*, so the + // menu entry is labeled Sign in (not Switch identity). It still calls + // onLoginClick — only the affordance changes. + it("labels the login entry 'Sign in' in browsing mode and calls onLoginClick", () => { + const onLoginClick = vi.fn(); + renderCard({ + status: "browsing", + identityId: IDENTITY_ID, + dpnsName: null, + onLoginClick, + }); + fireEvent.click(screen.getByRole("button", { expanded: false })); + expect( + screen.queryByRole("menuitem", { name: /switch identity/i }), + ).toBeNull(); + fireEvent.click(screen.getByRole("menuitem", { name: /^sign in$/i })); + expect(onLoginClick).toHaveBeenCalled(); + }); + it("calls session.logout when Log out is chosen from the menu", () => { const logout = vi.fn(); mockUseSession.mockReturnValue({ logout }); diff --git a/example-apps/dashnote/test/LoginModal.test.tsx b/example-apps/dashnote/test/LoginModal.test.tsx index 841ff9f..6faf75c 100644 --- a/example-apps/dashnote/test/LoginModal.test.tsx +++ b/example-apps/dashnote/test/LoginModal.test.tsx @@ -99,7 +99,7 @@ describe("LoginModal", () => { expect(onClose).not.toHaveBeenCalled(); expect(screen.getByPlaceholderText(/mnemonic phrase/i)).toBeTruthy(); - expect(screen.getByRole("button", { name: /^login$/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /^sign in$/i })).toBeTruthy(); }); it("submits the mnemonic via session.login and closes on success", async () => { @@ -112,7 +112,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "test mnemonic phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("test mnemonic phrase", { @@ -134,7 +134,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "garbage" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); expect(await screen.findByText("Bad mnemonic")).toBeTruthy(); }); @@ -145,7 +145,7 @@ describe("LoginModal", () => { render(); const button = screen.getByRole("button", { - name: /^login$/i, + name: /^sign in$/i, }) as HTMLButtonElement; expect(button.disabled).toBe(true); }); @@ -160,7 +160,7 @@ describe("LoginModal", () => { }); const button = screen.getByRole("button", { - name: /^login$/i, + name: /^sign in$/i, }) as HTMLButtonElement; expect(button.disabled).toBe(true); }); @@ -182,7 +182,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); const connectingButton = (await screen.findByRole("button", { name: /connecting/i, @@ -210,7 +210,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("phrase", { @@ -255,7 +255,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("phrase", { @@ -313,7 +313,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("phrase", { @@ -337,7 +337,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "phrase" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("phrase", { @@ -470,7 +470,7 @@ describe("LoginModal", () => { screen.getByPlaceholderText(/mnemonic phrase/i) as HTMLInputElement, { target: { value: "fresh mnemonic" } }, ); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("fresh mnemonic", { @@ -581,7 +581,7 @@ describe("LoginModal", () => { fireEvent.change(screen.getByPlaceholderText(/mnemonic phrase/i), { target: { value: "cVHcfvcWNc7DvqaPCwM6Z3DqZ" }, }); - fireEvent.click(screen.getByRole("button", { name: /^login$/i })); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); await waitFor(() => { expect(login).toHaveBeenCalledWith("cVHcfvcWNc7DvqaPCwM6Z3DqZ", { @@ -690,7 +690,7 @@ describe("LoginModal", () => { securityLevelName: "CRITICAL", }); const button = screen.getByRole("button", { - name: /^login$/i, + name: /^sign in$/i, }) as HTMLButtonElement; expect(button.disabled).toBe(true); }); @@ -702,7 +702,7 @@ describe("LoginModal", () => { dpnsName: null, }); const button = screen.getByRole("button", { - name: /^login$/i, + name: /^sign in$/i, }) as HTMLButtonElement; expect(button.disabled).toBe(true); }); @@ -712,7 +712,7 @@ describe("LoginModal", () => { // be able to hit Enter immediately after pasting. renderWithSecret({ status: "checking" }); const button = screen.getByRole("button", { - name: /^login$/i, + name: /^sign in$/i, }) as HTMLButtonElement; expect(button.disabled).toBe(false); }); diff --git a/example-apps/dashnote/test/NotesWorkspace.test.tsx b/example-apps/dashnote/test/NotesWorkspace.test.tsx index 9e7da1b..e615bfd 100644 --- a/example-apps/dashnote/test/NotesWorkspace.test.tsx +++ b/example-apps/dashnote/test/NotesWorkspace.test.tsx @@ -113,7 +113,7 @@ describe("NotesWorkspace", () => { ); expect(screen.getByText(/sign in to see your notes/i)).toBeTruthy(); - const loginButton = screen.getByRole("button", { name: /^log in$/i }); + const loginButton = screen.getByRole("button", { name: /^sign in$/i }); fireEvent.click(loginButton); expect(onOpenLogin).toHaveBeenCalled(); expect(screen.queryByRole("button", { name: /new note/i })).toBeNull(); diff --git a/example-apps/dashnote/test/SettingsPanel.test.tsx b/example-apps/dashnote/test/SettingsPanel.test.tsx index a5a7414..a76c25f 100644 --- a/example-apps/dashnote/test/SettingsPanel.test.tsx +++ b/example-apps/dashnote/test/SettingsPanel.test.tsx @@ -81,7 +81,7 @@ describe("SettingsPanel", () => { mockUseSession.mockReturnValue( makeSession({ identityId: "id-abc", dpnsName: "alice" }), ); - render(); + render(); const block = screen.getByTestId("settings-identity-block"); expect(within(block).getByText("id-abc")).toBeTruthy(); expect(within(block).getByText("✓ alice.dash")).toBeTruthy(); @@ -91,29 +91,32 @@ describe("SettingsPanel", () => { mockUseSession.mockReturnValue( makeSession({ identityId: "id-abc", dpnsName: null }), ); - render(); + render(); const block = screen.getByTestId("settings-identity-block"); expect(within(block).queryByText(/\.dash$/)).toBeNull(); }); it("renders the network indicator as testnet", () => { mockUseSession.mockReturnValue(makeSession()); - render(); + render(); expect(screen.getByText("testnet")).toBeTruthy(); }); - it("shows an empty state when not signed in or browsing", () => { + it("shows an empty state with a Sign in button when not signed in or browsing", () => { mockUseSession.mockReturnValue( makeSession({ status: "readonly", identityId: null, keyManager: null }), ); - render(); + const onOpenLogin = vi.fn(); + render(); expect(screen.getByText(/sign in to view/i)).toBeTruthy(); expect(screen.queryByTestId("settings-identity-block")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: /^sign in$/i })); + expect(onOpenLogin).toHaveBeenCalled(); }); it("hides the danger zone when nothing is remembered", () => { mockUseSession.mockReturnValue(makeSession({ rememberedIdentityId: null })); - render(); + render(); expect( screen.queryByRole("button", { name: /forget this device/i }), ).toBeNull(); @@ -127,7 +130,7 @@ describe("SettingsPanel", () => { forgetIdentity, }), ); - render(); + render(); fireEvent.click( screen.getByRole("button", { name: /forget this device/i }), ); @@ -141,7 +144,7 @@ describe("SettingsPanel", () => { value: { writeText }, }); mockUseSession.mockReturnValue(makeSession({ identityId: "id-xyz" })); - render(); + render(); fireEvent.click(screen.getByRole("button", { name: /copy identity id/i })); await waitFor(() => { expect(writeText).toHaveBeenCalledWith("id-xyz"); @@ -153,7 +156,7 @@ describe("SettingsPanel", () => { mockUseSession.mockReturnValue( makeSession({ contractId: "old", setContractId }), ); - render(); + render(); const input = screen.getByPlaceholderText(/paste a note contract id/i); fireEvent.change(input, { target: { value: " new-contract " } }); fireEvent.click(screen.getByRole("button", { name: /use this id/i })); @@ -169,7 +172,7 @@ describe("SettingsPanel", () => { mockUseSession.mockReturnValue( makeSession({ setContractId, sdk, keyManager, log }), ); - render(); + render(); fireEvent.click( screen.getByRole("button", { name: /register a fresh contract/i }), ); @@ -197,7 +200,7 @@ describe("SettingsPanel", () => { }), ); mockUseSession.mockReturnValue(makeSession({ setContractId: vi.fn() })); - render(); + render(); const button = screen.getByRole("button", { name: /register a fresh contract/i, }); @@ -214,7 +217,7 @@ describe("SettingsPanel", () => { const setContractId = vi.fn(); mockRegisterContract.mockRejectedValue(new Error("Network down")); mockUseSession.mockReturnValue(makeSession({ setContractId })); - render(); + render(); fireEvent.click( screen.getByRole("button", { name: /register a fresh contract/i }), ); @@ -224,7 +227,7 @@ describe("SettingsPanel", () => { it("invokes clearCachedNotes with the current identity ID", () => { mockUseSession.mockReturnValue(makeSession({ identityId: "id-cache" })); - render(); + render(); fireEvent.click( screen.getByRole("button", { name: /clear local cache for this device/i, diff --git a/example-apps/dashnote/test/e2e/auth.spec.ts b/example-apps/dashnote/test/e2e/auth.spec.ts index f5ba186..649d54a 100644 --- a/example-apps/dashnote/test/e2e/auth.spec.ts +++ b/example-apps/dashnote/test/e2e/auth.spec.ts @@ -93,7 +93,9 @@ test("Switch identity from the IdentityCard menu re-opens the login form", async const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); await expect(dialog.getByPlaceholder(/mnemonic phrase|wif/i)).toBeVisible(); - await expect(dialog.getByRole("button", { name: /^Login$/ })).toBeDisabled(); + await expect( + dialog.getByRole("button", { name: /^Sign in$/ }), + ).toBeDisabled(); }); test("Settings tab is reachable from both the IdentityCard menu and the sidebar NavButton", async ({ diff --git a/example-apps/dashnote/test/e2e/fixtures.ts b/example-apps/dashnote/test/e2e/fixtures.ts index 74c725f..bf1e817 100644 --- a/example-apps/dashnote/test/e2e/fixtures.ts +++ b/example-apps/dashnote/test/e2e/fixtures.ts @@ -52,7 +52,7 @@ function isMobile(page: Page): boolean { } /** - * Resolve a sidebar nav button (Notes / How it works / Login) by label. + * Resolve a sidebar nav button (Notes / How it works / Sign in) by label. * * On mobile the sidebar is off-canvas; this helper opens the hamburger * drawer first so the button is in the visible viewport. @@ -74,9 +74,11 @@ export async function navButton(page: Page, label: RegExp | string) { } /** - * Open the IdentityCard menu (the popover with Settings / Switch identity - * / Log out items). Only available when the session is `authenticated` or - * `browsing` — caller is responsible for being in that state. + * Open the IdentityCard menu (the popover with Settings, Switch identity + * or Sign in, and Log out items). Only available when the session is + * `authenticated` or `browsing` — caller is responsible for being in that + * state. The middle entry reads "Switch identity" when authenticated and + * "Sign in" when browsing read-only. * * On mobile the sidebar is off-canvas; opens the drawer transparently * via the hamburger first. @@ -102,9 +104,14 @@ export async function openIdentityMenu(page: Page) { } /** - * Open the LoginModal via the sidebar "Login" nav button, fill the - * mnemonic from PLATFORM_MNEMONIC, submit, and wait for the - * IdentityCard to report `Authenticated`. + * Open the LoginModal, fill the mnemonic from PLATFORM_MNEMONIC, submit, + * and wait for the IdentityCard to report `Authenticated`. + * + * The entry point depends on session state: in `idle/connecting/readonly` + * the sidebar exposes a "Sign in" NavButton; in `browsing` (remembered + * identity after reload) the sidebar entry is hidden and the modal is + * reached via the IdentityCard menu instead. We detect which surface + * exists and use whichever one is visible. * * Defaults to `rememberMe: false` (the modal's default is true) so tests * start from a clean localStorage; opt in explicitly when exercising the @@ -121,7 +128,17 @@ export async function loginViaModal( throw new Error("PLATFORM_MNEMONIC is required for loginViaModal"); } - await (await navButton(page, /login$/i)).click(); + const browsing = await page + .getByText("Browsing (read-only)", { exact: true }) + .isVisible() + .catch(() => false); + + if (browsing) { + await openIdentityMenu(page); + await page.getByRole("menuitem", { name: /^sign in$/i }).click(); + } else { + await (await navButton(page, /sign in$/i)).click(); + } const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); @@ -132,7 +149,7 @@ export async function loginViaModal( if (!rememberMe) { await rememberCheckbox.uncheck(); } - await dialog.getByRole("button", { name: /^Login$/ }).click(); + await dialog.getByRole("button", { name: /^Sign in$/ }).click(); await expect(dialog).toBeHidden({ timeout: 60_000 }); await expect(page.getByText("Authenticated", { exact: true })).toBeVisible({ diff --git a/example-apps/dashnote/test/e2e/smoke.spec.ts b/example-apps/dashnote/test/e2e/smoke.spec.ts index 81ffebc..cee777a 100644 --- a/example-apps/dashnote/test/e2e/smoke.spec.ts +++ b/example-apps/dashnote/test/e2e/smoke.spec.ts @@ -61,15 +61,15 @@ test.describe("theme toggle", () => { }); test.describe("login modal", () => { - test("sidebar Login button opens the modal", async ({ page }) => { - await (await navButton(page, /login$/i)).click(); + test("sidebar Sign in button opens the modal", async ({ page }) => { + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); await expect(dialog.getByPlaceholder(/mnemonic phrase|wif/i)).toBeVisible(); }); test("Cancel button closes the modal", async ({ page }) => { - await (await navButton(page, /login$/i)).click(); + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); await dialog.getByRole("button", { name: /^cancel$/i }).click(); @@ -77,19 +77,19 @@ test.describe("login modal", () => { }); test("Escape closes the modal", async ({ page }) => { - await (await navButton(page, /login$/i)).click(); + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); await expect(dialog).toBeVisible(); await page.keyboard.press("Escape"); await expect(dialog).toBeHidden(); }); - test("Login button is disabled until a secret is entered", async ({ + test("Sign in button is disabled until a secret is entered", async ({ page, }) => { - await (await navButton(page, /login$/i)).click(); + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); - const submit = dialog.getByRole("button", { name: /^Login$/ }); + const submit = dialog.getByRole("button", { name: /^Sign in$/ }); await expect(submit).toBeDisabled(); await dialog.getByPlaceholder(/mnemonic phrase|wif/i).fill("abandon"); await expect(submit).toBeEnabled(); @@ -98,7 +98,7 @@ test.describe("login modal", () => { test("Advanced settings reveals the identity-index field for mnemonic input", async ({ page, }) => { - await (await navButton(page, /login$/i)).click(); + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); // Type something with whitespace so detectSecretShape() picks "mnemonic" // and the identity-index field is rendered inside Advanced. @@ -113,7 +113,7 @@ test.describe("login modal", () => { }) => { // WIF input has no DIP-13 derivation, so identity-index is irrelevant // and the whole Advanced disclosure should disappear. - await (await navButton(page, /login$/i)).click(); + await (await navButton(page, /sign in$/i)).click(); const dialog = page.getByRole("dialog"); // Mnemonic-shaped input first → disclosure renders. From c6a6d792c88b20e2c654afc43aab1fc11e66ff41 Mon Sep 17 00:00:00 2001 From: thephez Date: Wed, 13 May 2026 17:37:31 -0400 Subject: [PATCH 3/3] feat(dashnote): add note delete confirmation modal Replaces window.confirm with a DeleteNoteModal that shows the note title in body copy, uses a danger-styled Delete button, disables both actions while a delete is in flight, and ignores Escape, Cancel, and backdrop dismiss mid-delete to keep parent state in sync. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/components/DeleteNoteModal.tsx | 55 +++++++ .../src/components/NotesWorkspace.tsx | 21 ++- .../dashnote/test/DeleteNoteModal.test.tsx | 150 ++++++++++++++++++ .../dashnote/test/NotesWorkspace.test.tsx | 15 +- example-apps/dashnote/test/e2e/fixtures.ts | 12 +- 5 files changed, 243 insertions(+), 10 deletions(-) create mode 100644 example-apps/dashnote/src/components/DeleteNoteModal.tsx create mode 100644 example-apps/dashnote/test/DeleteNoteModal.test.tsx diff --git a/example-apps/dashnote/src/components/DeleteNoteModal.tsx b/example-apps/dashnote/src/components/DeleteNoteModal.tsx new file mode 100644 index 0000000..3f4fc80 --- /dev/null +++ b/example-apps/dashnote/src/components/DeleteNoteModal.tsx @@ -0,0 +1,55 @@ +import { Modal } from "./Modal"; + +export interface DeleteNoteModalProps { + open: boolean; + noteTitle: string; + deleting: boolean; + onCancel: () => void; + onConfirm: () => void; +} + +export function DeleteNoteModal({ + open, + noteTitle, + deleting, + onCancel, + onConfirm, +}: DeleteNoteModalProps) { + const trimmed = noteTitle.trim(); + const subject = trimmed ? `“${trimmed}”` : "this note"; + + return ( + { + if (!deleting) onCancel(); + }} + footer={ + <> + + + + } + > +

+ Permanently delete {subject} from Dash Platform? This can't be + undone. +

+ + ); +} diff --git a/example-apps/dashnote/src/components/NotesWorkspace.tsx b/example-apps/dashnote/src/components/NotesWorkspace.tsx index 3c6578e..5139db6 100644 --- a/example-apps/dashnote/src/components/NotesWorkspace.tsx +++ b/example-apps/dashnote/src/components/NotesWorkspace.tsx @@ -11,6 +11,7 @@ import { createNote } from "../dash/createNote"; import { deleteNote } from "../dash/deleteNote"; import { getNote, listMyNotes, type NoteRecord } from "../dash/queries"; import { updateNote } from "../dash/updateNote"; +import { DeleteNoteModal } from "./DeleteNoteModal"; import { useMediaQuery } from "../hooks/useMediaQuery"; import { byteLength, FIELD_BYTE_LIMIT } from "../lib/fieldLimits"; import { errorMessage } from "../lib/logger"; @@ -71,6 +72,7 @@ export function NotesWorkspace({ const [detailLoading, setDetailLoading] = useState(false); const [saving, setSaving] = useState(false); const [deleting, setDeleting] = useState(false); + const [deleteRequested, setDeleteRequested] = useState(false); const [error, setError] = useState(null); const [revalidating, setRevalidating] = useState(false); const [editsReady, setEditsReady] = useState(false); @@ -545,13 +547,18 @@ export function NotesWorkspace({ } } - async function handleDelete() { + function requestDelete() { if (!sdk || !keyManager || !contractId || !isAuthed || !selectedId) return; if (selectedId === "new") { resetDraft(); return; } - if (!window.confirm("Delete this note permanently?")) return; + setDeleteRequested(true); + } + + async function confirmDelete() { + if (!sdk || !keyManager || !contractId || !isAuthed || !selectedId) return; + if (selectedId === "new") return; setDeleting(true); setError(null); @@ -564,6 +571,7 @@ export function NotesWorkspace({ noteId: selectedId, log, }); + setDeleteRequested(false); await reloadNotes(); } catch (err) { setError(errorMessage(err)); @@ -652,7 +660,7 @@ export function NotesWorkspace({ onTitleChange={setTitle} onMessageChange={setMessage} onSave={() => void handleSave()} - onDelete={() => void handleDelete()} + onDelete={requestDelete} onBack={handleBack} loading={detailLoading} saving={saving} @@ -673,6 +681,13 @@ export function NotesWorkspace({ )} + setDeleteRequested(false)} + onConfirm={() => void confirmDelete()} + /> ); } diff --git a/example-apps/dashnote/test/DeleteNoteModal.test.tsx b/example-apps/dashnote/test/DeleteNoteModal.test.tsx new file mode 100644 index 0000000..f891399 --- /dev/null +++ b/example-apps/dashnote/test/DeleteNoteModal.test.tsx @@ -0,0 +1,150 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { DeleteNoteModal } from "../src/components/DeleteNoteModal"; + +afterEach(() => { + cleanup(); +}); + +describe("DeleteNoteModal", () => { + it("renders nothing when open is false", () => { + const { container } = render( + , + ); + expect(container.firstChild).toBeNull(); + }); + + it("shows the note title in the body when present", () => { + render( + , + ); + expect(screen.getByText(/groceries/i)).toBeTruthy(); + }); + + it("falls back to 'this note' when the title is blank", () => { + render( + , + ); + expect(screen.getByText(/this note/i)).toBeTruthy(); + }); + + it("fires onConfirm when Delete is clicked", () => { + const onConfirm = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("button", { name: /^delete$/i })); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); + + it("fires onCancel when Cancel is clicked", () => { + const onCancel = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("button", { name: /^cancel$/i })); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("disables both buttons while a delete is in flight", () => { + render( + , + ); + expect( + screen.getByRole("button", { name: /deleting…/i }).hasAttribute("disabled"), + ).toBe(true); + expect( + screen.getByRole("button", { name: /^cancel$/i }).hasAttribute("disabled"), + ).toBe(true); + }); + + it("does not fire onCancel when the disabled Cancel button is clicked mid-delete", () => { + const onCancel = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("button", { name: /^cancel$/i })); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it("ignores Escape dismiss while deleting", () => { + const onCancel = vi.fn(); + render( + , + ); + // The shared Modal listens for Escape on window and calls onClose; + // DeleteNoteModal's onClose wrapper short-circuits while deleting. + fireEvent.keyDown(window, { key: "Escape" }); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it("ignores backdrop dismiss while deleting", () => { + const onCancel = vi.fn(); + render( + , + ); + // The shared Modal forwards backdrop clicks through onClose. The + // wrapper guards onCancel when deleting=true so an accidental + // dismiss mid-delete doesn't desync the parent's state. + const dialog = screen.getByRole("dialog"); + fireEvent.click(dialog.parentElement!); + expect(onCancel).not.toHaveBeenCalled(); + }); +}); diff --git a/example-apps/dashnote/test/NotesWorkspace.test.tsx b/example-apps/dashnote/test/NotesWorkspace.test.tsx index e615bfd..ec7f151 100644 --- a/example-apps/dashnote/test/NotesWorkspace.test.tsx +++ b/example-apps/dashnote/test/NotesWorkspace.test.tsx @@ -6,6 +6,7 @@ import { render, screen, waitFor, + within, } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -239,6 +240,10 @@ describe("NotesWorkspace", () => { }); fireEvent.click(screen.getByRole("button", { name: /^delete$/i })); + const confirmDialog = screen.getByRole("dialog"); + fireEvent.click( + within(confirmDialog).getByRole("button", { name: /^delete$/i }), + ); await waitFor(() => { expect(mockDeleteNote).toHaveBeenCalledWith( expect.objectContaining({ noteId: "note-2" }), @@ -664,7 +669,7 @@ describe("NotesWorkspace", () => { ).toBe("Edited offline"); }); - it("deletes a note via the bottom 'Delete note' button", async () => { + it("deletes a note via the bottom 'Delete note' button after confirming the modal", async () => { mockUseSession.mockReturnValue(makeSession()); mockListMyNotes.mockResolvedValue([noteFixture]); mockGetNote.mockResolvedValue(noteFixture); @@ -682,6 +687,14 @@ describe("NotesWorkspace", () => { fireEvent.click(screen.getByRole("button", { name: /delete note/i })); + // Trigger doesn't fire the delete directly anymore — the + // confirmation modal must be acknowledged first. + expect(mockDeleteNote).not.toHaveBeenCalled(); + const dialog = screen.getByRole("dialog"); + fireEvent.click( + within(dialog).getByRole("button", { name: /^delete$/i }), + ); + await waitFor(() => { expect(mockDeleteNote).toHaveBeenCalledWith( expect.objectContaining({ noteId: "note-mobile" }), diff --git a/example-apps/dashnote/test/e2e/fixtures.ts b/example-apps/dashnote/test/e2e/fixtures.ts index bf1e817..324dc92 100644 --- a/example-apps/dashnote/test/e2e/fixtures.ts +++ b/example-apps/dashnote/test/e2e/fixtures.ts @@ -268,12 +268,6 @@ export async function deleteNoteByTitle(page: Page, title: string) { timeout: 30_000, }); - // window.confirm("Delete this note permanently?") — auto-accept. - const dialogHandler = (dialog: import("@playwright/test").Dialog) => { - void dialog.accept(); - }; - page.once("dialog", dialogHandler); - // Desktop renders the "Delete" button in the editor header; mobile // renders "Delete note" near the bottom. Match either label. await page @@ -281,6 +275,12 @@ export async function deleteNoteByTitle(page: Page, title: string) { .first() .click(); + // DeleteNoteModal opens for confirmation; scope to its dialog so we + // don't accidentally re-match the editor's Delete trigger. + const confirmDialog = page.getByRole("dialog"); + await expect(confirmDialog).toBeVisible(); + await confirmDialog.getByRole("button", { name: /^delete$/i }).click(); + // Item leaves the list after reloadNotes resolves. await expect(page.locator("button", { hasText: title })).toHaveCount(0, { timeout: 60_000,