From 5a6e09bdc658a89cdb4473ce35abe37c7b20de5b Mon Sep 17 00:00:00 2001 From: UtkarshBhardwaj007 Date: Sat, 13 Jun 2026 15:40:55 +0200 Subject: [PATCH] fix(deploy): show mobile signer disabled (not hidden) when logged out When `playground deploy` runs without a logged-in session, the signer picker now renders the phone signer greyed out and unselectable instead of omitting it, so users can see the option exists. The cursor skips disabled options and defaults to the dev signer. The "mobile signing unavailable" notice moved below the options. Adds `disabled` support to the shared `Select` component, with the cursor-navigation helpers lifted into `selectNav.ts` and unit-tested. --- .changeset/disabled-mobile-signer.md | 5 ++ src/commands/decentralize/signerPrompt.ts | 9 ++- src/commands/deploy/DeployScreen.tsx | 18 ++--- src/commands/deploy/signerNotice.test.ts | 12 +++- src/commands/deploy/signerNotice.ts | 17 ++--- src/utils/ui/theme/Select.tsx | 22 ++++-- src/utils/ui/theme/selectNav.test.ts | 84 +++++++++++++++++++++++ src/utils/ui/theme/selectNav.ts | 49 +++++++++++++ 8 files changed, 189 insertions(+), 27 deletions(-) create mode 100644 .changeset/disabled-mobile-signer.md create mode 100644 src/utils/ui/theme/selectNav.test.ts create mode 100644 src/utils/ui/theme/selectNav.ts diff --git a/.changeset/disabled-mobile-signer.md b/.changeset/disabled-mobile-signer.md new file mode 100644 index 00000000..e9cc3bcd --- /dev/null +++ b/.changeset/disabled-mobile-signer.md @@ -0,0 +1,5 @@ +--- +"playground-cli": patch +--- + +When deploying without a logged-in session, the `playground deploy` signer picker now shows the phone signer greyed out and unselectable (instead of hiding it), so users can see the option exists. The "mobile signing unavailable" notice moved below the options. diff --git a/src/commands/decentralize/signerPrompt.ts b/src/commands/decentralize/signerPrompt.ts index 2ff7836a..cf597fbc 100644 --- a/src/commands/decentralize/signerPrompt.ts +++ b/src/commands/decentralize/signerPrompt.ts @@ -19,9 +19,12 @@ import type { SelectOption } from "../../utils/ui/theme/Select.js"; /** * Signer options for the interactive `playground decentralize` picker. The * phone signer leads, matching `playground deploy`. Unlike deploy, the phone - * option stays visible without a session (its hint points the user at - * `playground login`, and selecting it surfaces a login error), so both - * options are always present here. + * option stays selectable without a session (its hint points the user at + * `playground login`, and selecting it surfaces a login error), and the cursor + * defaults to the dev signer (see {@link decentralizeSignerInitialIndex}). + * Deploy instead renders the no-session phone option `disabled` (greyed out, + * cursor skips it); we keep the select-to-error behaviour here because it is + * the established decentralize UX. Both options are always present. */ export function decentralizeSignerOptions(hasSession: boolean): SelectOption[] { return [ diff --git a/src/commands/deploy/DeployScreen.tsx b/src/commands/deploy/DeployScreen.tsx index 87fb8631..c46d2b8d 100644 --- a/src/commands/deploy/DeployScreen.tsx +++ b/src/commands/deploy/DeployScreen.tsx @@ -324,11 +324,6 @@ export function DeployScreen({ {stage.kind === "prompt-signer" && ( - {!hasSession && ( - - {NO_SESSION_NOTICE_BODY} - - )} label="who signs the upload?" @@ -339,10 +334,15 @@ export function DeployScreen({ advance(skipBuild, m); }} /> - {/* Placed below the options (mirroring the phone-approval - notices) so the "no XP" trade-off appears right as the - cursor lands on the dev signer and disappears again when - the user moves back to the phone signer. */} + {/* Both notices sit below the options: the phone signer is + shown disabled above, and this explains why and how to + unlock it. The "no XP" trade-off (logged-in path) appears + right as the cursor lands on the dev signer. */} + {!hasSession && ( + + {NO_SESSION_NOTICE_BODY} + + )} {shouldShowDevNoXpWarning(hasSession, highlightedSigner) && ( {DEV_SIGNER_NO_XP_BODY} diff --git a/src/commands/deploy/signerNotice.test.ts b/src/commands/deploy/signerNotice.test.ts index e6025cec..622ace38 100644 --- a/src/commands/deploy/signerNotice.test.ts +++ b/src/commands/deploy/signerNotice.test.ts @@ -58,11 +58,19 @@ describe("deploySignerOptions", () => { expect(opts.map((o) => o.value)).toEqual(["phone", "dev"]); // The default cursor is index 0, so it lands on the phone signer. expect(opts[0].value).toBe("phone"); + // Both are selectable when logged in. + expect(opts.every((o) => !o.disabled)).toBe(true); }); - it("offers only the dev signer when there is no session", () => { + it("shows the phone signer disabled (not hidden) when there is no session", () => { const opts = deploySignerOptions(false); - expect(opts.map((o) => o.value)).toEqual(["dev"]); + // Both options stay visible so the user sees phone signing exists. + expect(opts.map((o) => o.value)).toEqual(["phone", "dev"]); + const phone = opts.find((o) => o.value === "phone"); + const dev = opts.find((o) => o.value === "dev"); + // The phone signer is greyed out and unselectable; dev stays enabled. + expect(phone?.disabled).toBe(true); + expect(dev?.disabled).toBeFalsy(); }); }); diff --git a/src/commands/deploy/signerNotice.ts b/src/commands/deploy/signerNotice.ts index f6516479..9890b0cc 100644 --- a/src/commands/deploy/signerNotice.ts +++ b/src/commands/deploy/signerNotice.ts @@ -21,8 +21,8 @@ import type { SelectOption } from "../../utils/ui/theme/Select.js"; * * Mobile (phone) signing needs a paired session from `playground login`; without * it the phone path is unavailable, but a dev deploy still works out of the box. - * The interactive picker renders this as a yellow Callout above the signer - * options (mirroring the `playground mod` "Community Code" notice); the headless + * The interactive picker renders this as a yellow Callout below the signer + * options (the phone option itself is shown disabled above); the headless * `--signer phone` path surfaces the same intent as a hard error since there's * no TUI to fall back into. */ @@ -34,7 +34,7 @@ export const NO_SESSION_NOTICE_BODY = "You can continue now with the dev signer. Logging in also lets your deploys earn XP."; /** - * Shown above the signer options when phone signing IS available, so the user + * Shown below the signer options when phone signing IS available, so the user * spots the trade-off before picking the dev signer. The dev signer publishes * from a shared test account, so XP earned for a deploy cannot accrue to the * user; only signing from their own (phone) account does. Rendered as a yellow @@ -56,22 +56,23 @@ export const NO_SESSION_HEADLESS_ERROR = /** * The signer options for the interactive `playground deploy` picker. The phone - * signer leads (and so is the default cursor position) when a session exists; - * without one it isn't offered at all and {@link NO_SESSION_NOTICE_BODY} - * explains how to enable it. + * signer always leads, but without a session it is shown disabled (greyed out, + * unselectable) so users can see the option exists; the cursor then defaults to + * the dev signer and {@link NO_SESSION_NOTICE_BODY} explains how to unlock it. */ export function deploySignerOptions(hasSession: boolean): SelectOption[] { const phone: SelectOption = { value: "phone", label: "your phone signer", - hint: "signs with your own account", + hint: hasSession ? "signs with your own account" : "requires `playground login` first", + disabled: !hasSession, }; const dev: SelectOption = { value: "dev", label: "dev signer", hint: "fast, no phone needed", }; - return hasSession ? [phone, dev] : [dev]; + return [phone, dev]; } /** diff --git a/src/utils/ui/theme/Select.tsx b/src/utils/ui/theme/Select.tsx index 7ba46b80..56312527 100644 --- a/src/utils/ui/theme/Select.tsx +++ b/src/utils/ui/theme/Select.tsx @@ -16,11 +16,14 @@ import { useEffect, useState } from "react"; import { Box, Text, useInput } from "ink"; import { COLOR, GLYPH, LAYOUT } from "./tokens.js"; +import { firstEnabledIndex, nextEnabledIndex } from "./selectNav.js"; export interface SelectOption { value: T; label: string; hint?: string; + /** Greyed out and unselectable; the cursor skips over it. */ + disabled?: boolean; } export interface SelectProps { @@ -41,7 +44,9 @@ export function Select({ onSelect, onHighlight, }: SelectProps) { - const [index, setIndex] = useState(Math.min(Math.max(initialIndex, 0), options.length - 1)); + const [index, setIndex] = useState(() => + firstEnabledIndex(options, Math.min(Math.max(initialIndex, 0), options.length - 1)), + ); useEffect(() => { onHighlight?.(options[index].value); @@ -52,12 +57,14 @@ export function Select({ useInput((_input, key) => { if (key.upArrow || key.leftArrow) { - setIndex((i) => (i - 1 + options.length) % options.length); + setIndex((i) => nextEnabledIndex(options, i, -1)); } if (key.downArrow || key.rightArrow) { - setIndex((i) => (i + 1) % options.length); + setIndex((i) => nextEnabledIndex(options, i, 1)); } - if (key.return) onSelect(options[index].value); + // The cursor never rests on a disabled option, but guard anyway so a + // confirm can't slip through if every option is disabled. + if (key.return && !options[index].disabled) onSelect(options[index].value); }); return ( @@ -67,12 +74,17 @@ export function Select({ {options.map((opt, i) => { const selected = i === index; + const disabled = !!opt.disabled; return ( {selected ? `${GLYPH.cursor} ` : " "} - + {opt.label} {opt.hint && ( diff --git a/src/utils/ui/theme/selectNav.test.ts b/src/utils/ui/theme/selectNav.test.ts new file mode 100644 index 00000000..21abf41e --- /dev/null +++ b/src/utils/ui/theme/selectNav.test.ts @@ -0,0 +1,84 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { describe, it, expect } from "vitest"; +import { firstEnabledIndex, nextEnabledIndex } from "./selectNav.js"; + +// Mirrors the deploy signer picker: phone disabled (logged out), dev enabled. +const PHONE_DISABLED = [{ disabled: true }, {}]; +const ALL_ENABLED = [{}, {}, {}]; +const MIDDLE_DISABLED = [{}, { disabled: true }, {}]; +const TAIL_DISABLED = [{}, { disabled: true }, { disabled: true }]; + +describe("firstEnabledIndex", () => { + it("returns the start index when it is enabled", () => { + expect(firstEnabledIndex(ALL_ENABLED, 0)).toBe(0); + expect(firstEnabledIndex(ALL_ENABLED, 2)).toBe(2); + }); + + it("skips a disabled start to the next enabled option (deploy logged-out case)", () => { + // The deploy picker passes initialIndex 0 with phone disabled; the + // cursor must land on the dev signer at index 1. + expect(firstEnabledIndex(PHONE_DISABLED, 0)).toBe(1); + }); + + it("wraps past a disabled tail back to an enabled head", () => { + expect(firstEnabledIndex(TAIL_DISABLED, 1)).toBe(0); + expect(firstEnabledIndex(TAIL_DISABLED, 2)).toBe(0); + }); + + it("falls back to start when every option is disabled", () => { + expect(firstEnabledIndex([{ disabled: true }, { disabled: true }], 1)).toBe(1); + }); + + it("returns the only index for a single-option list", () => { + expect(firstEnabledIndex([{}], 0)).toBe(0); + expect(firstEnabledIndex([{ disabled: true }], 0)).toBe(0); + }); +}); + +describe("nextEnabledIndex", () => { + it("moves to the adjacent enabled option in each direction", () => { + expect(nextEnabledIndex(ALL_ENABLED, 0, 1)).toBe(1); + expect(nextEnabledIndex(ALL_ENABLED, 1, -1)).toBe(0); + }); + + it("wraps around the ends", () => { + expect(nextEnabledIndex(ALL_ENABLED, 2, 1)).toBe(0); + expect(nextEnabledIndex(ALL_ENABLED, 0, -1)).toBe(2); + }); + + it("skips over a disabled option in both directions", () => { + // index 1 is disabled, so forward from 0 lands on 2, backward from 2 lands on 0. + expect(nextEnabledIndex(MIDDLE_DISABLED, 0, 1)).toBe(2); + expect(nextEnabledIndex(MIDDLE_DISABLED, 2, -1)).toBe(0); + }); + + it("never lands on the disabled phone option (deploy logged-out case)", () => { + // Starting on dev (index 1), both directions wrap back to dev, never phone (0). + expect(nextEnabledIndex(PHONE_DISABLED, 1, 1)).toBe(1); + expect(nextEnabledIndex(PHONE_DISABLED, 1, -1)).toBe(1); + }); + + it("stays put when every other option is disabled", () => { + expect(nextEnabledIndex(TAIL_DISABLED, 0, 1)).toBe(0); + expect(nextEnabledIndex(TAIL_DISABLED, 0, -1)).toBe(0); + }); + + it("stays put for a single-option list", () => { + expect(nextEnabledIndex([{}], 0, 1)).toBe(0); + expect(nextEnabledIndex([{}], 0, -1)).toBe(0); + }); +}); diff --git a/src/utils/ui/theme/selectNav.ts b/src/utils/ui/theme/selectNav.ts new file mode 100644 index 00000000..09bfc6d4 --- /dev/null +++ b/src/utils/ui/theme/selectNav.ts @@ -0,0 +1,49 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * Cursor-navigation helpers for {@link Select}, factored out of the `.tsx` so + * they can be unit-tested without rendering Ink. They operate on the minimal + * `{ disabled?: boolean }` shape so the logic stays decoupled from the full + * `SelectOption` type. + */ + +/** First selectable index at or after `start`, wrapping; falls back to `start` if all are disabled. */ +export function firstEnabledIndex( + options: readonly { disabled?: boolean }[], + start: number, +): number { + const n = options.length; + for (let step = 0; step < n; step++) { + const i = (start + step) % n; + if (!options[i].disabled) return i; + } + return start; +} + +/** Next selectable index from `from` in direction `dir` (+1/-1), wrapping past disabled options. */ +export function nextEnabledIndex( + options: readonly { disabled?: boolean }[], + from: number, + dir: 1 | -1, +): number { + const n = options.length; + let i = from; + for (let step = 0; step < n; step++) { + i = (i + dir + n) % n; + if (!options[i].disabled) return i; + } + return from; +}