fix(tui): keep model picker selection on Esc#2056
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the project version to 0.8.45 and continues the rebranding transition from DeepSeek-TUI to CodeWhale through extensive documentation updates, installation guide revisions, and configuration path migrations. Key functional improvements include the addition of a /balance command scaffold, deterministic sub-agent nicknames, enhanced TUI message visibility, and fixed YAML parsing for skill descriptions. A notable logic change was introduced to the TUI model picker, where the Esc key now applies the current selection if the user has moved the highlight. Feedback suggests documenting this model picker change in the changelogs and questions the UX implications of losing the ability to discard changes via Esc once a selection is touched.
| - **User messages stand out in the transcript.** The TUI now gives user turns | ||
| a clearer visual treatment so request/answer boundaries are easier to see | ||
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | ||
| @lpeng1711694086-lang for the original report. |
There was a problem hiding this comment.
The fix for the model picker selection behavior (issue #2037) is missing from the changelog. Since this PR includes a version bump and updates to the changelog for other issues, it should also include an entry for the primary fix described in the PR title.
| - **User messages stand out in the transcript.** The TUI now gives user turns | |
| a clearer visual treatment so request/answer boundaries are easier to see | |
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | |
| @lpeng1711694086-lang for the original report. | |
| - **Model picker keeps selection on Esc.** If the selection has moved, pressing | |
| Esc now applies the highlighted model/effort instead of cancelling, matching | |
| user expectations for quick selection (#2037). | |
| - **User messages stand out in the transcript.** The TUI now gives user turns | |
| a clearer visual treatment so request/answer boundaries are easier to see | |
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | |
| @lpeng1711694086-lang for the original report. |
| - **User messages stand out in the transcript.** The TUI now gives user turns | ||
| a clearer visual treatment so request/answer boundaries are easier to see | ||
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | ||
| @lpeng1711694086-lang for the original report. |
There was a problem hiding this comment.
The fix for the model picker selection behavior (issue #2037) is missing from the changelog. Please ensure this is documented here as well to keep the crate-level changelog in sync with the root.
| - **User messages stand out in the transcript.** The TUI now gives user turns | |
| a clearer visual treatment so request/answer boundaries are easier to see | |
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | |
| @lpeng1711694086-lang for the original report. | |
| - **Model picker keeps selection on Esc.** If the selection has moved, pressing | |
| Esc now applies the highlighted model/effort instead of cancelling, matching | |
| user expectations for quick selection (#2037). | |
| - **User messages stand out in the transcript.** The TUI now gives user turns | |
| a clearer visual treatment so request/answer boundaries are easier to see | |
| (#1995, closes #1672). Thanks @reidliu41 for the focused fix and | |
| @lpeng1711694086-lang for the original report. |
| KeyCode::Esc if self.selection_touched => ViewAction::EmitAndClose(self.build_event()), | ||
| KeyCode::Esc => ViewAction::Close, |
There was a problem hiding this comment.
This change makes Esc behave identically to Enter once the selection has been moved. While this addresses the issue of accidental cancellations, it removes the ability for a user to explicitly discard changes if they change their mind after moving the highlight.
Consider if there should be a way to still cancel (e.g., a different key combination or checking if the selection actually changed from the initial state) or if this 'sticky' behavior is the desired UX for all cases where the cursor moved.
reidliu41
force-pushed
the
fix/model-picker-esc-selection
branch
from
79c97a3 to
26ae8a6
Compare
|
hi @Hmbown feel free to take a look when you have time. thanks. |
2 participants
Summary
Closes #2037
The
/modelpicker previously only applied changes on Enter. If a user moved the highlighted model with the arrow keys and then pressed Esc, the picker closed without emitting the selected model, so the footer reverted to the previous active model.This PR keeps the last highlighted model/effort when Esc is pressed after the selection has moved. Opening the picker and
pressing Esc immediately still behaves as cancel/no-op.
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist