Skip to content

Commit e6b48da

Browse files
claudesinelaw
authored andcommitted
fix(tabs): clamp the "+" new-tab popup's hit-tests to its drawn position
The `+` new-tab popup (`NewTabMenu`) still had the clamp drift the shared `clamped_position` helper set out to eliminate for the tab and file- explorer menus: `render_new_tab_menu` hand-clamped `menu.position` when it would overflow the right/bottom edge, while the hover and click hit-tests used the raw, unclamped position. Opening the popup within `NEW_TAB_MENU_WIDTH` of the right edge drew it shifted onto screen but left its clickable/hoverable region anchored partly offscreen, so a click on a visible item was treated as an outside-click and silently dismissed the menu instead of activating it. Give `NewTabMenu` the same `height()`/`clamped_position()` helpers as `TabContextMenu` and route render + both hit-tests through them, clamping against the last rendered frame size (as the tab menu already does) so render and input always agree. Regression test `plus_button_menu_near_right_edge_clicks_land_on_drawn_items` fails without the hit-test fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQYut8xrWsYyQffc4D84dz
1 parent d133390 commit e6b48da

4 files changed

Lines changed: 98 additions & 20 deletions

File tree

crates/fresh-editor/src/app/mouse_input.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,11 +1028,16 @@ impl Editor {
10281028

10291029
// Check the "+" new-tab popup menu (rendered on top)
10301030
if let Some(ref menu) = self.active_window().new_tab_menu {
1031-
let menu_x = menu.position.0;
1032-
let menu_y = menu.position.1;
1031+
// Clamp against the last rendered frame size (mirrored from
1032+
// `frame.area()` each draw), matching the popup's render clamp — so
1033+
// a menu near the right/bottom edge hovers exactly where it draws.
1034+
let (menu_x, menu_y) = menu.clamped_position(
1035+
self.active_chrome().last_frame.width,
1036+
self.active_chrome().last_frame.height,
1037+
);
10331038
let menu_width = super::types::NEW_TAB_MENU_WIDTH;
10341039
let items = super::types::NewTabMenuItem::all();
1035-
let menu_height = items.len() as u16 + 2;
1040+
let menu_height = menu.height();
10361041

10371042
if col >= menu_x
10381043
&& col < menu_x + menu_width
@@ -3527,11 +3532,16 @@ impl Editor {
35273532
col: u16,
35283533
row: u16,
35293534
) -> Option<AnyhowResult<()>> {
3530-
let menu = self.active_window_mut().new_tab_menu.as_ref()?;
3531-
let (menu_x, menu_y) = menu.position;
3535+
// Clamp against the last rendered frame size, matching the popup's
3536+
// render and hover hit-test — during a resize `terminal_width/height`
3537+
// can lag `frame.area()`, drifting clicks off the drawn menu.
3538+
let frame_w = self.active_chrome().last_frame.width;
3539+
let frame_h = self.active_chrome().last_frame.height;
3540+
let menu = self.active_window().new_tab_menu.as_ref()?;
3541+
let (menu_x, menu_y) = menu.clamped_position(frame_w, frame_h);
35323542
let items = super::types::NewTabMenuItem::all();
35333543
let menu_width = super::types::NEW_TAB_MENU_WIDTH;
3534-
let menu_height = items.len() as u16 + 2; // items + borders
3544+
let menu_height = menu.height();
35353545

35363546
// Click outside the menu closes it.
35373547
if col < menu_x || col >= menu_x + menu_width || row < menu_y || row >= menu_y + menu_height

crates/fresh-editor/src/app/render.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3981,21 +3981,11 @@ impl Editor {
39813981

39823982
let items = super::types::NewTabMenuItem::all();
39833983
let menu_width = super::types::NEW_TAB_MENU_WIDTH;
3984-
let menu_height = items.len() as u16 + 2; // items + borders
3985-
3986-
let screen_width = frame.area().width;
3987-
let screen_height = frame.area().height;
3984+
let menu_height = menu.height();
39883985

3989-
let menu_x = if menu.position.0 + menu_width > screen_width {
3990-
screen_width.saturating_sub(menu_width)
3991-
} else {
3992-
menu.position.0
3993-
};
3994-
let menu_y = if menu.position.1 + menu_height > screen_height {
3995-
screen_height.saturating_sub(menu_height)
3996-
} else {
3997-
menu.position.1
3998-
};
3986+
// Shared clamp — the same helper hover and click hit-testing use, so
3987+
// the popup accepts input exactly where it draws.
3988+
let (menu_x, menu_y) = menu.clamped_position(frame.area().width, frame.area().height);
39993989

40003990
let area = ratatui::layout::Rect::new(menu_x, menu_y, menu_width, menu_height);
40013991

crates/fresh-editor/src/app/types/context_menu.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,31 @@ impl NewTabMenu {
180180
}
181181
}
182182

183+
/// Menu height including the top/bottom borders.
184+
pub fn height(&self) -> u16 {
185+
NewTabMenuItem::all().len() as u16 + 2
186+
}
187+
188+
/// Anchor position shifted so the menu fits on screen — the single source
189+
/// of truth shared by rendering, hover, and click hit-testing, exactly as
190+
/// [`TabContextMenu::clamped_position`]. Diverging copies would let the
191+
/// popup draw shifted onto screen while its clickable region stayed
192+
/// anchored offscreen.
193+
pub fn clamped_position(&self, screen_width: u16, screen_height: u16) -> (u16, u16) {
194+
let x = if self.position.0 + NEW_TAB_MENU_WIDTH > screen_width {
195+
screen_width.saturating_sub(NEW_TAB_MENU_WIDTH)
196+
} else {
197+
self.position.0
198+
};
199+
let h = self.height();
200+
let y = if self.position.1 + h > screen_height {
201+
screen_height.saturating_sub(h)
202+
} else {
203+
self.position.1
204+
};
205+
(x, y)
206+
}
207+
183208
/// Move highlight down.
184209
pub fn next_item(&mut self) {
185210
let items = NewTabMenuItem::all();

crates/fresh-editor/tests/e2e/tab_new_button.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,59 @@ fn plus_button_pins_to_right_edge_on_overflow() {
8787
harness.assert_screen_contains("New File");
8888
}
8989

90+
/// Near the right edge the popup clamps left so it fits on screen — and its
91+
/// click hit-test must clamp identically, or a click on the *drawn* item lands
92+
/// outside the menu's (unclamped) region and is treated as an outside-click.
93+
/// Regression: render clamped `menu.position` while the hit-tests used the raw
94+
/// position, so clicking a visible item near the edge silently dismissed the
95+
/// menu instead of activating it.
96+
#[test]
97+
fn plus_button_menu_near_right_edge_clicks_land_on_drawn_items() {
98+
// Narrow bar + overflow pins the "+" (and thus the popup anchor) hard
99+
// against the right edge, where `NEW_TAB_MENU_WIDTH` (18) overflows and
100+
// forces the clamp.
101+
let width: u16 = 50;
102+
let mut harness = EditorTestHarness::new(width, 24).unwrap();
103+
for _ in 0..8 {
104+
harness.new_buffer().unwrap();
105+
}
106+
harness.render().unwrap();
107+
108+
let screen = harness.screen_to_string();
109+
let plus_col = col_of_char_on_row(&screen, 1, '+').unwrap_or_else(|| {
110+
panic!("expected a pinned '+' button on the tab row. Screen:\n{screen}")
111+
});
112+
assert!(
113+
plus_col >= width - 3,
114+
"precondition: '+' pinned near the right edge, got {plus_col}. Screen:\n{screen}"
115+
);
116+
117+
harness.mouse_click(plus_col, 1).unwrap();
118+
harness.assert_screen_contains("New File");
119+
120+
let buffers_before = harness.editor().active_window().buffers.len();
121+
122+
// Click "New File" where it is actually drawn (its clamped position). With
123+
// the raw-position hit-test this click fell outside the menu → dismissed it
124+
// with no new buffer; with the shared clamp it activates the item.
125+
let screen = harness.screen_to_string();
126+
let (nf_col, nf_row) = pos_of_substr(&screen, "New File")
127+
.unwrap_or_else(|| panic!("expected 'New File' item in popup. Screen:\n{screen}"));
128+
harness.mouse_click(nf_col + 1, nf_row).unwrap();
129+
130+
let screen = harness.screen_to_string();
131+
assert!(
132+
!screen.contains("New Terminal"),
133+
"popup should be dismissed after selecting an item. Screen:\n{screen}"
134+
);
135+
assert_eq!(
136+
harness.editor().active_window().buffers.len(),
137+
buffers_before + 1,
138+
"clicking the drawn 'New File' near the edge must create a buffer, \
139+
not miss the item. Screen:\n{screen}"
140+
);
141+
}
142+
90143
#[test]
91144
fn plus_button_menu_captures_keyboard_and_filters_keys() {
92145
let mut harness = EditorTestHarness::new(120, 30).unwrap();

0 commit comments

Comments
 (0)