fix(cli): make SkillInboxDialog fit and scroll in alternate buffer#26455
fix(cli): make SkillInboxDialog fit and scroll in alternate buffer#26455SandyTao520 merged 2 commits intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses UI overflow issues in the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Hi @SandyTao520, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
There was a problem hiding this comment.
Code Review
This pull request introduces a ScrollableDiffViewport to the SkillInboxDialog to handle large diffs and implements dynamic height calculations to prevent terminal buffer overflows. It also adds row truncation for long descriptions and navigation affordances for scrolling. Feedback identifies a security vulnerability in how origin tags are determined from file paths and a performance issue where diff sections are not memoized, leading to expensive re-renders.
|
Size Change: +6.67 kB (+0.02%) Total Size: 34 MB
ℹ️ View Unchanged
|
The /memory inbox dialog overflowed the alternate screen buffer in
two ways: the diff preview rendered every line at full natural
height with no scrollback to recover them, and the list phase used
a hard-coded maxItemsToShow=8 with rows that wrapped to 3-5 lines
for long descriptions, pushing the dialog footer off the bottom of
the screen.
Diff rendering branches on `useAlternateBuffer`:
- Alt-buffer (no terminal scrollback): the skill-preview,
patch-preview, and memory-preview phases wrap the diff in a
fixed-height ScrollableList of diff lines.
PgUp/PgDn/Shift+arrows move the viewport over arbitrarily long
diffs. The DialogFooter advertises "PgUp/PgDn to scroll".
- Non-alt-buffer: the codebase's standard bounded DiffRenderer +
ShowMoreLines + Ctrl+O pattern (matches FolderTrustDialog and
ThemeDialog). Clipped content lands in terminal scrollback when
the user expands via Ctrl+O. The Ctrl+O hint is surfaced inline
by ShowMoreLines above the footer.
The dialog is wrapped in OverflowProvider so ShowMoreLines can
detect MaxSizedBox overflow inside DiffRenderer.
List rows now render at exactly height={2} with wrap="truncate-end"
so long descriptions no longer wrap unpredictably. This also fixes
a visible artifact where the date sibling broke the description's
word wrap. maxItemsToShow is derived from terminalHeight using a
documented chrome budget so the dialog footer stays visible on
shorter terminals.
Address PR #26455 review feedback. Each ScrollableDiffViewport call site previously built its sections array as a fresh literal on every render of InboxDialog. ScrollableDiffViewport memoizes its expensive parseDiffWithLineNumbers + renderDiffLines work on `sections`, so the literals defeated the memo and re-colorized the diff on every parent re-render (e.g. each feedback toggle). Hoist the per-phase preview data (skill, patch, memory) into a single useMemo keyed on `selectedItem`. The memo also subsumes the memory-patch grouping logic so both the alt-buffer and non-alt-buffer memory-preview branches read the same stable references. The hook lives above the dialog's loading / empty-state early returns to satisfy React's rules-of-hooks.
9baf3d8 to
b1e9dc4
Compare
Summary
/memory inboxoverflowed the alternate screen buffer in two ways: thediff preview rendered every line at full natural height with no
scrollback to recover them, and the list phase used a hard-coded
maxItemsToShow=8with rows that wrapped to 3-5 lines for longdescriptions, pushing the dialog footer off the bottom of the screen.
Details
demo.webm
Scrollable diff viewport — the skill-preview and patch-preview
phases now wrap the diff in a fixed-height
ScrollableListof difflines.
PgUp/PgDn,Shift+arrows, andCtrl/Shift+Home/Endmove the viewport over arbitrarily long diffs. The
DialogFooter's existingnavigationActionsslot surfaces theaffordance to the user.
Predictable list-row height — every list row (skill, patch) now
renders at exactly
height={2}withwrap="truncate-end". Previouslylong descriptions used
wrap="wrap"and were rendered as siblingsof the date in a
flexDirection="row"box, causing each item tooccupy 3-5 rows and visibly interleaving the date into the
description's word wrap (e.g.
· May 4,\nmode2026).Terminal-aware list windowing —
maxItemsToShowis now derivedfrom
terminalHeightminus a documented chrome budget (border,padding, title/subtitle, footer, scroll arrows,
DefaultAppLayout'salt-buffer
paddingBottom). The dialog footer remains visible onshorter terminals.
The scrollable diff viewport follows the same
ScrollableListprimitive used by
MainContentandDetailedMessagesDisplay. Thelist-row truncation pattern matches the height-budgeting approach
established in
FooterConfigDialogandRewindViewer.Related Issues
Related to #18007
How to Validate
npm run startin alternate buffer mode (the default)./memory inboxwith a populated skill inbox containing at leastone skill with a long description.
Esc to close) isvisible and that no list row wraps onto a third line.
SKILL.mdor multi-file diff.Apply/Dismiss(or destination) action list is below it, and thefooter shows
Enter to confirm · PgUp/PgDn to scroll · Esc to go back.PgDn(orShift+Down) — the diff scrolls; the action listand footer stay in place.
useAlternateBuffer: falseinsettings) to verify nothing regressed.
Pre-Merge Checklist