mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
refactor(tui): clarify searchable select list width layout (#16378)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: fecbade822
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Reviewed-by: @steipete
This commit is contained in:
committed by
GitHub
parent
f19eabee54
commit
4133f4bd37
@@ -39,6 +39,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/BlueBubbles: require explicit `mediaLocalRoots` allowlists for local outbound media path reads to prevent local file disclosure. (#16322) Thanks @mbelinky.
|
||||
- Cron/Slack: preserve agent identity (name and icon) when cron jobs deliver outbound messages. (#16242) Thanks @robbyczgw-cla.
|
||||
- Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow.
|
||||
- TUI: refactor searchable select list description layout and add regression coverage for ANSI-highlight width bounds.
|
||||
|
||||
## 2026.2.14
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import { type AddressInfo, createServer } from "node:net";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, vi } from "vitest";
|
||||
import type { MockFn } from "../test-utils/vitest-mock-fn.js";
|
||||
|
||||
type HarnessState = {
|
||||
testPort: number;
|
||||
@@ -22,23 +23,23 @@ const state: HarnessState = {
|
||||
prevGatewayPort: undefined,
|
||||
};
|
||||
|
||||
export function getBrowserControlServerTestState() {
|
||||
export function getBrowserControlServerTestState(): HarnessState {
|
||||
return state;
|
||||
}
|
||||
|
||||
export function getBrowserControlServerBaseUrl() {
|
||||
export function getBrowserControlServerBaseUrl(): string {
|
||||
return `http://127.0.0.1:${state.testPort}`;
|
||||
}
|
||||
|
||||
export function setBrowserControlServerCreateTargetId(targetId: string | null) {
|
||||
export function setBrowserControlServerCreateTargetId(targetId: string | null): void {
|
||||
state.createTargetId = targetId;
|
||||
}
|
||||
|
||||
export function setBrowserControlServerAttachOnly(attachOnly: boolean) {
|
||||
export function setBrowserControlServerAttachOnly(attachOnly: boolean): void {
|
||||
state.cfgAttachOnly = attachOnly;
|
||||
}
|
||||
|
||||
export function setBrowserControlServerReachable(reachable: boolean) {
|
||||
export function setBrowserControlServerReachable(reachable: boolean): void {
|
||||
state.reachable = reachable;
|
||||
}
|
||||
|
||||
@@ -51,8 +52,8 @@ const cdpMocks = vi.hoisted(() => ({
|
||||
})),
|
||||
}));
|
||||
|
||||
export function getCdpMocks() {
|
||||
return cdpMocks;
|
||||
export function getCdpMocks(): { createTargetViaCdp: MockFn; snapshotAria: MockFn } {
|
||||
return cdpMocks as unknown as { createTargetViaCdp: MockFn; snapshotAria: MockFn };
|
||||
}
|
||||
|
||||
const pwMocks = vi.hoisted(() => ({
|
||||
@@ -97,8 +98,8 @@ const pwMocks = vi.hoisted(() => ({
|
||||
waitForViaPlaywright: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
export function getPwMocks() {
|
||||
return pwMocks;
|
||||
export function getPwMocks(): Record<string, MockFn> {
|
||||
return pwMocks as unknown as Record<string, MockFn>;
|
||||
}
|
||||
|
||||
const chromeUserDataDir = vi.hoisted(() => ({ dir: "/tmp/openclaw" }));
|
||||
|
||||
@@ -2,6 +2,7 @@ import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, vi } from "vitest";
|
||||
import type { MockFn } from "../test-utils/vitest-mock-fn.js";
|
||||
|
||||
let originalIsTTY: boolean | undefined;
|
||||
let originalStateDir: string | undefined;
|
||||
@@ -19,40 +20,40 @@ function setStdinTty(value: boolean | undefined) {
|
||||
}
|
||||
}
|
||||
|
||||
export const readConfigFileSnapshot: ReturnType<typeof vi.fn> = vi.fn();
|
||||
export const confirm: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(true);
|
||||
export const select: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue("node");
|
||||
export const note: ReturnType<typeof vi.fn> = vi.fn();
|
||||
export const writeConfigFile: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(undefined);
|
||||
export const resolveOpenClawPackageRoot: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(null);
|
||||
export const runGatewayUpdate: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue({
|
||||
export const readConfigFileSnapshot = vi.fn() as unknown as MockFn;
|
||||
export const confirm = vi.fn().mockResolvedValue(true) as unknown as MockFn;
|
||||
export const select = vi.fn().mockResolvedValue("node") as unknown as MockFn;
|
||||
export const note = vi.fn() as unknown as MockFn;
|
||||
export const writeConfigFile = vi.fn().mockResolvedValue(undefined) as unknown as MockFn;
|
||||
export const resolveOpenClawPackageRoot = vi.fn().mockResolvedValue(null) as unknown as MockFn;
|
||||
export const runGatewayUpdate = vi.fn().mockResolvedValue({
|
||||
status: "skipped",
|
||||
mode: "unknown",
|
||||
steps: [],
|
||||
durationMs: 0,
|
||||
});
|
||||
export const migrateLegacyConfig: ReturnType<typeof vi.fn> = vi.fn((raw: unknown) => ({
|
||||
}) as unknown as MockFn;
|
||||
export const migrateLegacyConfig = vi.fn((raw: unknown) => ({
|
||||
config: raw as Record<string, unknown>,
|
||||
changes: ["Moved routing.allowFrom → channels.whatsapp.allowFrom."],
|
||||
}));
|
||||
})) as unknown as MockFn;
|
||||
|
||||
export const runExec: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue({
|
||||
export const runExec = vi.fn().mockResolvedValue({
|
||||
stdout: "",
|
||||
stderr: "",
|
||||
});
|
||||
export const runCommandWithTimeout: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue({
|
||||
}) as unknown as MockFn;
|
||||
export const runCommandWithTimeout = vi.fn().mockResolvedValue({
|
||||
stdout: "",
|
||||
stderr: "",
|
||||
code: 0,
|
||||
signal: null,
|
||||
killed: false,
|
||||
});
|
||||
}) as unknown as MockFn;
|
||||
|
||||
export const ensureAuthProfileStore: ReturnType<typeof vi.fn> = vi
|
||||
export const ensureAuthProfileStore = vi
|
||||
.fn()
|
||||
.mockReturnValue({ version: 1, profiles: {} });
|
||||
.mockReturnValue({ version: 1, profiles: {} }) as unknown as MockFn;
|
||||
|
||||
export const legacyReadConfigFileSnapshot: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue({
|
||||
export const legacyReadConfigFileSnapshot = vi.fn().mockResolvedValue({
|
||||
path: "/tmp/openclaw.json",
|
||||
exists: false,
|
||||
raw: null,
|
||||
@@ -61,30 +62,28 @@ export const legacyReadConfigFileSnapshot: ReturnType<typeof vi.fn> = vi.fn().mo
|
||||
config: {},
|
||||
issues: [],
|
||||
legacyIssues: [],
|
||||
});
|
||||
export const createConfigIO: ReturnType<typeof vi.fn> = vi.fn(() => ({
|
||||
}) as unknown as MockFn;
|
||||
export const createConfigIO = vi.fn(() => ({
|
||||
readConfigFileSnapshot: legacyReadConfigFileSnapshot,
|
||||
}));
|
||||
})) as unknown as MockFn;
|
||||
|
||||
export const findLegacyGatewayServices: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue([]);
|
||||
export const uninstallLegacyGatewayServices: ReturnType<typeof vi.fn> = vi
|
||||
export const findLegacyGatewayServices = vi.fn().mockResolvedValue([]) as unknown as MockFn;
|
||||
export const uninstallLegacyGatewayServices = vi.fn().mockResolvedValue([]) as unknown as MockFn;
|
||||
export const findExtraGatewayServices = vi.fn().mockResolvedValue([]) as unknown as MockFn;
|
||||
export const renderGatewayServiceCleanupHints = vi
|
||||
.fn()
|
||||
.mockResolvedValue([]);
|
||||
export const findExtraGatewayServices: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue([]);
|
||||
export const renderGatewayServiceCleanupHints: ReturnType<typeof vi.fn> = vi
|
||||
.fn()
|
||||
.mockReturnValue(["cleanup"]);
|
||||
export const resolveGatewayProgramArguments: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue({
|
||||
.mockReturnValue(["cleanup"]) as unknown as MockFn;
|
||||
export const resolveGatewayProgramArguments = vi.fn().mockResolvedValue({
|
||||
programArguments: ["node", "cli", "gateway", "--port", "18789"],
|
||||
});
|
||||
export const serviceInstall: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(undefined);
|
||||
export const serviceIsLoaded: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(false);
|
||||
export const serviceStop: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(undefined);
|
||||
export const serviceRestart: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(undefined);
|
||||
export const serviceUninstall: ReturnType<typeof vi.fn> = vi.fn().mockResolvedValue(undefined);
|
||||
export const callGateway: ReturnType<typeof vi.fn> = vi
|
||||
}) as unknown as MockFn;
|
||||
export const serviceInstall = vi.fn().mockResolvedValue(undefined) as unknown as MockFn;
|
||||
export const serviceIsLoaded = vi.fn().mockResolvedValue(false) as unknown as MockFn;
|
||||
export const serviceStop = vi.fn().mockResolvedValue(undefined) as unknown as MockFn;
|
||||
export const serviceRestart = vi.fn().mockResolvedValue(undefined) as unknown as MockFn;
|
||||
export const serviceUninstall = vi.fn().mockResolvedValue(undefined) as unknown as MockFn;
|
||||
export const callGateway = vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error("gateway closed"));
|
||||
.mockRejectedValue(new Error("gateway closed")) as unknown as MockFn;
|
||||
|
||||
vi.mock("@clack/prompts", () => ({
|
||||
confirm,
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { beforeEach, vi, type Mock } from "vitest";
|
||||
import { beforeEach, vi } from "vitest";
|
||||
import type { MockFn } from "../test-utils/vitest-mock-fn.js";
|
||||
import { resetInboundDedupe } from "../auto-reply/reply/inbound-dedupe.js";
|
||||
|
||||
type AnyMock = Mock<(...args: unknown[]) => unknown>;
|
||||
type AnyAsyncMock = Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
type AnyMock = MockFn<(...args: unknown[]) => unknown>;
|
||||
type AnyAsyncMock = MockFn<(...args: unknown[]) => Promise<unknown>>;
|
||||
|
||||
type ReplyOpts =
|
||||
| {
|
||||
onReplyStart?: () => void | Promise<void>;
|
||||
@@ -74,12 +76,12 @@ vi.mock("../pairing/pairing-store.js", () => ({
|
||||
upsertChannelPairingRequest,
|
||||
}));
|
||||
|
||||
export const useSpy: Mock<(arg: unknown) => void> = vi.fn();
|
||||
export const middlewareUseSpy: Mock<(...args: unknown[]) => unknown> = vi.fn();
|
||||
export const onSpy: Mock<(...args: unknown[]) => unknown> = vi.fn();
|
||||
export const stopSpy: Mock<(...args: unknown[]) => unknown> = vi.fn();
|
||||
export const commandSpy: Mock<(...args: unknown[]) => unknown> = vi.fn();
|
||||
export const botCtorSpy: Mock<(...args: unknown[]) => unknown> = vi.fn();
|
||||
export const useSpy: MockFn<(arg: unknown) => void> = vi.fn();
|
||||
export const middlewareUseSpy: AnyMock = vi.fn();
|
||||
export const onSpy: AnyMock = vi.fn();
|
||||
export const stopSpy: AnyMock = vi.fn();
|
||||
export const commandSpy: AnyMock = vi.fn();
|
||||
export const botCtorSpy: AnyMock = vi.fn();
|
||||
export const answerCallbackQuerySpy: AnyAsyncMock = vi.fn(async () => undefined);
|
||||
export const sendChatActionSpy: AnyMock = vi.fn();
|
||||
export const setMessageReactionSpy: AnyAsyncMock = vi.fn(async () => undefined);
|
||||
@@ -154,7 +156,7 @@ vi.mock("@grammyjs/transformer-throttler", () => ({
|
||||
apiThrottler: () => throttlerSpy(),
|
||||
}));
|
||||
|
||||
export const replySpy: Mock<(ctx: unknown, opts?: ReplyOpts) => Promise<void>> = vi.fn(
|
||||
export const replySpy: MockFn<(ctx: unknown, opts?: ReplyOpts) => Promise<void>> = vi.fn(
|
||||
async (_ctx, opts) => {
|
||||
await opts?.onReplyStart?.();
|
||||
return undefined;
|
||||
|
||||
6
src/test-utils/vitest-mock-fn.ts
Normal file
6
src/test-utils/vitest-mock-fn.ts
Normal file
@@ -0,0 +1,6 @@
|
||||
// Centralized Vitest mock type for harness modules under `src/`.
|
||||
// Using an explicit named type avoids exporting inferred `vi.fn()` types that can trip TS2742.
|
||||
//
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
export type MockFn<T extends (...args: any[]) => any = (...args: any[]) => any> =
|
||||
import("vitest").Mock<T>;
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { visibleWidth } from "../../terminal/ansi.js";
|
||||
import { SearchableSelectList, type SearchableSelectListTheme } from "./searchable-select-list.js";
|
||||
|
||||
const mockTheme: SearchableSelectListTheme = {
|
||||
@@ -48,6 +49,60 @@ describe("SearchableSelectList", () => {
|
||||
expect(output).toContain(tail);
|
||||
});
|
||||
|
||||
it("does not show description layout at width 40 (boundary)", () => {
|
||||
const items = [
|
||||
{ value: "one", label: "one", description: "desc" },
|
||||
{ value: "two", label: "two", description: "desc" },
|
||||
];
|
||||
const list = new SearchableSelectList(items, 5, mockTheme);
|
||||
list.setSelectedIndex(1); // ensure first row is not selected so description styling is applied
|
||||
|
||||
const output = list.render(40).join("\n");
|
||||
expect(output).not.toContain("(desc)");
|
||||
});
|
||||
|
||||
it("shows description layout at width 41 (boundary)", () => {
|
||||
const items = [
|
||||
{ value: "one", label: "one", description: "desc" },
|
||||
{ value: "two", label: "two", description: "desc" },
|
||||
];
|
||||
const list = new SearchableSelectList(items, 5, mockTheme);
|
||||
list.setSelectedIndex(1); // ensure first row is not selected so description styling is applied
|
||||
|
||||
const output = list.render(41).join("\n");
|
||||
expect(output).toContain("(desc)");
|
||||
});
|
||||
|
||||
it("keeps ANSI-highlighted description rows within terminal width", () => {
|
||||
const ansiTheme: SearchableSelectListTheme = {
|
||||
selectedPrefix: (t) => t,
|
||||
selectedText: (t) => t,
|
||||
description: (t) => t,
|
||||
scrollInfo: (t) => t,
|
||||
noMatch: (t) => t,
|
||||
searchPrompt: (t) => t,
|
||||
searchInput: (t) => t,
|
||||
matchHighlight: (t) => `\u001b[31m${t}\u001b[0m`,
|
||||
};
|
||||
const label = `provider/${"x".repeat(80)}`;
|
||||
const items = [
|
||||
{ value: label, label, description: "Some description text that should not overflow" },
|
||||
{ value: "other", label: "other", description: "Other description" },
|
||||
];
|
||||
const list = new SearchableSelectList(items, 5, ansiTheme);
|
||||
list.setSelectedIndex(1); // make first row non-selected so description styling is applied
|
||||
|
||||
for (const ch of "provider") {
|
||||
list.handleInput(ch);
|
||||
}
|
||||
|
||||
const width = 80;
|
||||
const output = list.render(width);
|
||||
for (const line of output) {
|
||||
expect(visibleWidth(line)).toBeLessThanOrEqual(width);
|
||||
}
|
||||
});
|
||||
|
||||
it("filters items when typing", () => {
|
||||
const list = new SearchableSelectList(testItems, 5, mockTheme);
|
||||
|
||||
|
||||
@@ -33,6 +33,12 @@ export class SearchableSelectList implements Component {
|
||||
onCancel?: () => void;
|
||||
onSelectionChange?: (item: SelectItem) => void;
|
||||
|
||||
private static readonly DESCRIPTION_LAYOUT_MIN_WIDTH = 40;
|
||||
private static readonly DESCRIPTION_MIN_WIDTH = 12;
|
||||
private static readonly DESCRIPTION_SPACING_WIDTH = 2;
|
||||
// Keep a small right margin so we don't risk wrapping due to styling/terminal quirks.
|
||||
private static readonly RIGHT_MARGIN_WIDTH = 2;
|
||||
|
||||
constructor(items: SelectItem[], maxVisible: number, theme: SearchableSelectListTheme) {
|
||||
this.items = items;
|
||||
this.filteredItems = items;
|
||||
@@ -218,23 +224,20 @@ export class SearchableSelectList implements Component {
|
||||
const prefixWidth = prefix.length;
|
||||
const displayValue = this.getItemLabel(item);
|
||||
|
||||
if (item.description && width > 40) {
|
||||
const minDescriptionWidth = 12;
|
||||
const spacingWidth = 2;
|
||||
const availableWidth = Math.max(1, width - prefixWidth - 2);
|
||||
|
||||
if (availableWidth > minDescriptionWidth + spacingWidth + 1) {
|
||||
const maxValueWidth = availableWidth - minDescriptionWidth - spacingWidth;
|
||||
const truncatedValue = truncateToWidth(displayValue, maxValueWidth, "");
|
||||
const description = item.description;
|
||||
if (description) {
|
||||
const descriptionLayout = this.getDescriptionLayout(width, prefixWidth);
|
||||
if (descriptionLayout) {
|
||||
const truncatedValue = truncateToWidth(displayValue, descriptionLayout.maxValueWidth, "");
|
||||
const valueText = this.highlightMatch(truncatedValue, query);
|
||||
|
||||
const usedByValue = visibleWidth(valueText);
|
||||
const remainingWidth = availableWidth - usedByValue;
|
||||
const remainingWidth = descriptionLayout.availableWidth - usedByValue;
|
||||
const descriptionWidth = remainingWidth - descriptionLayout.spacingWidth;
|
||||
|
||||
if (remainingWidth > spacingWidth + 1) {
|
||||
const descriptionWidth = remainingWidth - spacingWidth;
|
||||
const spacing = " ".repeat(spacingWidth);
|
||||
const truncatedDesc = truncateToWidth(item.description, descriptionWidth, "");
|
||||
if (descriptionWidth >= SearchableSelectList.DESCRIPTION_MIN_WIDTH) {
|
||||
const spacing = " ".repeat(descriptionLayout.spacingWidth);
|
||||
const truncatedDesc = truncateToWidth(description, descriptionWidth, "");
|
||||
// Highlight plain text first, then apply theme styling to avoid corrupting ANSI codes
|
||||
const highlightedDesc = this.highlightMatch(truncatedDesc, query);
|
||||
const descText = isSelected ? highlightedDesc : this.theme.description(highlightedDesc);
|
||||
@@ -251,6 +254,34 @@ export class SearchableSelectList implements Component {
|
||||
return isSelected ? this.theme.selectedText(line) : line;
|
||||
}
|
||||
|
||||
private getDescriptionLayout(
|
||||
width: number,
|
||||
prefixWidth: number,
|
||||
): { availableWidth: number; maxValueWidth: number; spacingWidth: number } | null {
|
||||
if (width <= SearchableSelectList.DESCRIPTION_LAYOUT_MIN_WIDTH) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const availableWidth = Math.max(
|
||||
1,
|
||||
width - prefixWidth - SearchableSelectList.RIGHT_MARGIN_WIDTH,
|
||||
);
|
||||
const maxValueWidth =
|
||||
availableWidth -
|
||||
SearchableSelectList.DESCRIPTION_MIN_WIDTH -
|
||||
SearchableSelectList.DESCRIPTION_SPACING_WIDTH;
|
||||
|
||||
if (maxValueWidth < 1) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
availableWidth,
|
||||
maxValueWidth,
|
||||
spacingWidth: SearchableSelectList.DESCRIPTION_SPACING_WIDTH,
|
||||
};
|
||||
}
|
||||
|
||||
handleInput(keyData: string): void {
|
||||
if (isKeyRelease(keyData)) {
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user