fix: keep fitting input values visible and record bare Delete/Backspace hotkeys

crates/moon-ui-components/component-manifest.json

@@ -348,7 +348,7 @@
       "public_path": "moon_ui::MoonInput",
       "upstream_ref": "Longbridge::input@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675",
       "fork_reason": "Reviewed TrackedFork drift: Moon tone/theme hooks plus UTF-8, mask, number, and search hardening live in base input; Longbridge editor engine remains source-owned",
-      "donor_drift_budget": 19,
+      "donor_drift_budget": 20,
       "contracts": ["input.utf8_boundary_clamp", "input.mask_contract", "gallery.visual_coverage"]
     },
     {
@@ -732,7 +732,7 @@
       "public_path": "moon_ui::MoonTextArea",
       "upstream_ref": "Longbridge::input@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675",
       "fork_reason": "Reviewed TrackedFork drift: MoonTextArea shares the reviewed Moon input engine drift; Longbridge multiline input behavior remains source-owned",
-      "donor_drift_budget": 19,
+      "donor_drift_budget": 20,
       "contracts": ["gallery.visual_coverage"]
     },
     {

crates/moon-ui-components/src/input/element.rs

@@ -30,6 +30,9 @@ const FOLD_ICON_WIDTH: Pixels = px(14.);
 const FOLD_ICON_HITBOX_WIDTH: Pixels = px(18.);
 const MAX_HIGHLIGHT_LINE_LENGTH: usize = 10_000;
 
+#[cfg(test)]
+mod regression_tests;
+
 #[derive(Clone, Copy, Debug, PartialEq)]
 struct EditorScrollbarLayout {
     bounds: Bounds<Pixels>,
@@ -335,13 +338,15 @@ impl TextElement {
         });
     }
 
-    /// Returns the:
+    /// Lay out the cursor and follow its selection within the supplied viewport.
     ///
     /// - cursor bounds
     /// - scroll offset
     /// - current row index (No only the visible lines, but all lines)
     ///
-    /// This method also will update for track scroll to cursor.
+    /// Mutates `bounds` to the painted text origin and returns the cursor bounds, scroll
+    /// target and current row. Plain single-line inputs never move vertically; fitting
+    /// values return to their horizontal origin before both cursor and text are positioned.
     fn layout_cursor(
         &self,
         last_layout: &LastLayout,
@@ -374,6 +379,20 @@ impl TextElement {
         let mut current_row = None;
         let mut scroll_offset = state.scroll_handle.offset();
         let mut cursor_bounds = None;
+        let single_line = matches!(
+            state.mode,
+            InputMode::PlainText {
+                multi_line: false,
+                ..
+            }
+        );
+        let fits_horizontally = last_layout.lines.first().is_some_and(|line| {
+            line.size(line_height).width + line_number_width <= bounds.size.width
+        });
+        if single_line && fits_horizontally {
+            // A shorter edit or wider viewport no longer needs the previous horizontal offset.
+            scroll_offset.x = px(0.);
+        }
 
         // Padding kept between the cursor and the viewport's top/bottom
         // edges, used by the auto-scroll-into-view computation below.
@@ -474,17 +493,19 @@ impl TextElement {
                     TextAlign::Center => CURSOR_WIDTH,
                 };
 
-                scroll_offset.x = if scroll_offset.x + cursor_pos.x
-                    > (bounds.size.width - line_number_width - safety_margin)
-                {
-                    // cursor is out of right
-                    bounds.size.width - line_number_width - safety_margin - cursor_pos.x
-                } else if scroll_offset.x + cursor_pos.x < px(0.) {
-                    // cursor is out of left
-                    scroll_offset.x - cursor_pos.x
-                } else {
-                    scroll_offset.x
-                };
+                if !(single_line && fits_horizontally) {
+                    scroll_offset.x = if scroll_offset.x + cursor_pos.x
+                        > (bounds.size.width - line_number_width - safety_margin)
+                    {
+                        // cursor is out of right
+                        bounds.size.width - line_number_width - safety_margin - cursor_pos.x
+                    } else if scroll_offset.x + cursor_pos.x < px(0.) {
+                        // cursor is out of left
+                        scroll_offset.x - cursor_pos.x
+                    } else {
+                        scroll_offset.x
+                    };
+                }
 
                 // Vertical cursor-follow is suppressed while auto-scroll manages the y axis,
                 // to prevent fighting the background scroll task.
@@ -562,6 +583,15 @@ impl TextElement {
             bounds.size.height,
         );
 
+        if single_line {
+            // Persistence clamps after paint; enforce the invariant before moving the sole line,
+            // including when a deferred target was computed against an earlier layout.
+            scroll_offset.y = px(0.);
+            if fits_horizontally {
+                scroll_offset.x = px(0.);
+            }
+        }
+
         bounds.origin = bounds.origin + scroll_offset;
 
         (cursor_bounds, scroll_offset, current_row)

crates/moon-ui-components/src/input/element/regression_tests.rs

@@ -0,0 +1,233 @@
+//! Single-line prepaint regressions, including nonzero glyph advances absent from NoopTextSystem.
+
+use std::{cell::RefCell, rc::Rc, sync::Arc};
+
+use gpui::{
+    AppContext as _, Bounds, Context, Entity, FontId, GlyphId, InteractiveElement as _,
+    IntoElement, Modifiers, ParentElement as _, Render, ShapedGlyph, ShapedLine, ShapedRun,
+    Styled as _, TestAppContext, TextAlign, VisualTestContext, Window, point, px, size,
+};
+
+use super::TextElement;
+use crate::{
+    Root, h_flex,
+    input::{InputState, blink_cursor::CURSOR_WIDTH, display_map::LineLayout},
+    moon::{MoonNumberInput, MoonSize, MoonTheme, ThemeMode},
+};
+
+/// Hosts the actual Xs number builder using caller-owned state and an optional unit.
+struct NumberHarness {
+    state: Entity<InputState>,
+    unit: Option<&'static str>,
+}
+
+impl Render for NumberHarness {
+    /// Return the compact numeric field in a content-sized mono row.
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        let mut input = MoonNumberInput::new("scroll-regression")
+            .state(&self.state)
+            .range(0., 256.)
+            .step(1.)
+            .size(MoonSize::Xs);
+        if let Some(unit) = self.unit {
+            input = input.unit(unit);
+        }
+        h_flex()
+            .min_w(px(120.))
+            .font_family("Geist Mono")
+            .debug_selector(|| "number-box".into())
+            .child(input)
+    }
+}
+
+/// Assert text geometry is inside the control before the persisted-offset clamp can hide a shift.
+fn assert_visible(cx: &mut VisualTestContext, input: &Entity<InputState>) {
+    let control = cx.debug_bounds("number-box").expect("number is rendered");
+    cx.update(|_, cx| {
+        let state = input.read(cx);
+        assert_eq!(state.value().as_ref(), "10");
+        assert_eq!(state.scroll_offset(), point(px(0.), px(0.)));
+        let layout = state.last_layout.as_ref().expect("text is laid out");
+        let painted = state.last_bounds.expect("text is painted");
+        assert_eq!(painted.origin, state.input_bounds.origin);
+        assert!(painted.top() >= control.top());
+        assert!(painted.top() + layout.line_height <= control.bottom());
+        let width = layout.lines[0].size(layout.line_height).width;
+        let left = painted.left() + layout.alignment_offset(width);
+        assert!(left >= state.input_bounds.left());
+        assert!(left + width <= state.input_bounds.right());
+    });
+}
+
+/// Catches applying a deferred y target to the sole line before persistence clamps it: the Xs
+/// value disappears despite scroll_offset().y being zero, and must remain visible after blur.
+#[gpui::test]
+fn xs_number_ignores_deferred_vertical_scroll(cx: &mut TestAppContext) {
+    cx.update(crate::init);
+    for theme in [ThemeMode::Dark, ThemeMode::Light] {
+        cx.update(|cx| MoonTheme::set_mode(theme, cx));
+        for unit in [None, Some("ms")] {
+            let slot = Rc::new(RefCell::new(None));
+            let window = cx.add_window({
+                let slot = slot.clone();
+                move |window, cx| {
+                    let state = cx.new(|cx| InputState::new(window, cx).default_value("10"));
+                    *slot.borrow_mut() = Some(state.clone());
+                    let view = cx.new(|_| NumberHarness { state, unit });
+                    Root::new(view, window, cx).bordered(false)
+                }
+            });
+            let input = slot.borrow().clone().expect("input is created");
+            let mut visual = VisualTestContext::from_window(window.into(), cx);
+            visual.run_until_parked();
+            assert_visible(&mut visual, &input);
+            let click = visual.update(|_, cx| input.read(cx).input_bounds.center());
+            visual.simulate_click(click, Modifiers::none());
+            visual.update(|window, cx| assert!(input.read(cx).focus_handle.is_focused(window)));
+            assert_visible(&mut visual, &input);
+            visual.simulate_keystrokes(if cfg!(target_os = "macos") {
+                "cmd-a"
+            } else {
+                "ctrl-a"
+            });
+            visual.simulate_input("10");
+            assert_visible(&mut visual, &input);
+            visual.update(|_, cx| {
+                input.update(cx, |state, cx| {
+                    state.set_scroll_offset(point(px(0.), px(-16.)), cx);
+                })
+            });
+            visual.run_until_parked();
+            assert_visible(&mut visual, &input);
+            visual.update(|window, _| window.blur());
+            visual.run_until_parked();
+            assert_visible(&mut visual, &input);
+        }
+    }
+}
+
+/// Supply fixed glyph advances for ASCII digits, independent of platform fonts.
+/// Returns the same shaped-line representation consumed by production cursor layout.
+fn numeric_line(text: &str, width: f32) -> ShapedLine {
+    let mut line = ShapedLine::default();
+    let layout = Arc::get_mut(&mut line).expect("new shaped layout is uniquely owned");
+    layout.width = px(width);
+    layout.len = text.len();
+    layout.runs = vec![ShapedRun {
+        font_id: FontId(0),
+        glyphs: (0..text.len())
+            .map(|index| ShapedGlyph {
+                id: GlyphId(0),
+                index,
+                position: point(px(index as f32 * width / text.len() as f32), px(0.)),
+                is_emoji: false,
+            })
+            .collect(),
+    }];
+    line.text = text.to_string().into();
+    line
+}
+
+/// Catches stale horizontal follow hiding a fitting centered value after a shorter edit:
+/// in a 60px value box, two 7px glyphs must occupy x=23..37 and the cursor must be at x=37.
+/// Nonzero synthetic advances cover the real arithmetic that NoopTextSystem cannot exercise.
+#[gpui::test]
+fn centered_short_value_resets_stale_scroll_before_paint(cx: &mut TestAppContext) {
+    cx.update(crate::init);
+    let slot = Rc::new(RefCell::new(None));
+    let window = cx.add_window({
+        let slot = slot.clone();
+        move |window, cx| {
+            let input = cx.new(|cx| InputState::new(window, cx).default_value("10"));
+            *slot.borrow_mut() = Some(input.clone());
+            Root::new(input, window, cx).bordered(false)
+        }
+    });
+    let input = slot.borrow().clone().expect("input is created");
+    let mut visual = VisualTestContext::from_window(window.into(), cx);
+    visual.run_until_parked();
+    visual.update(|window, cx| {
+        let mut layout = input
+            .read(cx)
+            .last_layout
+            .clone()
+            .expect("initial layout exists");
+        layout.line_height = px(16.);
+        layout.content_width = px(60.);
+        layout.text_align = TextAlign::Center;
+        layout.lines = Rc::new(vec![
+            LineLayout::new().lines(smallvec::smallvec![numeric_line("10", 14.)]),
+        ]);
+        input.update(cx, |state, _| {
+            state.selected_range = (2..2).into();
+            state.last_selected_range = None;
+            state.deferred_scroll_offset = None;
+            state.scroll_handle.set_offset(point(px(-100.), px(0.)));
+        });
+        let mut bounds = Bounds::new(point(px(0.), px(0.)), size(px(60.), px(16.)));
+        let (cursor, offset, _) = TextElement::new(input.clone()).layout_cursor(
+            &layout,
+            &mut bounds,
+            size(px(14.), px(16.)),
+            window,
+            cx,
+        );
+        assert_eq!(
+            offset.x,
+            px(0.),
+            "fitting centered value must not inherit old scroll"
+        );
+        assert_eq!(bounds.left() + layout.alignment_offset(px(14.)), px(23.));
+        assert_eq!(cursor.expect("cursor is laid out").left(), px(37.));
+        // Twenty 7px digits still follow the cursor. Right clearance comes from the independent
+        // caret-style constant in blink_cursor, whose width differs between platforms.
+        let long_value = "01234567890123456789";
+        layout.lines = Rc::new(vec![
+            LineLayout::new().lines(smallvec::smallvec![numeric_line(long_value, 140.)]),
+        ]);
+        input.update(cx, |state, cx| {
+            state.set_value(long_value, window, cx);
+            state.last_selected_range = None;
+            state.deferred_scroll_offset = None;
+        });
+        let mut bounds = Bounds::new(point(px(0.), px(0.)), size(px(60.), px(16.)));
+        let (cursor, offset, _) = TextElement::new(input.clone()).layout_cursor(
+            &layout,
+            &mut bounds,
+            size(px(140.), px(16.)),
+            window,
+            cx,
+        );
+        assert_eq!(offset.x, px(-80.) - CURSOR_WIDTH);
+        assert_eq!(
+            cursor.expect("long value cursor is laid out").left(),
+            px(60.) - CURSOR_WIDTH
+        );
+        // A 93px value fits a 100px box even though the left-aligned scroll extent includes
+        // a 10px safety margin. It must keep its first glyph at zero and its cursor at 93.
+        layout.text_align = TextAlign::Left;
+        layout.content_width = px(100.);
+        layout.lines = Rc::new(vec![
+            LineLayout::new().lines(smallvec::smallvec![numeric_line("10", 93.)]),
+        ]);
+        input.update(cx, |state, cx| {
+            state.set_value("10", window, cx);
+            state.last_selected_range = None;
+            state.deferred_scroll_offset = None;
+        });
+        let mut bounds = Bounds::new(point(px(0.), px(0.)), size(px(100.), px(16.)));
+        let (cursor, offset, _) = TextElement::new(input.clone()).layout_cursor(
+            &layout,
+            &mut bounds,
+            size(px(103.), px(16.)),
+            window,
+            cx,
+        );
+        assert_eq!(
+            offset.x,
+            px(0.),
+            "cursor safety padding must not hide fitting text"
+        );
+        assert_eq!(cursor.expect("left cursor is laid out").left(), px(93.));
+    });
+}

crates/moon-ui-components/src/moon/hotkey_input.rs

@@ -663,6 +663,9 @@ fn lone_modifier_key(modifiers: Modifiers) -> Option<&'static str> {
     only
 }
 
+/// Classifies a keystroke as a field command or a binding for the current recording state.
+///
+/// Bare Delete and Backspace clear only while idle; recording captures them as bindings.
 pub fn moon_hotkey_capture(stroke: &Keystroke, recording: bool) -> MoonHotkeyCapture {
     let key = stroke.key.as_str();
     let plain = !stroke.modifiers.modified();
@@ -676,7 +679,6 @@ pub fn moon_hotkey_capture(stroke: &Keystroke, recording: bool) -> MoonHotkeyCap
 
     match key {
         "escape" if plain => MoonHotkeyCapture::Cancel,
-        "backspace" | "delete" if plain => MoonHotkeyCapture::Clear,
         key if is_modifier_key_name(key) => MoonHotkeyCapture::WaitForKey,
         _ => MoonHotkeyCapture::Commit(stroke.clone()),
     }