Fix tool output clipping and live-edge scroll following
Preserve full terminal-grid width with horizontal scrolling in tool panes. Keep hidden command control changes from unpinning the viewport, and re-pin scrolling at the live edge in normal and inverted layouts. Validation: formatting, test-layout checks, both presubmit Clippy commands, and 141 relevant tests passed. Ten other terminal-view tests failed identically on the untouched baseline (keyboard, cloud-agent, input status, and footer behavior).
This commit is contained in:
@@ -8,7 +8,9 @@ use std::sync::Arc;
|
||||
use galaxy_core::ui::appearance::Appearance;
|
||||
use galaxy_core::ui::Icon;
|
||||
use galaxy_editor::render::element::VerticalExpansionBehavior;
|
||||
use galaxyui::elements::new_scrollable::SingleAxisConfig;
|
||||
use galaxyui::elements::new_scrollable::{
|
||||
ClippedAxisConfiguration, DualAxisConfig, ScrollableAppearance,
|
||||
};
|
||||
use galaxyui::elements::{
|
||||
Align, Border, ChildView, Clipped, ClippedScrollStateHandle, ConstrainedBox, Container,
|
||||
CornerRadius, CrossAxisAlignment, Expanded, Fill, Flex, MainAxisSize, MouseStateHandle,
|
||||
@@ -336,6 +338,7 @@ pub struct RequestedCommandView {
|
||||
is_user_expanded: bool,
|
||||
header_mouse_state: MouseStateHandle,
|
||||
output_scroll_state: ClippedScrollStateHandle,
|
||||
output_horizontal_scroll_state: ClippedScrollStateHandle,
|
||||
follow_command_output: Cell<bool>,
|
||||
is_editing: bool,
|
||||
|
||||
@@ -596,6 +599,7 @@ impl RequestedCommandView {
|
||||
is_user_expanded: false,
|
||||
header_mouse_state: Default::default(),
|
||||
output_scroll_state: ClippedScrollStateHandle::new(),
|
||||
output_horizontal_scroll_state: ClippedScrollStateHandle::new(),
|
||||
follow_command_output: Cell::new(true),
|
||||
copied_from_citation: None,
|
||||
derived_from_citations: Default::default(),
|
||||
@@ -1731,15 +1735,27 @@ impl View for RequestedCommandView {
|
||||
if should_use_ligature_rendering(app) {
|
||||
output_grid = output_grid.with_ligature_rendering();
|
||||
}
|
||||
let scrollable = NewScrollable::vertical(
|
||||
SingleAxisConfig::Clipped {
|
||||
// The terminal grid can be wider than this padded tool pane. Lay it out
|
||||
// at its natural width so every column remains reachable by scrolling.
|
||||
let scrollable = NewScrollable::horizontal_and_vertical(
|
||||
DualAxisConfig::Clipped {
|
||||
horizontal: ClippedAxisConfiguration {
|
||||
handle: self.output_horizontal_scroll_state.clone(),
|
||||
max_size: None,
|
||||
stretch_child: false,
|
||||
},
|
||||
vertical: ClippedAxisConfiguration {
|
||||
handle: self.output_scroll_state.clone(),
|
||||
max_size: None,
|
||||
stretch_child: false,
|
||||
},
|
||||
child: output_grid.finish(),
|
||||
},
|
||||
Fill::None,
|
||||
Fill::None,
|
||||
theme.nonactive_ui_detail().into(),
|
||||
theme.active_ui_detail().into(),
|
||||
Fill::None,
|
||||
)
|
||||
.with_horizontal_scrollbar(ScrollableAppearance::new(SCROLLBAR_WIDTH, false))
|
||||
.with_propagate_mousewheel_if_not_handled(true)
|
||||
.finish();
|
||||
let output_body = ConstrainedBox::new(scrollable)
|
||||
|
||||
@@ -136,6 +136,9 @@ impl ScrollLines {
|
||||
|
||||
#[derive(Copy, Clone, Debug, PartialEq)]
|
||||
pub enum ScrollPosition {
|
||||
/// Follow newly inserted content at the top of an inverted block list.
|
||||
FollowsTopOfMostRecentBlock,
|
||||
|
||||
/// The scrolling follows the bottom of the most recently executed block.
|
||||
/// In terms of scroll_top, this implies scrolling stays locked to max_scroll_top.
|
||||
FollowsBottomOfMostRecentBlock,
|
||||
@@ -645,6 +648,7 @@ impl<'a> ViewportState<'a> {
|
||||
/// How far the view is scrolled from the top of all blocks in lines.
|
||||
pub fn scroll_top_in_lines(&self) -> Lines {
|
||||
match (self.input_mode, self.scroll_position) {
|
||||
(_, ScrollPosition::FollowsTopOfMostRecentBlock) => Lines::zero(),
|
||||
(
|
||||
InputMode::PinnedToBottom,
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
@@ -775,8 +779,18 @@ impl<'a> ViewportState<'a> {
|
||||
return self.scroll_position;
|
||||
}
|
||||
|
||||
// Keep an established live-edge pin across tool/response insertion.
|
||||
// Recomputing a waterfall gap offset here can turn it into a fixed offset.
|
||||
match self.scroll_position {
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
| ScrollPosition::FollowsBottomOfMostRecentBlock => self.scroll_position,
|
||||
ScrollPosition::WaterfallGapFollowsBottomOfMostRecentBlock { .. }
|
||||
| ScrollPosition::FixedAtPosition { .. }
|
||||
| ScrollPosition::FixedWithinLongRunningBlock { .. } => {
|
||||
self.scroll_position_after_command_execution(app)
|
||||
}
|
||||
}
|
||||
}
|
||||
ScrollPositionUpdate::AfterKeydownOnTerminal
|
||||
| ScrollPositionUpdate::AfterTypedCharacters
|
||||
| ScrollPositionUpdate::AfterWriteUserBytesToPty => {
|
||||
@@ -821,34 +835,19 @@ impl<'a> ViewportState<'a> {
|
||||
self.scroll_position
|
||||
}
|
||||
}
|
||||
ScrollPositionUpdate::AfterPageUp => {
|
||||
let new_scroll_top = (self.scroll_top_in_lines()
|
||||
- self.content_element_height_lines()
|
||||
+ 1.0.into_lines())
|
||||
.max(Lines::zero());
|
||||
ScrollPosition::FixedAtPosition {
|
||||
scroll_lines: self.scroll_lines_from_scroll_top(new_scroll_top),
|
||||
}
|
||||
}
|
||||
ScrollPositionUpdate::AfterPageDown => {
|
||||
let total_block_heights = self.block_list.block_heights().summary().height;
|
||||
let visible_rows = self.content_element_height_lines();
|
||||
let current_position = self.scroll_top_in_lines();
|
||||
if current_position + visible_rows - 1.0.into_lines()
|
||||
> (total_block_heights - visible_rows).max(Lines::zero())
|
||||
{
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
ScrollPositionUpdate::AfterPageUp => self
|
||||
.scroll_position_for_delta(self.content_element_height_lines() - 1.0.into_lines()),
|
||||
ScrollPositionUpdate::AfterPageDown => self
|
||||
.scroll_position_for_delta(1.0.into_lines() - self.content_element_height_lines()),
|
||||
ScrollPositionUpdate::AfterHome => self.scroll_position_for_offset(Lines::zero()),
|
||||
ScrollPositionUpdate::AfterEnterAgentView => {
|
||||
if self.input_mode.is_inverted_blocklist() {
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
} else {
|
||||
let new_scroll_top = current_position + visible_rows - 1.0.into_lines();
|
||||
ScrollPosition::FixedAtPosition {
|
||||
scroll_lines: self.scroll_lines_from_scroll_top(new_scroll_top),
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
}
|
||||
}
|
||||
}
|
||||
ScrollPositionUpdate::AfterHome => ScrollPosition::FixedAtPosition {
|
||||
scroll_lines: self.scroll_lines_from_scroll_top(Lines::zero()),
|
||||
},
|
||||
ScrollPositionUpdate::AfterEnd | ScrollPositionUpdate::AfterEnterAgentView => {
|
||||
ScrollPositionUpdate::AfterEnd => {
|
||||
if matches!(
|
||||
self.input_mode,
|
||||
InputMode::PinnedToBottom | InputMode::Waterfall
|
||||
@@ -904,7 +903,8 @@ impl<'a> ViewportState<'a> {
|
||||
) -> ScrollPosition {
|
||||
if matches!(
|
||||
self.scroll_position,
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
| ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
| ScrollPosition::WaterfallGapFollowsBottomOfMostRecentBlock { .. }
|
||||
) {
|
||||
return self.scroll_position;
|
||||
@@ -975,7 +975,8 @@ impl<'a> ViewportState<'a> {
|
||||
) -> ScrollPosition {
|
||||
if matches!(
|
||||
self.scroll_position,
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
| ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
| ScrollPosition::WaterfallGapFollowsBottomOfMostRecentBlock { .. }
|
||||
) {
|
||||
return self.scroll_position;
|
||||
@@ -1203,9 +1204,8 @@ impl<'a> ViewportState<'a> {
|
||||
|
||||
fn scroll_position_after_clear(&self) -> ScrollPosition {
|
||||
match self.input_mode {
|
||||
InputMode::PinnedToTop | InputMode::PinnedToBottom => {
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
}
|
||||
InputMode::PinnedToTop => ScrollPosition::FollowsTopOfMostRecentBlock,
|
||||
InputMode::PinnedToBottom => ScrollPosition::FollowsBottomOfMostRecentBlock,
|
||||
InputMode::Waterfall => ScrollPosition::WaterfallGapFollowsBottomOfMostRecentBlock {
|
||||
scroll_top_in_lines: self.max_scroll_top_in_lines(),
|
||||
},
|
||||
@@ -1254,6 +1254,7 @@ impl<'a> ViewportState<'a> {
|
||||
// Returns the scroll position to set after a command has finished executing
|
||||
fn scroll_position_after_command_execution(&self, app: &AppContext) -> ScrollPosition {
|
||||
match (self.input_mode, self.block_list.active_gap()) {
|
||||
(InputMode::PinnedToTop, _) => ScrollPosition::FollowsTopOfMostRecentBlock,
|
||||
(InputMode::Waterfall, Some(gap)) => {
|
||||
// In gap waterfall mode, the logic is somewhat complex for how to adjust scroll position
|
||||
// after a command is executed. The basic result we want is:
|
||||
@@ -1297,13 +1298,20 @@ impl<'a> ViewportState<'a> {
|
||||
let current_top = self.scroll_top_in_lines();
|
||||
|
||||
let new_top = (current_top - delta).max(Lines::zero()).min(max_scroll_top);
|
||||
let fix_to_bottom = new_top >= max_scroll_top
|
||||
&& matches!(
|
||||
self.input_mode,
|
||||
InputMode::PinnedToBottom | InputMode::Waterfall
|
||||
);
|
||||
// Horizontal wheel gestures and no-op events must not change follow state.
|
||||
if delta == Lines::zero() {
|
||||
return self.scroll_position;
|
||||
}
|
||||
self.scroll_position_for_offset(new_top)
|
||||
}
|
||||
|
||||
if fix_to_bottom {
|
||||
fn scroll_position_for_offset(&self, new_top: Lines) -> ScrollPosition {
|
||||
let tolerance = Pixels::new(0.5).to_lines(self.size_info.cell_height_px());
|
||||
if self.input_mode.is_inverted_blocklist() && new_top <= tolerance {
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
} else if !self.input_mode.is_inverted_blocklist()
|
||||
&& new_top + tolerance >= self.max_scroll_top_in_lines()
|
||||
{
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
} else if self.block_list.active_block().is_active_and_long_running()
|
||||
&& self.does_block_exceed_viewport(self.block_list.active_block_index(), new_top)
|
||||
|
||||
@@ -6797,7 +6797,17 @@ impl TerminalView {
|
||||
..
|
||||
} => {
|
||||
if !*agent_has_control && ctx.is_self_or_child_focused() {
|
||||
let block_index = self.model.lock().block_list().block_index_for_id(block_id);
|
||||
// Tool output is rendered in its agent card. Its hidden shell block
|
||||
// must not steal the viewport or clear the user's follow-scroll state.
|
||||
let block_index = {
|
||||
let model = self.model.lock();
|
||||
let blocks = model.block_list();
|
||||
blocks.block_index_for_id(block_id).filter(|index| {
|
||||
blocks
|
||||
.block_at(*index)
|
||||
.is_some_and(|block| block.is_visible(blocks.agent_view_state()))
|
||||
})
|
||||
};
|
||||
if let Some(block_index) = block_index {
|
||||
self.update_scroll_position_locking(
|
||||
ScrollPositionUpdate::ScrollToBottomOfBlock { block_index },
|
||||
|
||||
@@ -3433,7 +3433,7 @@ fn test_viewport_most_recent_at_top() {
|
||||
},
|
||||
ctx
|
||||
),
|
||||
ScrollPosition::FixedAtPosition { .. }
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
));
|
||||
assert_eq!(
|
||||
Lines::zero(),
|
||||
@@ -8309,3 +8309,121 @@ fn child_shell_startup_failure_finishes_the_waiting_conversation() {
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scrolling_to_live_edge_repins_both_blocklist_directions() {
|
||||
App::test((), |mut app| async move {
|
||||
initialize_app_for_terminal_view(&mut app);
|
||||
let terminal = add_window_with_terminal(&mut app, None);
|
||||
terminal.update(&mut app, |view, ctx| {
|
||||
for mode in [
|
||||
InputMode::PinnedToBottom,
|
||||
InputMode::Waterfall,
|
||||
InputMode::PinnedToTop,
|
||||
] {
|
||||
let mut model = view.model.lock();
|
||||
for _ in 0..100 {
|
||||
model.simulate_block("echo", "output");
|
||||
}
|
||||
let pinned = if mode.is_inverted_blocklist() {
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock
|
||||
} else {
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock
|
||||
};
|
||||
view.scroll_position = ScrollState::new(pinned);
|
||||
let away = if mode.is_inverted_blocklist() {
|
||||
-5.0
|
||||
} else {
|
||||
5.0
|
||||
};
|
||||
let viewport = view.viewport_state(model.block_list(), mode, ctx);
|
||||
let unpinned = viewport.next_scroll_position(
|
||||
ScrollPositionUpdate::AfterScrollEvent {
|
||||
scroll_delta: away.into_lines(),
|
||||
},
|
||||
ctx,
|
||||
);
|
||||
assert!(matches!(unpinned, ScrollPosition::FixedAtPosition { .. }));
|
||||
view.scroll_position = ScrollState::new(unpinned);
|
||||
model.simulate_block("echo", "new output");
|
||||
assert_eq!(view.scroll_position(), unpinned);
|
||||
let viewport = view.viewport_state(model.block_list(), mode, ctx);
|
||||
let toward = if mode.is_inverted_blocklist() {
|
||||
10000.0
|
||||
} else {
|
||||
-10000.0
|
||||
};
|
||||
let repinned = viewport.next_scroll_position(
|
||||
ScrollPositionUpdate::AfterScrollEvent {
|
||||
scroll_delta: toward.into_lines(),
|
||||
},
|
||||
ctx,
|
||||
);
|
||||
assert_eq!(repinned, pinned);
|
||||
view.scroll_position = ScrollState::new(repinned);
|
||||
model.simulate_block("echo", "more output");
|
||||
let viewport = view.viewport_state(model.block_list(), mode, ctx);
|
||||
assert_eq!(
|
||||
viewport
|
||||
.next_scroll_position(ScrollPositionUpdate::AfterRichBlockInserted, ctx),
|
||||
pinned
|
||||
);
|
||||
assert_eq!(
|
||||
viewport.next_scroll_position(
|
||||
ScrollPositionUpdate::AfterScrollEvent {
|
||||
scroll_delta: Lines::zero()
|
||||
},
|
||||
ctx
|
||||
),
|
||||
pinned
|
||||
);
|
||||
if mode.is_inverted_blocklist() {
|
||||
assert_eq!(viewport.scroll_top_in_lines(), Lines::zero());
|
||||
} else {
|
||||
assert_eq!(
|
||||
viewport.scroll_top_in_lines(),
|
||||
viewport.max_scroll_top_in_lines()
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hidden_tool_control_changes_preserve_pinned_and_unpinned_scroll() {
|
||||
App::test((), |mut app| async move {
|
||||
initialize_app_for_terminal_view(&mut app);
|
||||
let terminal = add_window_with_terminal(&mut app, None);
|
||||
terminal.update(&mut app, |view, ctx| {
|
||||
view.focus_terminal(ctx);
|
||||
assert!(ctx.is_self_or_child_focused());
|
||||
let block_id = {
|
||||
let mut model = view.model.lock();
|
||||
model.simulate_cmd("echo tool output");
|
||||
let block = model.block_list_mut().active_block_mut();
|
||||
block.hide();
|
||||
block.id().clone()
|
||||
};
|
||||
for position in [
|
||||
ScrollPosition::FollowsBottomOfMostRecentBlock,
|
||||
ScrollPosition::FollowsTopOfMostRecentBlock,
|
||||
ScrollPosition::FixedAtPosition {
|
||||
scroll_lines: ScrollLines::ScrollTop(2.0.into_lines()),
|
||||
},
|
||||
] {
|
||||
view.scroll_position = ScrollState::new(position);
|
||||
view.handle_cli_subagent_controller_event(
|
||||
view.cli_subagent_controller.clone(),
|
||||
&CLISubagentEvent::UpdatedControl {
|
||||
block_id: block_id.clone(),
|
||||
requested_command_action_id: None,
|
||||
agent_has_control: false,
|
||||
},
|
||||
ctx,
|
||||
);
|
||||
assert_eq!(view.scroll_position(), position);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user