Repository navigation
feat(tui): show session transcript scrollbar by default - #56567
imthegoodboy wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
This works as described. I ran the TUI from source on the base and on this branch with a long simulated reply. The transcript scrollbar now appears by default, and setting session.scrollbar: false still hides it. The toggle command and the settings dialog stay in sync with the new ?? true fallback. The new test and bun run typecheck in packages/tui pass.
Public API: no keys are added or removed. The effective default of the session.scrollbar config key changes from false to true. Configs that set it explicitly keep their behaviour. Users who never set it will now see the scrollbar.
Things maintainers should weigh:
- This reverses an earlier decision. Maintainer PR #20947 deliberately set this default back to hidden and kept the scrollbar opt-in, and several earlier attempts to flip it (#46650, #33412, #32195, #15282) were closed without merging. CONTRIBUTING.md also says UI features need design review before implementation. Whether to ship this is a product call for maintainers, not a code question.
- The test is large for a one-line default change. See the inline comment.
The before/after captures in the description look swapped (the one with the thumb is labelled "Before"). The attached screenshots show the base and this branch.
| @@ -0,0 +1,89 @@ | |||
| import { expect, test } from "bun:test" | |||
There was a problem hiding this comment.
This is 89 lines, including a hand-written stub of the session API, to check what is essentially a one-line change of the default. The opt-out case with mouse-wheel scrolling tests behaviour this PR doesn't touch. I'd keep at most the default-visible case, or drop the file so the change stays minimal.
d47659f to
03774bf
Compare
03774bf to
c951c51
Compare
|
Thanks for the review.
|
Issue for this PR
Closes #46654 — the open feature request for exactly this default.
Type of change
What does this PR do?
The transcript scrollbar exists as a config option but shipped off: out of the box the CLI shows no scrollbar while scrolling the session transcript. This makes it the default — session.scrollbar now falls back to on, and the settings dialog's default marker matches it. Users who prefer it gone can still set session.scrollbar to false in cli.json, and the existing "Toggle session scrollbar" command keeps working the same way.
The sample config in the docs CLI config page is updated to the new default so the example matches what a fresh install does.
On history: #20947 deliberately kept the scrollbar opt-in, and earlier flips were closed. This PR is meant as that product decision's review with its standing user request (#46654) attached — if maintainers prefer to keep it opt-in, closing this is a perfectly fine outcome.
How did you verify your code works?
New test packages/tui/test/session-scrollbar-default.test.tsx mounts a session on the app fixture with no scrollbar config and asserts the transcript scrollbox reports its scrollbar visible; session.scrollbar=false keeps its existing behaviour. The rest of the packages/tui suite behaves identically to plain dev (same 7 pre-existing environment/timing failures in plugin-source, dialog-shell-output, app-lifecycle; zero new diffs). bun typecheck from packages/tui and the full bun run check pass.
Screenshots / recordings
Text captures from the test renderer at 120 columns. The scrollbar draws as a color-filled track with a thumb on the transcript's right edge, so it reads subtle in plain text:
Before — the previous default, scrollbar off:
After — the new default, scrollbar on (thumb's edge cell at the right border; the thumb body is color fill):
Checklist