Skip to content

Commit f3e00d2

Browse files
Merge pull request #379 from paritytech/deploy-signer-disabled-phone
fix(deploy): show mobile signer disabled (not hidden) when logged out
2 parents 5809661 + 5a6e09b commit f3e00d2

8 files changed

Lines changed: 189 additions & 27 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"playground-cli": patch
3+
---
4+
5+
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.

src/commands/decentralize/signerPrompt.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ import type { SelectOption } from "../../utils/ui/theme/Select.js";
1919
/**
2020
* Signer options for the interactive `playground decentralize` picker. The
2121
* phone signer leads, matching `playground deploy`. Unlike deploy, the phone
22-
* option stays visible without a session (its hint points the user at
23-
* `playground login`, and selecting it surfaces a login error), so both
24-
* options are always present here.
22+
* option stays selectable without a session (its hint points the user at
23+
* `playground login`, and selecting it surfaces a login error), and the cursor
24+
* defaults to the dev signer (see {@link decentralizeSignerInitialIndex}).
25+
* Deploy instead renders the no-session phone option `disabled` (greyed out,
26+
* cursor skips it); we keep the select-to-error behaviour here because it is
27+
* the established decentralize UX. Both options are always present.
2528
*/
2629
export function decentralizeSignerOptions(hasSession: boolean): SelectOption<SignerMode>[] {
2730
return [

src/commands/deploy/DeployScreen.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,6 @@ export function DeployScreen({
324324

325325
{stage.kind === "prompt-signer" && (
326326
<Box flexDirection="column">
327-
{!hasSession && (
328-
<Callout tone="warning" title={NO_SESSION_NOTICE_TITLE}>
329-
<Text>{NO_SESSION_NOTICE_BODY}</Text>
330-
</Callout>
331-
)}
332327
<PromptInfo box={SIGNER_HELP} />
333328
<Select<SignerMode>
334329
label="who signs the upload?"
@@ -339,10 +334,15 @@ export function DeployScreen({
339334
advance(skipBuild, m);
340335
}}
341336
/>
342-
{/* Placed below the options (mirroring the phone-approval
343-
notices) so the "no XP" trade-off appears right as the
344-
cursor lands on the dev signer and disappears again when
345-
the user moves back to the phone signer. */}
337+
{/* Both notices sit below the options: the phone signer is
338+
shown disabled above, and this explains why and how to
339+
unlock it. The "no XP" trade-off (logged-in path) appears
340+
right as the cursor lands on the dev signer. */}
341+
{!hasSession && (
342+
<Callout tone="warning" title={NO_SESSION_NOTICE_TITLE}>
343+
<Text>{NO_SESSION_NOTICE_BODY}</Text>
344+
</Callout>
345+
)}
346346
{shouldShowDevNoXpWarning(hasSession, highlightedSigner) && (
347347
<Callout tone="warning" title={DEV_SIGNER_NO_XP_TITLE}>
348348
<Text>{DEV_SIGNER_NO_XP_BODY}</Text>

src/commands/deploy/signerNotice.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,19 @@ describe("deploySignerOptions", () => {
5858
expect(opts.map((o) => o.value)).toEqual(["phone", "dev"]);
5959
// The default cursor is index 0, so it lands on the phone signer.
6060
expect(opts[0].value).toBe("phone");
61+
// Both are selectable when logged in.
62+
expect(opts.every((o) => !o.disabled)).toBe(true);
6163
});
6264

63-
it("offers only the dev signer when there is no session", () => {
65+
it("shows the phone signer disabled (not hidden) when there is no session", () => {
6466
const opts = deploySignerOptions(false);
65-
expect(opts.map((o) => o.value)).toEqual(["dev"]);
67+
// Both options stay visible so the user sees phone signing exists.
68+
expect(opts.map((o) => o.value)).toEqual(["phone", "dev"]);
69+
const phone = opts.find((o) => o.value === "phone");
70+
const dev = opts.find((o) => o.value === "dev");
71+
// The phone signer is greyed out and unselectable; dev stays enabled.
72+
expect(phone?.disabled).toBe(true);
73+
expect(dev?.disabled).toBeFalsy();
6674
});
6775
});
6876

src/commands/deploy/signerNotice.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import type { SelectOption } from "../../utils/ui/theme/Select.js";
2121
*
2222
* Mobile (phone) signing needs a paired session from `playground login`; without
2323
* it the phone path is unavailable, but a dev deploy still works out of the box.
24-
* The interactive picker renders this as a yellow Callout above the signer
25-
* options (mirroring the `playground mod` "Community Code" notice); the headless
24+
* The interactive picker renders this as a yellow Callout below the signer
25+
* options (the phone option itself is shown disabled above); the headless
2626
* `--signer phone` path surfaces the same intent as a hard error since there's
2727
* no TUI to fall back into.
2828
*/
@@ -34,7 +34,7 @@ export const NO_SESSION_NOTICE_BODY =
3434
"You can continue now with the dev signer. Logging in also lets your deploys earn XP.";
3535

3636
/**
37-
* Shown above the signer options when phone signing IS available, so the user
37+
* Shown below the signer options when phone signing IS available, so the user
3838
* spots the trade-off before picking the dev signer. The dev signer publishes
3939
* from a shared test account, so XP earned for a deploy cannot accrue to the
4040
* user; only signing from their own (phone) account does. Rendered as a yellow
@@ -56,22 +56,23 @@ export const NO_SESSION_HEADLESS_ERROR =
5656

5757
/**
5858
* The signer options for the interactive `playground deploy` picker. The phone
59-
* signer leads (and so is the default cursor position) when a session exists;
60-
* without one it isn't offered at all and {@link NO_SESSION_NOTICE_BODY}
61-
* explains how to enable it.
59+
* signer always leads, but without a session it is shown disabled (greyed out,
60+
* unselectable) so users can see the option exists; the cursor then defaults to
61+
* the dev signer and {@link NO_SESSION_NOTICE_BODY} explains how to unlock it.
6262
*/
6363
export function deploySignerOptions(hasSession: boolean): SelectOption<SignerMode>[] {
6464
const phone: SelectOption<SignerMode> = {
6565
value: "phone",
6666
label: "your phone signer",
67-
hint: "signs with your own account",
67+
hint: hasSession ? "signs with your own account" : "requires `playground login` first",
68+
disabled: !hasSession,
6869
};
6970
const dev: SelectOption<SignerMode> = {
7071
value: "dev",
7172
label: "dev signer",
7273
hint: "fast, no phone needed",
7374
};
74-
return hasSession ? [phone, dev] : [dev];
75+
return [phone, dev];
7576
}
7677

7778
/**

src/utils/ui/theme/Select.tsx

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@
1616
import { useEffect, useState } from "react";
1717
import { Box, Text, useInput } from "ink";
1818
import { COLOR, GLYPH, LAYOUT } from "./tokens.js";
19+
import { firstEnabledIndex, nextEnabledIndex } from "./selectNav.js";
1920

2021
export interface SelectOption<T> {
2122
value: T;
2223
label: string;
2324
hint?: string;
25+
/** Greyed out and unselectable; the cursor skips over it. */
26+
disabled?: boolean;
2427
}
2528

2629
export interface SelectProps<T> {
@@ -41,7 +44,9 @@ export function Select<T>({
4144
onSelect,
4245
onHighlight,
4346
}: SelectProps<T>) {
44-
const [index, setIndex] = useState(Math.min(Math.max(initialIndex, 0), options.length - 1));
47+
const [index, setIndex] = useState(() =>
48+
firstEnabledIndex(options, Math.min(Math.max(initialIndex, 0), options.length - 1)),
49+
);
4550

4651
useEffect(() => {
4752
onHighlight?.(options[index].value);
@@ -52,12 +57,14 @@ export function Select<T>({
5257

5358
useInput((_input, key) => {
5459
if (key.upArrow || key.leftArrow) {
55-
setIndex((i) => (i - 1 + options.length) % options.length);
60+
setIndex((i) => nextEnabledIndex(options, i, -1));
5661
}
5762
if (key.downArrow || key.rightArrow) {
58-
setIndex((i) => (i + 1) % options.length);
63+
setIndex((i) => nextEnabledIndex(options, i, 1));
5964
}
60-
if (key.return) onSelect(options[index].value);
65+
// The cursor never rests on a disabled option, but guard anyway so a
66+
// confirm can't slip through if every option is disabled.
67+
if (key.return && !options[index].disabled) onSelect(options[index].value);
6168
});
6269

6370
return (
@@ -67,12 +74,17 @@ export function Select<T>({
6774
</Box>
6875
{options.map((opt, i) => {
6976
const selected = i === index;
77+
const disabled = !!opt.disabled;
7078
return (
7179
<Box key={i} flexDirection="row">
7280
<Text color={selected ? COLOR.accent : undefined}>
7381
{selected ? `${GLYPH.cursor} ` : " "}
7482
</Text>
75-
<Text color={selected ? COLOR.accent : undefined} bold={selected}>
83+
<Text
84+
color={selected ? COLOR.accent : undefined}
85+
bold={selected}
86+
dimColor={disabled}
87+
>
7688
{opt.label}
7789
</Text>
7890
{opt.hint && (
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (C) Parity Technologies (UK) Ltd.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
import { describe, it, expect } from "vitest";
17+
import { firstEnabledIndex, nextEnabledIndex } from "./selectNav.js";
18+
19+
// Mirrors the deploy signer picker: phone disabled (logged out), dev enabled.
20+
const PHONE_DISABLED = [{ disabled: true }, {}];
21+
const ALL_ENABLED = [{}, {}, {}];
22+
const MIDDLE_DISABLED = [{}, { disabled: true }, {}];
23+
const TAIL_DISABLED = [{}, { disabled: true }, { disabled: true }];
24+
25+
describe("firstEnabledIndex", () => {
26+
it("returns the start index when it is enabled", () => {
27+
expect(firstEnabledIndex(ALL_ENABLED, 0)).toBe(0);
28+
expect(firstEnabledIndex(ALL_ENABLED, 2)).toBe(2);
29+
});
30+
31+
it("skips a disabled start to the next enabled option (deploy logged-out case)", () => {
32+
// The deploy picker passes initialIndex 0 with phone disabled; the
33+
// cursor must land on the dev signer at index 1.
34+
expect(firstEnabledIndex(PHONE_DISABLED, 0)).toBe(1);
35+
});
36+
37+
it("wraps past a disabled tail back to an enabled head", () => {
38+
expect(firstEnabledIndex(TAIL_DISABLED, 1)).toBe(0);
39+
expect(firstEnabledIndex(TAIL_DISABLED, 2)).toBe(0);
40+
});
41+
42+
it("falls back to start when every option is disabled", () => {
43+
expect(firstEnabledIndex([{ disabled: true }, { disabled: true }], 1)).toBe(1);
44+
});
45+
46+
it("returns the only index for a single-option list", () => {
47+
expect(firstEnabledIndex([{}], 0)).toBe(0);
48+
expect(firstEnabledIndex([{ disabled: true }], 0)).toBe(0);
49+
});
50+
});
51+
52+
describe("nextEnabledIndex", () => {
53+
it("moves to the adjacent enabled option in each direction", () => {
54+
expect(nextEnabledIndex(ALL_ENABLED, 0, 1)).toBe(1);
55+
expect(nextEnabledIndex(ALL_ENABLED, 1, -1)).toBe(0);
56+
});
57+
58+
it("wraps around the ends", () => {
59+
expect(nextEnabledIndex(ALL_ENABLED, 2, 1)).toBe(0);
60+
expect(nextEnabledIndex(ALL_ENABLED, 0, -1)).toBe(2);
61+
});
62+
63+
it("skips over a disabled option in both directions", () => {
64+
// index 1 is disabled, so forward from 0 lands on 2, backward from 2 lands on 0.
65+
expect(nextEnabledIndex(MIDDLE_DISABLED, 0, 1)).toBe(2);
66+
expect(nextEnabledIndex(MIDDLE_DISABLED, 2, -1)).toBe(0);
67+
});
68+
69+
it("never lands on the disabled phone option (deploy logged-out case)", () => {
70+
// Starting on dev (index 1), both directions wrap back to dev, never phone (0).
71+
expect(nextEnabledIndex(PHONE_DISABLED, 1, 1)).toBe(1);
72+
expect(nextEnabledIndex(PHONE_DISABLED, 1, -1)).toBe(1);
73+
});
74+
75+
it("stays put when every other option is disabled", () => {
76+
expect(nextEnabledIndex(TAIL_DISABLED, 0, 1)).toBe(0);
77+
expect(nextEnabledIndex(TAIL_DISABLED, 0, -1)).toBe(0);
78+
});
79+
80+
it("stays put for a single-option list", () => {
81+
expect(nextEnabledIndex([{}], 0, 1)).toBe(0);
82+
expect(nextEnabledIndex([{}], 0, -1)).toBe(0);
83+
});
84+
});

src/utils/ui/theme/selectNav.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright (C) Parity Technologies (UK) Ltd.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
/**
17+
* Cursor-navigation helpers for {@link Select}, factored out of the `.tsx` so
18+
* they can be unit-tested without rendering Ink. They operate on the minimal
19+
* `{ disabled?: boolean }` shape so the logic stays decoupled from the full
20+
* `SelectOption` type.
21+
*/
22+
23+
/** First selectable index at or after `start`, wrapping; falls back to `start` if all are disabled. */
24+
export function firstEnabledIndex(
25+
options: readonly { disabled?: boolean }[],
26+
start: number,
27+
): number {
28+
const n = options.length;
29+
for (let step = 0; step < n; step++) {
30+
const i = (start + step) % n;
31+
if (!options[i].disabled) return i;
32+
}
33+
return start;
34+
}
35+
36+
/** Next selectable index from `from` in direction `dir` (+1/-1), wrapping past disabled options. */
37+
export function nextEnabledIndex(
38+
options: readonly { disabled?: boolean }[],
39+
from: number,
40+
dir: 1 | -1,
41+
): number {
42+
const n = options.length;
43+
let i = from;
44+
for (let step = 0; step < n; step++) {
45+
i = (i + dir + n) % n;
46+
if (!options[i].disabled) return i;
47+
}
48+
return from;
49+
}

0 commit comments

Comments
 (0)