From ee377ebb1a78836c2c0716b8f3cad7c8a3112abe Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sat, 24 Oct 2026 23:13:20 +0200 Subject: [PATCH 1/6] users: reset-password shows the password-policy reason and allows retry with the same token (#667) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 4 + .../tests-js/users_ResetPassword.test.tsx | 114 ++++++++++++++++++ .../tests/test_users_reset_password_retry.py | 40 ++++++ .../users/auth_local/resetPasswordError.ts | 13 ++ modules/users/users/pages/ResetPassword.tsx | 4 +- 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 modules/users/tests-js/users_ResetPassword.test.tsx create mode 100644 modules/users/tests/test_users_reset_password_retry.py create mode 100644 modules/users/users/auth_local/resetPasswordError.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e0bfd2339..a9074fcf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] +### Fixed +- Users: show password policy reasons on the reset screen instead of suggesting + that a valid link expired; allow retry with the same token (#667). + ## [0.0.40] — 2026-10-24 ### Fixed diff --git a/modules/users/tests-js/users_ResetPassword.test.tsx b/modules/users/tests-js/users_ResetPassword.test.tsx new file mode 100644 index 000000000..a1bb4e21c --- /dev/null +++ b/modules/users/tests-js/users_ResetPassword.test.tsx @@ -0,0 +1,114 @@ +import { configureI18n, keys } from '@simple-module-py/i18n'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { json, sent, stubFetch } from './users_fixtures'; + +vi.mock('@inertiajs/react', () => ({ + Head: () => null, + router: { visit: vi.fn() }, + usePage: () => ({ props: { branding: { appName: 'Acme', logoUrl: null } } }), +})); + +import ResetPassword from '../users/pages/ResetPassword'; + +configureI18n({ + locale: 'en', + messages: { + [keys.users.common.error_try_again]: 'Please try again.', + [keys.users.reset_password.error_failed]: 'The reset link may have expired.', + [keys.users.reset_password.saved_title]: 'Password saved', + [keys.users.reset_password.submit]: 'Save password', + }, +}); + +afterEach(() => vi.unstubAllGlobals()); + +function reset() { + render( + , + ); +} + +function submit(password: string) { + for (const id of ['password', 'confirm']) { + fireEvent.change(document.getElementById(id) as HTMLElement, { target: { value: password } }); + } + fireEvent.submit(document.querySelector('form') as HTMLFormElement); +} + +describe('ResetPassword password policy errors (#667)', () => { + it.each([ + ['short', 'Password must be at least 8 characters, not counting spaces'], + ['12345678', 'Password cannot be all numbers'], + ])('shows the policy reason for %s and retries with the same token', async (weak, reason) => { + const fetch = stubFetch(() => + json(400, { detail: { code: 'RESET_PASSWORD_INVALID_PASSWORD', reason } }), + ); + reset(); + submit(weak); + expect(await screen.findByText(reason)).toBeInTheDocument(); + expect(screen.queryByText('The reset link may have expired.')).toBeNull(); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Save password' })).toBeEnabled(), + ); + fetch.mockImplementation(async () => json(200, null)); + submit('CompliantPass1!'); + expect(await screen.findByRole('heading', { name: 'Password saved' })).toBeInTheDocument(); + expect(screen.queryByText(reason)).toBeNull(); + expect(JSON.parse((await sent(fetch, 0)).text)).toEqual({ + token: 'same-token', + password: weak, + }); + expect(JSON.parse((await sent(fetch, 1)).text)).toEqual({ + token: 'same-token', + password: 'CompliantPass1!', + }); + }); + + it.each([ + null, + { code: 'UNRELATED', reason: 'Do not display' }, + { code: 'RESET_PASSWORD_INVALID_PASSWORD', reason: 42 }, + { code: 'RESET_PASSWORD_INVALID_PASSWORD', reason: '' }, + { code: 'RESET_PASSWORD_INVALID_PASSWORD', reason: ' ' }, + ['RESET_PASSWORD_INVALID_PASSWORD'], + ])('falls back safely for malformed or unrelated detail %j', async (detail) => { + stubFetch(() => json(400, { detail })); + reset(); + submit('short'); + expect(await screen.findByText('The reset link may have expired.')).toBeInTheDocument(); + expect(screen.queryByText('Do not display')).toBeNull(); + }); + + it('does not treat a server failure as a password policy rejection', async () => { + stubFetch(() => + json(500, { + detail: { code: 'RESET_PASSWORD_INVALID_PASSWORD', reason: 'Do not display' }, + }), + ); + reset(); + submit('short'); + expect(await screen.findByText('The reset link may have expired.')).toBeInTheDocument(); + expect(screen.queryByText('Do not display')).toBeNull(); + }); + + it('preserves bad-token details', async () => { + stubFetch(() => json(400, { detail: 'RESET_PASSWORD_BAD_TOKEN' })); + reset(); + submit('CompliantPass1!'); + expect(await screen.findByText('RESET_PASSWORD_BAD_TOKEN')).toBeInTheDocument(); + }); + + it('keeps network failures separate from expired links', async () => { + stubFetch(() => Promise.reject(new TypeError('Offline'))); + reset(); + submit('CompliantPass1!'); + expect(await screen.findByText('Please try again.')).toBeInTheDocument(); + expect(screen.queryByText('The reset link may have expired.')).toBeNull(); + }); +}); diff --git a/modules/users/tests/test_users_reset_password_retry.py b/modules/users/tests/test_users_reset_password_retry.py new file mode 100644 index 000000000..e5e6b0bb6 --- /dev/null +++ b/modules/users/tests/test_users_reset_password_retry.py @@ -0,0 +1,40 @@ +"""A password policy rejection must leave a fresh reset token usable (#667).""" + +from urllib.parse import parse_qs, urlsplit + +import pytest +from _users_app_builders import _make_standard_user + + +@pytest.mark.parametrize( + ("password", "reason"), + [ + ("short", "Password must be at least 8 characters, not counting spaces"), + ("12345678", "Password cannot be all numbers"), + ], +) +async def test_users_reset_rejected_password_allows_same_token_retry( + admin_client, anon_client, users_app, password, reason +): + user = await _make_standard_user(users_app) + link = await admin_client.post(f"/api/users/admin/{user.id}/reset-password-link") + assert link.status_code == 200, link.text + token = parse_qs(urlsplit(link.json()["link"]).query)["token"][0] + + rejected = await anon_client.post( + "/api/users/auth/reset-password", json={"token": token, "password": password} + ) + assert rejected.status_code == 400, rejected.text + assert rejected.json()["detail"] == { + "code": "RESET_PASSWORD_INVALID_PASSWORD", + "reason": reason, + } + + accepted = await anon_client.post( + "/api/users/auth/reset-password", json={"token": token, "password": "CompliantPass1!"} + ) + assert accepted.status_code == 200, accepted.text + login = await anon_client.post( + "/api/users/auth/login", data={"username": user.email, "password": "CompliantPass1!"} + ) + assert login.status_code == 204, login.text diff --git a/modules/users/users/auth_local/resetPasswordError.ts b/modules/users/users/auth_local/resetPasswordError.ts new file mode 100644 index 000000000..a77f4fcc7 --- /dev/null +++ b/modules/users/users/auth_local/resetPasswordError.ts @@ -0,0 +1,13 @@ +/** Extract only fastapi-users' password-policy reason; other failures keep their fallback. */ +export function resetPasswordReason(error: unknown): string | null { + if (!error || typeof error !== 'object' || !('detail' in error)) return null; + const detail = error.detail; + if (!detail || typeof detail !== 'object' || !('code' in detail) || !('reason' in detail)) { + return null; + } + return detail.code === 'RESET_PASSWORD_INVALID_PASSWORD' && + typeof detail.reason === 'string' && + detail.reason.trim() !== '' + ? detail.reason + : null; +} diff --git a/modules/users/users/pages/ResetPassword.tsx b/modules/users/users/pages/ResetPassword.tsx index 0b82447d0..db6efaa3f 100644 --- a/modules/users/users/pages/ResetPassword.tsx +++ b/modules/users/users/pages/ResetPassword.tsx @@ -9,6 +9,7 @@ import { CheckCircle2, TimerOff } from 'lucide-react'; import { useState } from 'react'; import { AuthStateCard } from '../auth_local/components/AuthStateCard'; import { PasswordFields } from '../auth_local/components/PasswordFields'; +import { resetPasswordReason } from '../auth_local/resetPasswordError'; import { api } from './api'; import { signIn } from './signIn'; @@ -64,7 +65,8 @@ function ResetPassword({ return; } if (res.status !== 200 && res.status !== 204) { - setError(res.detail ?? t(keys.users.reset_password.error_failed)); + const reason = res.status === 400 ? resetPasswordReason(res.error) : null; + setError(reason ?? res.detail ?? t(keys.users.reset_password.error_failed)); return; } if (await signedIn()) { From 0b07e25a763be925b468e8920a93bd3d134c2fe9 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sat, 24 Oct 2026 23:13:48 +0200 Subject: [PATCH 2/6] settings: field rows size by the editor card (container query), stack when narrow, wrap long identifiers (#669) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 3 + .../settings/pages/components/FieldSource.tsx | 6 +- .../pages/components/ModuleFieldRow.tsx | 22 +++---- .../settings/pages/components/ModuleForm.tsx | 4 +- modules/settings/tests-js/ModuleForm.test.tsx | 2 +- .../settings_ModuleFieldLayout.test.tsx | 62 +++++++++++++++++++ 6 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 modules/settings/tests-js/settings_ModuleFieldLayout.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index a9074fcf6..7625a4884 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ All notable changes to this project are documented in this file. The format is b ### Fixed - Users: show password policy reasons on the reset screen instead of suggesting that a valid link expired; allow retry with the same token (#667). +- Settings: size module field rows by the editor card width, stack controls in narrow + cards, and wrap long field/ENV identifiers to prevent collapsed inputs and horizontal + overflow with two sidebars (#669). ## [0.0.40] — 2026-10-24 diff --git a/modules/settings/settings/pages/components/FieldSource.tsx b/modules/settings/settings/pages/components/FieldSource.tsx index 0dfbce2ee..df791f209 100644 --- a/modules/settings/settings/pages/components/FieldSource.tsx +++ b/modules/settings/settings/pages/components/FieldSource.tsx @@ -55,10 +55,10 @@ export function FieldSource({ field, onReset }: { field: FieldMeta; onReset: () const showDefault = !isBlank(field.default) && String(field.default) !== String(field.value); return ( // Wraps rather than clips: `SM_FILE_STORAGE_QUOTA_BYTES` has no space to - // break at, so a 210px column cut it mid-word with nothing to say so. - // `break-words` breaks only the tokens that do not fit. + // break at, and as an unbreakable token it would set the column's minimum + // width; `overflow-wrap:anywhere` lets it break inside the identifier. + // Query the editor card: two sidebars can leave little space at a wide viewport. +
-
-
+
@@ -75,7 +75,7 @@ export function ModuleFieldRow({ {error && (

{error}

diff --git a/modules/settings/settings/pages/components/ModuleForm.tsx b/modules/settings/settings/pages/components/ModuleForm.tsx index 0b4e8794a..ee2ef0479 100644 --- a/modules/settings/settings/pages/components/ModuleForm.tsx +++ b/modules/settings/settings/pages/components/ModuleForm.tsx @@ -134,7 +134,7 @@ export function ModuleForm({ module: m, checks = [] }: Props) { } return ( -
+
{/* Outside the card: the heading names the pane, and the deck keeps the card for the fields alone. */}
@@ -178,7 +178,7 @@ export function ModuleForm({ module: m, checks = [] }: Props) {
)} - + {/* No inner scroller: the card grows and the page scrolls, which is one scrollbar for the screen instead of three nested ones. */}
diff --git a/modules/settings/tests-js/ModuleForm.test.tsx b/modules/settings/tests-js/ModuleForm.test.tsx index 4639f999a..a47108ce5 100644 --- a/modules/settings/tests-js/ModuleForm.test.tsx +++ b/modules/settings/tests-js/ModuleForm.test.tsx @@ -105,7 +105,7 @@ describe('ModuleForm', () => { // control's cell, where it wrapped a word per line. const error = screen.getByText(/item rejected/); expect(error.contains(screen.getByLabelText('retention_days'))).toBe(false); - expect(error.className).toContain('lg:col-span-2'); + expect(error.className).toContain('@xl:col-span-2'); }); test('shows an error whose loc names no field at form level', async () => { diff --git a/modules/settings/tests-js/settings_ModuleFieldLayout.test.tsx b/modules/settings/tests-js/settings_ModuleFieldLayout.test.tsx new file mode 100644 index 000000000..919da11f3 --- /dev/null +++ b/modules/settings/tests-js/settings_ModuleFieldLayout.test.tsx @@ -0,0 +1,62 @@ +import { configureI18n } from '@simple-module-py/i18n'; +import { render, screen } from '@testing-library/react'; +import { expect, test } from 'vitest'; +import type { FieldMeta } from '../settings/pages/components/FieldInput'; +import { ModuleFieldRow } from '../settings/pages/components/ModuleFieldRow'; +import { ModuleForm } from '../settings/pages/components/ModuleForm'; + +configureI18n({ locale: 'en', messages: {} }); + +const field: FieldMeta = { + name: 'very_long_underscore_separated_setting_name', + env_var: 'SM_FILE_STORAGE_VERY_LONG_UNDERSCORE_SEPARATED_SETTING_NAME', + type: 'string', + value: '', + default: '', + description: '', + is_secret: false, + requires_restart: false, + group: null, +}; + +// jsdom has no layout engine: these guard the responsive CSS contract. +test('the editor card supplies the field rows container width', () => { + const { container } = render( + , + ); + expect(container.querySelector('[data-slot="card"]')).toHaveClass('@container', 'min-w-0'); +}); + +test('rows bound their tracks and wrap identifiers without viewport breakpoints', () => { + const { container } = render( + {}} + onReset={() => {}} + />, + ); + const row = container.firstElementChild; + expect(row).toHaveClass( + 'grid-cols-1', + '@md:grid-cols-[minmax(0,140px)_minmax(180px,1fr)]', + '@xl:grid-cols-[minmax(0,140px)_minmax(180px,1fr)_minmax(0,160px)]', + ); + expect(row?.className).not.toMatch(/(?:^|\s)(?:sm|lg):/); + expect(screen.getByText(field.name)).toHaveClass('[overflow-wrap:anywhere]'); + const source = screen.getByText(field.env_var); + expect(source).toHaveClass('[overflow-wrap:anywhere]'); + expect(source.parentElement).toHaveClass('min-w-0', '@md:col-span-2', '@xl:col-span-1'); + expect(screen.getByText('Invalid setting')).toHaveClass('@md:col-start-2', '@xl:col-span-2'); +}); From df97ac78b9d725c2275493f5622c018e6db2dd1a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sat, 24 Oct 2026 23:15:25 +0200 Subject: [PATCH 3/6] ui: closing the command palette returns focus to its opener (#668) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 3 + .../ui/src/components/CommandPalette.test.tsx | 128 ++++++++++++++++++ packages/ui/src/components/CommandPalette.tsx | 33 ++++- packages/ui/src/components/ui/command.tsx | 3 + 4 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 packages/ui/src/components/CommandPalette.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 7625a4884..e2feaea80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ All notable changes to this project are documented in this file. The format is b - Settings: size module field rows by the editor card width, stack controls in narrow cards, and wrap long field/ENV identifiers to prevent collapsed inputs and horizontal overflow with two sidebars (#669). +- Restore keyboard focus to the Search command palette's opener after dismissal, including Ctrl/Cmd+K toggles, without stealing focus after command navigation (#668). +- Raise shared muted sidebar text contrast above 4.5:1 on both the default and + hover backgrounds in light and dark modes, preserving secondary text hierarchy (#670). ## [0.0.40] — 2026-10-24 diff --git a/packages/ui/src/components/CommandPalette.test.tsx b/packages/ui/src/components/CommandPalette.test.tsx new file mode 100644 index 000000000..a45017a6a --- /dev/null +++ b/packages/ui/src/components/CommandPalette.test.tsx @@ -0,0 +1,128 @@ +import { router } from '@inertiajs/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { CommandPalette } from './CommandPalette'; + +vi.mock('@inertiajs/react', () => ({ router: { visit: vi.fn(), post: vi.fn() } })); +vi.mock('@simple-module-py/i18n', async (importOriginal) => ({ + ...(await importOriginal()), + useT: () => ({ t: (key: string) => key }), +})); + +beforeAll(() => { + // cmdk scrolls the selected result; jsdom has no layout or scrollIntoView. + HTMLElement.prototype.scrollIntoView = vi.fn(); +}); +afterEach(() => vi.resetAllMocks()); + +function setup() { + const view = render( + <> + + + + , + ); + return { + ...view, + user: userEvent.setup(), + search: screen.getByRole('button', { name: /trigger/ }), + }; +} + +async function expectClosedWithFocus(target: HTMLElement) { + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + await waitFor(() => expect(target).toHaveFocus()); +} + +describe('CommandPalette focus', () => { + it.each(['Escape', 'Close', 'overlay', 'Control+k', 'Meta+k'])( + 'restores Search after dismissal with %s and continues tab order', + async (dismiss) => { + const { user, search } = setup(); + await user.tab(); + await user.tab(); + expect(search).toHaveFocus(); + await user.keyboard('{Enter}'); + expect(screen.getByRole('combobox')).toHaveFocus(); + + if (dismiss === 'Close') await user.click(screen.getByRole('button', { name: 'Close' })); + else if (dismiss === 'overlay') { + const overlay = document.querySelector('[data-slot="dialog-overlay"]'); + if (!overlay) throw new Error('Missing dialog overlay'); + await user.click(overlay); + } else if (dismiss === 'Escape') await user.keyboard('{Escape}'); + else await user.keyboard(`{${dismiss.split('+')[0]}>}k{/${dismiss.split('+')[0]}}`); + + await expectClosedWithFocus(search); + await user.tab(); + expect(screen.getByRole('button', { name: 'After' })).toHaveFocus(); + }, + ); + + it.each(['Control', 'Meta'])( + 'restores the previous element after opening with %s+k', + async (key) => { + const { user } = setup(); + const before = screen.getByRole('button', { name: 'Before' }); + before.focus(); + await user.keyboard(`{${key}>}k{/${key}}`); + expect(screen.getByRole('combobox')).toHaveFocus(); + await user.keyboard('{Escape}'); + await expectClosedWithFocus(before); + }, + ); + + it.each(['Dashboard', 'Logout'])('does not restore focus after selecting %s', async (label) => { + const { user, search } = setup(); + await user.click(search); + const restore = vi.spyOn(search, 'focus'); + await user.click(screen.getByRole('option', { name: label })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + // Radix schedules close autofocus after unmount. + await act(() => new Promise((resolve) => setTimeout(resolve, 10))); + expect(restore).not.toHaveBeenCalled(); + expect(label === 'Logout' ? router.post : router.visit).toHaveBeenCalledWith( + label === 'Logout' ? '/logout' : '/dashboard', + ); + restore.mockRestore(); + + // A later dismissal must capture a fresh opener after navigation. + await user.click(search); + await user.keyboard('{Escape}'); + await expectClosedWithFocus(search); + }); + + it('ignores a disconnected opener', async () => { + const { user, container } = setup(); + const before = screen.getByRole('button', { name: 'Before' }); + before.focus(); + await user.keyboard('{Control>}k{/Control}'); + const restore = vi.spyOn(before, 'focus'); + container.removeChild(before); + await user.keyboard('{Escape}'); + await expectClosedWithFocus(document.body); + expect(restore).not.toHaveBeenCalled(); + restore.mockRestore(); + container.prepend(before); + }); + + it('does not restore an external opener when the palette unmounts', async () => { + const { user, rerender } = setup(); + const before = screen.getByRole('button', { name: 'Before' }); + before.focus(); + await user.keyboard('{Control>}k{/Control}'); + const restore = vi.spyOn(before, 'focus'); + rerender(); + await act(() => new Promise((resolve) => setTimeout(resolve, 10))); + expect(before.isConnected).toBe(true); + expect(restore).not.toHaveBeenCalled(); + restore.mockRestore(); + fireEvent.keyDown(document, { key: 'k', ctrlKey: true }); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/ui/src/components/CommandPalette.tsx b/packages/ui/src/components/CommandPalette.tsx index 6fc8c2cea..b51465aa0 100644 --- a/packages/ui/src/components/CommandPalette.tsx +++ b/packages/ui/src/components/CommandPalette.tsx @@ -9,7 +9,7 @@ import { CommandList, } from '@simple-module-py/ui/components/ui/command'; import { Search } from 'lucide-react'; -import { useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { isPostMenuItem, type MenuItem } from '../types'; import { NavIcon } from './NavIcon'; @@ -32,19 +32,37 @@ interface CommandPaletteProps { export function CommandPalette({ navItems, accountItems }: CommandPaletteProps) { const { t } = useT(); const [open, setOpen] = useState(false); + const returnFocus = useRef(null); + + const changeOpen = useCallback((nextOpen: boolean) => { + if (nextOpen) { + returnFocus.current = + document.activeElement instanceof HTMLElement ? document.activeElement : null; + } + setOpen(nextOpen); + }, []); + + useEffect( + () => () => { + returnFocus.current = null; + }, + [], + ); useEffect(() => { const onKey = (e: KeyboardEvent) => { if (e.key.toLowerCase() === 'k' && (e.metaKey || e.ctrlKey)) { e.preventDefault(); - setOpen((prev) => !prev); + changeOpen(!open); } }; document.addEventListener('keydown', onKey); return () => document.removeEventListener('keydown', onKey); - }, []); + }, [open, changeOpen]); const go = (item: MenuItem) => { + // Navigation owns focus on the destination, even when this layout stays mounted. + returnFocus.current = null; setOpen(false); if (isPostMenuItem(item)) router.post(item.url); else router.visit(item.url); @@ -75,7 +93,7 @@ export function CommandPalette({ navItems, accountItems }: CommandPaletteProps) <>