Skip to content

fix(tui): drop digit-shortcut UI hints on >10-option PickerMenu lists#767

Merged
kelsonpw merged 5 commits into
mainfrom
salvage/tui-picker-large-list
May 14, 2026
Merged

fix(tui): drop digit-shortcut UI hints on >10-option PickerMenu lists#767
kelsonpw merged 5 commits into
mainfrom
salvage/tui-picker-large-list

Conversation

@kelsonpw
Copy link
Copy Markdown
Member

@kelsonpw kelsonpw commented May 14, 2026

Summary

Salvaged from #731 by cherry-picking its unique commits onto a fresh branch from main. The original PR was stacked on the closed feat/v2-tui-redesign (#696); its git history carried ~35 commits of v2 orchestration scaffolding that couldn't be rebased onto main's parallel orchestration implementation.

Original commits preserved: 76023dc4 0bcef173

Notes:

  • PickerMenu.tsx conflict carried isFlashing / PICKER_FLASH_MS references from the unmerged feat(tui): v3 delight pass — gradient wordmark, picker flash, spinner moods, typewriter, voice #715 delight pass. Resolved by taking main's structure plus the digit-shortcut hint change (showDigit prop, hint row, chrome budget tweak); dropped the flash references.
  • Test file (PickerMenu.large-list.test.tsx) lost its PICKER_FLASH_MS import for the same reason; one follow-up commit substitutes a 5ms advance since main's onSelect is synchronous.

Test plan

  • tsc clean
  • eslint clean
  • pnpm build clean
  • CI will run vitest + Bugbot

Generated with Claude Code


Note

Low Risk
Low risk UI/test change: only affects TUI rendering hints and layout row budgeting, plus relaxes a flaky timer assertion in a screen test.

Overview
PickerMenu no longer shows digit-shortcut UI on large lists. When options.length > 10, the per-row [N] chips are suppressed (in both single and multi pickers) and a muted "Use arrows + Enter to pick" hint is rendered instead, while the underlying digit handler still works for indices 0–9.

Layout accounting was updated so the extra hint row is included in the chrome/visible-row budgeting for constrained single-picker rendering, preventing the hint/scroll indicators from being clipped.

Tests: adds PickerMenu.large-list regression tests for the new behavior and loosens elapsed-time assertions in EventPlanFullScreen tests to tolerate 1s CI timer jitter.

Reviewed by Cursor Bugbot for commit a43d38d. Bugbot is set up for automated code reviews on this repo. Configure here.

@kelsonpw kelsonpw requested a review from a team as a code owner May 14, 2026 00:35
kelsonpw added a commit that referenced this pull request May 14, 2026
…o 30s (#777)

The "refreshes the MCP Authorization header when the token rotates
between attempts" test simulates a 60s cold-start stall plus jittered
backoff (2-30s) inside fake timers — up to ~90s of virtual time per
attempt. The test logic is correct, but the per-test 15s real-time
timeout left no headroom for the scheduler under CI load on Node 20,
producing intermittent "Test timed out in 15000ms" failures across
multiple PRs (#765, #766, #767, #768, #769, #771).

Double the timeout to 30s. No logic change.
kelsonpw and others added 5 commits May 14, 2026 12:11
The PickerMenu primitive consumed digit keystrokes 1-9 (+ 0 → index 9)
and rendered a `[N]` chip next to each row. The handler only covers
the first 10 indices, but the chip was emitted for those first 10
rows regardless of total list length — so on lists with >10 options
(e.g. Amplitude users with 20+ orgs, or projects with many envs)
items 11+ looked visually identical to shortcut-enabled rows but
typing their visible number did nothing, while items 1-10 advertised
shortcuts that worked.

Now:
  - options.length <= 10 → render `[1]`-`[9]`/`[0]` chips as before.
  - options.length  > 10 → drop the chips on EVERY row and show a
    muted "Use arrows + Enter to pick" hint at the bottom of the list.

The digit handler itself is unchanged for indices 0-9 so power users
who learned the shortcut keep it; the UI just stops advertising it
once the affordance would be incomplete.

Audit #4 finding (TUI pickers + primitives). Stacks on #728.
PR A12's new "Use arrows + Enter to pick" footer (rendered when
options.length > DIGIT_SHORTCUT_LIMIT) added a row that wasn't
budgeted. `CONSTRAINED_CHROME_RESERVE_ROWS = 3` covered the
PromptLabel header + 2 scroll indicators, so when both indicators
were visible and the hint rendered, total height became
`availableRows + 1` and the parent's `overflow="hidden"` clipped
the bottom — precisely on the >10-option lists this PR targets.

Add a dynamic `hintRows` (0 or 1) to the chrome reserve so the hint
fits inside the budget on both constrained (AuthScreen org/project
/env pickers) and natural-height paths.
The PICKER_FLASH_MS export came from the unmerged #715 delight pass
(selection-confirmation flash). On main, PickerMenu's `onSelect` is
synchronous so we just advance fake timers by 5ms.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cut tests

After merging main, PickerMenu introduced a 250ms selection-confirmation
flash before onSelect fires. The large-list tests advanced fake time
by only 5ms, so they read the still-null `chosen` slot and failed
with `expected null to be 'v3'`.

Advance by `PICKER_FLASH_MS + 5` so the flash timer drains and
`onSelect` is invoked before the assertion.
…rtions

Same fix as PR #768. The revising-screen elapsed timer ticks every
1s via setInterval; on Node 22 under CI clock pressure the interval
can fire one tick before the test's advanceTimersByTime boundary,
so the rendered "elapsed:" string lands at `1m 29s` instead of
`1m 30s`. Both readings are correct for the test's intent
(verifying tier WIRING, not exact copy), so widen the assertions
to a regex that accepts ±1s.
@kelsonpw kelsonpw force-pushed the salvage/tui-picker-large-list branch from 4e9bb29 to a43d38d Compare May 14, 2026 19:12
@kelsonpw kelsonpw merged commit aee4b1f into main May 14, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant