[RAPTOR-19762] feat(workload): highlight the selected picker row with a yellow bar - #857
Conversation
|
🎫 Jira: |
|
Wonder how this handles light mode vs dark mode. cc @cdevent |
…in-use base image Two related fixes to the dr workload up / config wizard's selectors. Selection is now one yellow everywhere it appears — menu options, the advanced-options row, and the list-picker rows — so a selection reads the same across every list the wizard offers. The pickers could not show a colour before: bubbles/table colours each cell first, and those cells' ANSI resets truncate any style wrapped around the finished row. The table's cells are now left uncoloured so the cursor row's colour shows; non-selected rows go neutral, the usual look for a selection list. The base-image picker now marks the environment a bound workload is already built on with a green '· in use' tag and lifts it to the top, matching the menus (the list runs long, so the likely choice should not be scrolled for). The tag trails on the last cell, which keeps it clean under selection: a coloured cell resets colour where it ends, and only at the row's end does that reset land past the highlighted content. RAPTOR-19762 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6f67a4b to
714f250
Compare
Code OwnershipWorkload Cli
Review requested from the teams above. Labels will be removed automatically upon approval. |
|
/approve-smoke-tests |
|
🔐 Fork PR smoke tests triggered by @ajalon1 What happens next:
|
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
|
✅ All smoke tests passed! (Fork PR) ✅ Security Scan: success |
There was a problem hiding this comment.
Nice work — the yellow selection now reads consistently across every list in the wizard, and the · in use lift in the base-image picker is a real usability win. Thanks for using GetAdaptiveColor(tui.DrYellow, tui.DrYellowDark).
One blocking finding (inline): the coloured tag inside a table cell collides with columnsFor's len(cell) sizing and bubbles/table's ANSI-unaware truncation — reproducible with a probe against the real escape bytes. Plus one minor consistency nit on the cursor glyph. Separately, I'd like to have a discussion in the future about making the UI consistent outside the workload wizard — today cmd/templates/list, cmd/llm-gateway/select, cmd/component/shared and tui/hostpicker each carry their own selection vocabulary (mostly non-adaptive purples) — whether that's adopting this wizard's design (a shared exported selectedStyle in tui) or something else entirely. Not a blocker for this PR, which is correctly scoped.
| // rest of the row intact. | ||
| if liveID != "" && ee.ID == liveID { | ||
| last := len(row.cells) - 1 | ||
| row.cells[last] += liveStyle.Render(inUseSuffix) |
There was a problem hiding this comment.
[P1] The coloured tag inside this cell breaks column sizing and can be cut mid-escape-sequence by bubbles/table.
Two downstream consumers don't know this cell now carries ~24 bytes of SGR codes:
columnsFor(table.go:158) sizes columns withlen(cell), so whenever a bound workload exists the ID column is sized ~20 columns wider than its visible content.- Worse:
bubbles/tabletruncates every cell withrunewidth.Truncate(value, col.Width, "…"), and go-runewidth is not ANSI-aware. Verified with a probe against the exact escape bytes lipgloss emits:
len(cell)=62 runewidth.StringWidth(cell)=59 // escape interiors counted as visible
Truncate(cell, 40) => "ee-1a2b3c4d5e6f7a8b9c0d1e2f\x1b[38;2;12…"
The truncated string ends inside the SGR sequence — an unterminated escape that bleeds green over the rest of the row. This fires whenever the overflow branch in columnsFor (or the 56-char maxColumnWidth cap) shrinks the ID column below the tagged cell's escape-inflated width — i.e. a bound workload on an ~80-column terminal, which is exactly this feature's use case. The tests miss it because they run in the no-colour profile, where liveStyle.Render is plain text.
Suggested direction: keep escape sequences out of the measured/truncated path — either measure with an ANSI-aware width consistent with what bubbles/table itself sees (runewidth.StringWidth, making Truncate a no-op) and accept the cosmetic width, or render the picker's suffix as plain text and keep the green tag on the menu screens only.
|
|
||
| if i == c.selected { | ||
| b.WriteString(tui.InfoStyle.Render("> " + line)) | ||
| b.WriteString(selectedStyle.Render("> " + line)) |
There was a problem hiding this comment.
[P3] Two cursor glyphs inside one wizard.
The PR's stated goal is "one selection look", but the menus render the cursor as ASCII "> " here while the pickers render " ❯" (table.go syncRows). Same semantic, sibling-file inconsistency — worth unifying on ❯ while this PR is already touching both.
2 participants
RATIONALE
In the
dr workload up/configwizard, it was hard to see which option was selected. The list pickers (workload binding, base-image selection) marked the cursor row with only a❯— the colour cue that was meant to accompany it never showed, becausebubbles/tablecolours each cell first and those cells' ANSI resets truncate any style wrapped around the finished row. And the "· in use" tag that marks a bound workload's current choice appeared on the menus but not in the base-image picker.Ticket: RAPTOR-19762
CHANGES
bubbles/tablerenders the cursor row's colour over plain cells cleanly instead of having it swallowed. Non-selected rows go neutral, the conventional look for a selection list.· in usein the base-image picker. The picker now tags the environment a bound workload is already built on and lifts it to the top — the list runs long, and the option most likely to be kept should not have to be scrolled for. The tag is green (the wizard's live-value colour) and trails on the right, matching the menus; it rides the last cell, which is what keeps it clean under selection — a coloured cell resets colour where it ends, and only at the row's end does that reset land past the highlighted content. Scoped to a bound run: a fresh setup has no live environment, so nothing is tagged and the order is left as it came.valueStyle; foldedselectedRowStyleintoselectedStyle.Verified against the rendered escapes: a selected in-use picker row emits yellow across name/language/id and green for the trailing
· in use, with non-selected rows plain.PR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation tests🤖 Generated with Claude Code