Skip to content

Commit 2eaa316

Browse files
committed
fix(settings): require non-empty text inputs, stop the token field closing on empty Enter, and clarify the credentials error
TextInputModal now gates OK on a non-empty (trimmed) value via bOKDisabled and only runs the confirm/close on Enter when the value is non-empty, so an empty or whitespace-only field can no longer be confirmed with R2/Enter on the Deck. The ConnectModal token field is wrapped in a Focusable (the structure proven not to close the modal on an incomplete field) and all three keydown handlers consume Enter with preventDefault/stopPropagation before deciding to submit, advance, or no-op, so R2/OSK-Enter on the empty token field no longer closes the modal. The credentials 403 message now names both possible causes — wrong username/password and an account without token-creation permission — because RomM returns 403 indistinguishably for both.
1 parent ea2c000 commit 2eaa316

6 files changed

Lines changed: 118 additions & 23 deletions

File tree

py_modules/services/connection.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@
5050
_INVALID_URL_MESSAGE = "Enter a valid http(s):// server URL"
5151

5252
_FORBIDDEN_TOKEN_MESSAGE = (
53-
"Your RomM account cannot create API tokens — ask your admin to grant "
54-
"token permissions or use an account with a higher role."
53+
"RomM rejected the sign-in. Check your username and password — if they are correct, your account may not be "
54+
"allowed to create API tokens (ask your admin or use an account with a higher role)."
5555
)
5656

5757
# Pasted-token validation (``establish_user_token``): a 401 means the token
@@ -279,8 +279,10 @@ async def establish_token(
279279
minted = await self._loop.run_in_executor(None, self._mint, username, password)
280280
except RommForbiddenError:
281281
# 403 on token mint: same AUTH_FAILED slug as a 401, but a distinct
282-
# message — the account lacks token-creation permission (or a
283-
# Cloudflare bot-fight 403 at the edge), not wrong credentials.
282+
# message. RomM answers 403 indistinguishably for wrong credentials
283+
# AND for an account that lacks token-creation permission (and a
284+
# Cloudflare bot-fight 403 lands here too), so the message names both
285+
# causes rather than asserting only the permission one.
284286
self._restore_auth_state(snapshot)
285287
return {"success": False, "reason": ErrorCode.AUTH_FAILED.value, "message": _FORBIDDEN_TOKEN_MESSAGE}
286288
except Exception as e:

src/components/settings/ConnectModal.test.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,14 +331,27 @@ describe("ConnectModal", () => {
331331
expect(closeModal).toHaveBeenCalledTimes(1);
332332
});
333333

334-
it("ignores Enter in the token field while it is empty", () => {
334+
it("ignores Enter on an empty token field and consumes the event so the modal cannot close", () => {
335335
const onConnectToken = ok();
336+
const closeModal = vi.fn();
336337
const { getByTestId } = render(
337-
<ConnectModal onConnect={ok()} onConnectToken={onConnectToken} onConnectPairing={ok()} />,
338+
<ConnectModal
339+
closeModal={closeModal}
340+
onConnect={ok()}
341+
onConnectToken={onConnectToken}
342+
onConnectPairing={ok()}
343+
/>,
338344
);
339345
fireEvent.click(getByTestId("mode-token"));
340-
fireEvent.keyDown(getByTestId("field-API Token"), { key: "Enter" });
346+
// fireEvent returns false when a handler called preventDefault. The handler
347+
// must consume the Enter so Steam's ModalRoot cannot fire its own default
348+
// confirm/close on the empty field — the Deck regression. The real close
349+
// can't be reproduced in happy-dom, so a consumed event is the observable
350+
// proxy that the fix is in place, alongside the no-submit/no-close effects.
351+
const notPrevented = fireEvent.keyDown(getByTestId("field-API Token"), { key: "Enter" });
352+
expect(notPrevented).toBe(false);
341353
expect(onConnectToken).not.toHaveBeenCalled();
354+
expect(closeModal).not.toHaveBeenCalled();
342355
});
343356

344357
it("switches back to credentials mode and calls onConnect again", async () => {

src/components/settings/ConnectModal.tsx

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,16 @@ export const ConnectModal: FC<ConnectModalProps> = ({ closeModal, onConnect, onC
238238
focusBoxAtEnd(index - 1);
239239
return;
240240
}
241-
// Enter (the on-screen keyboard's "Eingabe" key) confirms once every box is
242-
// filled — an incomplete code is a no-op. submit() owns closing on success,
243-
// so this path must not close the modal itself.
244-
if (e.key === "Enter" && code.every((c) => c !== "")) {
245-
void submit();
241+
if (e.key === "Enter") {
242+
// Consume the event so Steam's ModalRoot cannot fire its own default
243+
// confirm/close — regardless of whether the code is complete enough to
244+
// submit (an incomplete field must be an inert no-op, not a close).
245+
e.preventDefault();
246+
e.stopPropagation();
247+
// Enter (the on-screen keyboard's "Eingabe" key) confirms once every box
248+
// is filled — an incomplete code is a no-op. submit() owns closing on
249+
// success, so this path must not close the modal itself.
250+
if (code.every((c) => c !== "")) void submit();
246251
}
247252
};
248253

@@ -251,13 +256,24 @@ export const ConnectModal: FC<ConnectModalProps> = ({ closeModal, onConnect, onC
251256
// action. If the password input can't be found, it's a no-op (never a submit).
252257
const handleUsernameKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
253258
if (e.key !== "Enter") return;
259+
// Consume the event so Steam's ModalRoot cannot fire its own default
260+
// confirm/close before focus advances to the password field.
261+
e.preventDefault();
262+
e.stopPropagation();
254263
credentialsRowRef.current?.querySelectorAll("input")[1]?.focus();
255264
};
256265

257266
// Enter on a completing field (password / token) submits, but only when the
258267
// mode's fields are complete — otherwise it's a no-op.
259268
const handleCompletingKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
260-
if (e.key === "Enter" && canSubmit) void submit();
269+
if (e.key !== "Enter") return;
270+
// Consume the event so Steam's ModalRoot cannot fire its own default
271+
// confirm/close on an incomplete field — regardless of whether we submit.
272+
// (On the Deck, R2/OSK-Enter on the empty token field otherwise closed the
273+
// modal even though this handler correctly no-ops when canSubmit is false.)
274+
e.preventDefault();
275+
e.stopPropagation();
276+
if (canSubmit) void submit();
261277
};
262278

263279
const handleModeChange = (option: { data: string }) => {
@@ -301,14 +317,23 @@ export const ConnectModal: FC<ConnectModalProps> = ({ closeModal, onConnect, onC
301317
scopes listed in the plugin docs so downloads, saves, and device sync work. The plugin never deletes a
302318
pasted token; you manage it in RomM.
303319
</div>
304-
<TextField
305-
focusOnMount={true}
306-
label="API Token"
307-
value={token}
308-
bIsPassword
309-
onChange={handleTokenChange}
310-
onKeyDown={handleCompletingKeyDown}
311-
/>
320+
{/* The token field is the only input in this mode and, unwrapped, is a
321+
direct child of the modal body. On the Deck, R2/OSK-Enter on the
322+
empty field closes the ModalRoot even though handleCompletingKeyDown
323+
no-ops (canSubmit false). The pairing boxes' <Focusable> wrapper is
324+
the one structure proven not to close on an incomplete field, so the
325+
token field is wrapped the same way. flow-children is irrelevant for
326+
a single field, so it is omitted. */}
327+
<Focusable>
328+
<TextField
329+
focusOnMount={true}
330+
label="API Token"
331+
value={token}
332+
bIsPassword
333+
onChange={handleTokenChange}
334+
onKeyDown={handleCompletingKeyDown}
335+
/>
336+
</Focusable>
312337
</>
313338
)}
314339
{mode === "pairing" && (

src/components/settings/TextInputModal.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ type AnyProps = Record<string, unknown> & { children?: unknown };
1010
interface CapturedConfirm {
1111
onOK?: (() => void) | undefined;
1212
bDisableBackgroundDismiss?: boolean | undefined;
13+
bOKDisabled?: boolean | undefined;
1314
closeModal?: unknown;
1415
strTitle?: string | undefined;
1516
}
@@ -19,6 +20,7 @@ vi.mock("@decky/ui", () => ({
1920
ConfirmModal: (p: AnyProps & CapturedConfirm) => {
2021
captured.onOK = p.onOK;
2122
captured.bDisableBackgroundDismiss = p.bDisableBackgroundDismiss;
23+
captured.bOKDisabled = p.bOKDisabled;
2224
captured.closeModal = p.closeModal;
2325
captured.strTitle = p.strTitle;
2426
return createElement("div", { "data-testid": "confirm-modal" }, p.children as never);
@@ -97,6 +99,49 @@ describe("TextInputModal", () => {
9799
expect(closeModal).toHaveBeenCalledTimes(1);
98100
});
99101

102+
describe("non-empty requirement", () => {
103+
it("disables OK when the value is empty", () => {
104+
render(<TextInputModal label="x" value="" onSubmit={vi.fn()} />);
105+
expect(captured.bOKDisabled).toBe(true);
106+
});
107+
108+
it("disables OK when the value is whitespace-only", () => {
109+
render(<TextInputModal label="x" value="" onSubmit={vi.fn()} />);
110+
const input = document.querySelector("input") as HTMLInputElement;
111+
fireEvent.change(input, { target: { value: " " } });
112+
expect(captured.bOKDisabled).toBe(true);
113+
});
114+
115+
it("enables OK once the value is non-empty", () => {
116+
render(<TextInputModal label="x" value="" onSubmit={vi.fn()} />);
117+
expect(captured.bOKDisabled).toBe(true);
118+
const input = document.querySelector("input") as HTMLInputElement;
119+
fireEvent.change(input, { target: { value: "x" } });
120+
expect(captured.bOKDisabled).toBe(false);
121+
});
122+
123+
it("ignores Enter on an empty field — no submit, no close", () => {
124+
const onSubmit = vi.fn();
125+
const closeModal = vi.fn();
126+
render(<TextInputModal label="x" value="" closeModal={closeModal} onSubmit={onSubmit} />);
127+
const input = document.querySelector("input") as HTMLInputElement;
128+
fireEvent.keyDown(input, { key: "Enter" });
129+
expect(onSubmit).not.toHaveBeenCalled();
130+
expect(closeModal).not.toHaveBeenCalled();
131+
});
132+
133+
it("ignores Enter on a whitespace-only field — no submit, no close", () => {
134+
const onSubmit = vi.fn();
135+
const closeModal = vi.fn();
136+
render(<TextInputModal label="x" value="" closeModal={closeModal} onSubmit={onSubmit} />);
137+
const input = document.querySelector("input") as HTMLInputElement;
138+
fireEvent.change(input, { target: { value: " " } });
139+
fireEvent.keyDown(input, { key: "Enter" });
140+
expect(onSubmit).not.toHaveBeenCalled();
141+
expect(closeModal).not.toHaveBeenCalled();
142+
});
143+
});
144+
100145
it("does NOT trim — submits whitespace as-is (parent is responsible)", () => {
101146
const onSubmit = vi.fn();
102147
render(<TextInputModal label="x" value="" onSubmit={onSubmit} />);

src/components/settings/TextInputModal.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ export const TextInputModal: FC<TextInputModalProps> = ({
2929
onSubmit,
3030
}) => {
3131
const [value, setValue] = useState(initial);
32+
// Every field this modal edits (SteamGridDB API key, RomM URL, Default Save
33+
// Slot) must be non-empty, so a whitespace-only value never counts as a submit.
34+
const canSubmit = value.trim() !== "";
3235
const handleOK = () => {
3336
if (field) {
3437
pendingEdits[field] = value;
@@ -39,6 +42,7 @@ export const TextInputModal: FC<TextInputModalProps> = ({
3942
<ConfirmModal
4043
{...(closeModal !== undefined ? { closeModal } : {})}
4144
onOK={handleOK}
45+
bOKDisabled={!canSubmit}
4246
strTitle={label}
4347
bDisableBackgroundDismiss={true}
4448
>
@@ -51,8 +55,11 @@ export const TextInputModal: FC<TextInputModalProps> = ({
5155
// Enter (the on-screen keyboard's "Eingabe" key) confirms, same as the OK
5256
// button. ConfirmModal auto-closes on its OK button but not on this manual
5357
// path, so close here too — handleOK is shared so the two never drift.
58+
// Gated on canSubmit so an empty/whitespace field is a no-op on Enter,
59+
// matching the disabled OK button (on the Deck, R2/Enter would otherwise
60+
// confirm an empty value and close the modal).
5461
onKeyDown={(e: KeyboardEvent<HTMLInputElement>) => {
55-
if (e.key === "Enter") {
62+
if (e.key === "Enter" && canSubmit) {
5663
handleOK();
5764
closeModal?.();
5865
}

tests/services/test_connection.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,10 @@ def test_forbidden_mint_returns_actionable_message(self, event_loop, romm_api, l
681681
result = event_loop.run_until_complete(service.establish_token("http://romm.local", "u", "p"))
682682
assert result["success"] is False
683683
assert result["reason"] == "auth_failed"
684-
assert "cannot create API tokens" in result["message"]
684+
# The 403 is indistinguishable between wrong credentials and a missing
685+
# token-creation permission, so the message names both causes.
686+
assert "username and password" in result["message"]
687+
assert "create API tokens" in result["message"]
685688

686689
def test_auth_error_mint_returns_auth_error(self, event_loop, romm_api, logger):
687690
romm_api.mint_client_token.side_effect = RommAuthError("401")

0 commit comments

Comments
 (0)