TUI testing fixes: discovery picker + focus/navigation UX#5
Merged
Conversation
Live TUI testing of the per-repo discovery picker surfaced two bugs: - Skill labels were invisible: the scroll FrameView's content size was set to width 1 (`new Size(1, count)`), clipping every checkbox to its glyph. Compute the content width from the (bounded) labels so names and truncated descriptions render; long descriptions are capped. - The "N/M selected" count never updated and Space/A/N appeared dead: the checkbox change event is `ValueChanged` in this Terminal.Gui version (an earlier wrong guess was dropped to get it compiling), so nothing recomputed the count. Subscribe each box's ValueChanged to RefreshValidity. Verified live: Space → 112/113, A → 113/113, N → "select at least one skill" with Install disabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the live TUI testing pass. All verified by driving the real binary in a pseudo-terminal. - Esc no longer quits the app at root. Terminal.Gui's default quit-on-Esc exited with no confirmation; swallow Esc in the window shortcut handler (modals/Doctor/Updates/search field still handle it themselves) and hint "Press q to quit". - Installed tab now focuses the skill list on activation/startup, not the filter field. Previously the filter held focus and swallowed advertised global hotkeys (?, d, x). A deferred FocusList() beats Terminal.Gui's post-reflow default. - Esc on a non-empty Installed filter clears it (the list is live-filtered, so stray text left it stuck at "no matches"); an empty filter still falls through to the normal Back shortcut. - Strip gh's "[namespace] " tag from listed skill names. gh emits "[root] name<TAB>desc"; the bracket is display decoration, so the picker showed "[root] name" and per-name subset installs would have passed an unresolvable argument. Root → bare name, others → ns/name. - Discovery picker no longer leaves a stale "discovering skills in …" status after the picker is cancelled. Tests: +8 (NormalizeSkillName cases, root-namespace parse). 521 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
harder
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes found by driving the real TUI in a pseudo-terminal (tmux) to test all workflows after #4 merged. Two commits; every fix verified live.
Commit 1 — discovery picker rendering
1, clipping every row to the☑glyph. Now sized from the labels.ValueChanged. Verified:Space→112/113,A→113/113,N→select at least one skill(Install disabled).Commit 2 — focus / navigation / discovery UX
Escquit the app at root with no confirmation (Terminal.Gui's default). Now swallowed at the top-level list with a "Press q to quit" hint; modals/Doctor/Updates/search keep their own Esc.?,d,x) were typed into it. Now focuses the skill list on activation/startup (deferredFocusList()beats TG's post-reflow default). Verified:?opens Help at startup.Escleft stray filter text (list stuck at "no matches"). Esc on a non-empty Installed filter now clears it; empty-filter Esc still does Back.[root]namespace tag in names. gh lists[root] name<TAB>desc; the tag is decoration. Stripping it fixes the picker label and per-name subset installs (root → bare name, elsens/name).Testing
Apicker,Escat root,?at startup,f/Esc filter clear, picker labels/toggles).521unit tests pass (+18 across both commits).--allpath (all selected) is the common case.🤖 Generated with Claude Code