fix(ui): move margin from top to bottom in ToolGroupMessage#17198
fix(ui): move margin from top to bottom in ToolGroupMessage#17198scidomino merged 1 commit intogoogle-gemini:mainfrom
Conversation
gemini-cli
bot
added
the
status/need-issue
|
Closing as the issue is not reproducible. |
|
Opening because of false negative. Issue persists. |
imadraude
force-pushed
the
fix/tool-group-margin
branch
from
8b42b7b to
4b8de10
Compare
|
/gemini review |
![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 aims to improve layout consistency by moving the vertical spacing for ToolGroupMessage from internal margins to a marginBottom on the root container. The change in ToolGroupMessage.tsx and the updated snapshots reflect this intent. However, I've identified a significant issue where an internal height calculation was not updated to reflect this change, which leads to incorrect rendering and content truncation. A fix is suggested in the detailed comment, aligning with the principle of maintaining consistent UI layout.
imadraude
force-pushed
the
fix/tool-group-margin
branch
from
4b8de10 to
7cc3eba
Compare
gemini-cli
bot
added
area/core
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the spacing for the ToolGroupMessage component by moving it from an internal height calculation to a marginBottom on the root container. This correctly adds a consistent margin after the tool group, improving layout separation. The change to staticHeight is a good bug fix, as it was previously over-reserving space for conditional UI elements, and now correctly allocates more vertical space for tool content. The snapshot updates are consistent with these intended layout improvements. Overall, this is a solid change that improves layout correctness and component encapsulation.
gemini-cli
bot
added
priority/p2
|
I will attempted to resolve the conflicts and merge. |
scidomino
force-pushed
the
fix/tool-group-margin
branch
from
7cc3eba to
ea053c4
Compare
scidomino
force-pushed
the
fix/tool-group-margin
branch
from
ea053c4 to
c89121a
Compare
The unconditionally applied `marginBottom={1}` introduced in PR #17198 caused a visual gap
when a tool group is partially rendered between the static and pending areas
(i.e. `borderBottomOverride === false`).
By conditioning `marginBottom` on `borderBottomOverride !== false`, intermediate chunks
seamlessly connect with their bottom border chunks, while completed tool groups
still get the appropriate padding margin.
Fixes regression introduced in e7f97df.
The unconditionally applied `marginBottom={1}` introduced in PR #17198 caused a visual gap
when a tool group is partially rendered between the static and pending areas
(i.e. `borderBottomOverride === false`).
By conditioning `marginBottom` on `borderBottomOverride !== false`, intermediate chunks
seamlessly connect with their bottom border chunks, while completed tool groups
still get the appropriate padding margin.
Fixes regression introduced in e7f97df.
2 participants
Summary
Moves the spacing for the
ToolGroupMessagecomponent frommarginTopon internal elements tomarginBottomon the root container. This ensures better logical separation of the tool block from subsequent messages.Details
marginTopwhere it was causing layout issues.marginBottom={1}to the rootBoxofToolGroupMessage.Related Issues
Fixes #17283
How to Validate
list_directory).npm test -w @google/gemini-cli -- src/ui/components/messages/ToolGroupMessage.test.tsxPre-Merge Checklist