From 77dda3b89d1a469ce00cc9e91fb9caff272536f3 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:21:41 +0200 Subject: [PATCH 1/4] fix(input): keep fitting single-line values visible --- .../moon-ui-components/src/input/element.rs | 56 ++++- .../src/input/element/regression_tests.rs | 232 ++++++++++++++++++ 2 files changed, 275 insertions(+), 13 deletions(-) create mode 100644 crates/moon-ui-components/src/input/element/regression_tests.rs diff --git a/crates/moon-ui-components/src/input/element.rs b/crates/moon-ui-components/src/input/element.rs index 5623f8d..309b751 100644 --- a/crates/moon-ui-components/src/input/element.rs +++ b/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, @@ -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) diff --git a/crates/moon-ui-components/src/input/element/regression_tests.rs b/crates/moon-ui-components/src/input/element/regression_tests.rs new file mode 100644 index 0000000..c7af31d --- /dev/null +++ b/crates/moon-ui-components/src/input/element/regression_tests.rs @@ -0,0 +1,232 @@ +//! 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, 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, + 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) -> 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) { + 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 2px cursor: 140px - 58px = 82px scroll. + 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(-82.)); + assert_eq!( + cursor.expect("long value cursor is laid out").left(), + px(58.) + ); + // 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.)); + }); +} From 7e5ab81b63efdbfaac87cd4b53ef2e83a7590503 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:38:22 +0200 Subject: [PATCH 2/4] test(input): use platform caret width in scroll regression --- .../src/input/element/regression_tests.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/moon-ui-components/src/input/element/regression_tests.rs b/crates/moon-ui-components/src/input/element/regression_tests.rs index c7af31d..92ce70f 100644 --- a/crates/moon-ui-components/src/input/element/regression_tests.rs +++ b/crates/moon-ui-components/src/input/element/regression_tests.rs @@ -11,7 +11,7 @@ use gpui::{ use super::TextElement; use crate::{ Root, h_flex, - input::{InputState, display_map::LineLayout}, + input::{InputState, blink_cursor::CURSOR_WIDTH, display_map::LineLayout}, moon::{MoonNumberInput, MoonSize, MoonTheme, ThemeMode}, }; @@ -179,7 +179,8 @@ fn centered_short_value_resets_stale_scroll_before_paint(cx: &mut TestAppContext ); 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 2px cursor: 140px - 58px = 82px scroll. + // 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.)]), @@ -197,10 +198,10 @@ fn centered_short_value_resets_stale_scroll_before_paint(cx: &mut TestAppContext window, cx, ); - assert_eq!(offset.x, px(-82.)); + assert_eq!(offset.x, px(-80.) - CURSOR_WIDTH); assert_eq!( cursor.expect("long value cursor is laid out").left(), - px(58.) + 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. From eaed27fe389dae0501043884a56cf2ec78fa16c6 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:39:37 +0200 Subject: [PATCH 3/4] chore(input): refresh reviewed mirror metadata Regenerate only the input and text_area mirror entries for the input fix and regression tests. The new sibling test module adds one donor-different file, so raise only those two reviewed drift budgets from 19 to 20 as authorized by the coordinator. --- .../component-manifest.json | 4 ++-- docs/component-mirror-baseline.json | 24 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/crates/moon-ui-components/component-manifest.json b/crates/moon-ui-components/component-manifest.json index 58e61ee..157759c 100644 --- a/crates/moon-ui-components/component-manifest.json +++ b/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"] }, { diff --git a/docs/component-mirror-baseline.json b/docs/component-mirror-baseline.json index 0f03821..8625a2a 100644 --- a/docs/component-mirror-baseline.json +++ b/docs/component-mirror-baseline.json @@ -437,7 +437,7 @@ "local_paths": [ "input" ], - "local_hash": "5c656e0ef4caf7de", + "local_hash": "f3a029b50a9fa93c", "local_files": [ { "path": "input/blink_cursor.rs", @@ -496,8 +496,13 @@ }, { "path": "input/element.rs", - "hash": "511c756aa3009759", - "bytes": 96171 + "hash": "a3ec163629b2e82f", + "bytes": 97481 + }, + { + "path": "input/element/regression_tests.rs", + "hash": "bec5af9ae7e1324d", + "bytes": 9830 }, { "path": "input/indent.rs", @@ -809,6 +814,7 @@ "input/display_map/display_map.rs", "input/display_map/text_wrapper.rs", "input/element.rs", + "input/element/regression_tests.rs", "input/input.rs", "input/lsp/document_colors.rs", "input/lsp/hover.rs", @@ -1506,7 +1512,7 @@ "local_paths": [ "input" ], - "local_hash": "5c656e0ef4caf7de", + "local_hash": "f3a029b50a9fa93c", "local_files": [ { "path": "input/blink_cursor.rs", @@ -1565,8 +1571,13 @@ }, { "path": "input/element.rs", - "hash": "511c756aa3009759", - "bytes": 96171 + "hash": "a3ec163629b2e82f", + "bytes": 97481 + }, + { + "path": "input/element/regression_tests.rs", + "hash": "bec5af9ae7e1324d", + "bytes": 9830 }, { "path": "input/indent.rs", @@ -1878,6 +1889,7 @@ "input/display_map/display_map.rs", "input/display_map/text_wrapper.rs", "input/element.rs", + "input/element/regression_tests.rs", "input/input.rs", "input/lsp/document_colors.rs", "input/lsp/hover.rs", From c25c81818804ae56dc6c63d5e6b214b7d7937209 Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:37:13 +0200 Subject: [PATCH 4/4] fix(hotkey): record bare Delete and Backspace bindings --- .../src/moon/hotkey_input.rs | 4 +- .../src/moon/hotkey_input/tests.rs | 95 ++++++++++++++++++- crates/moon-ui-components/src/moon/kbd.rs | 3 +- .../moon-ui-components/src/moon/kbd/tests.rs | 19 ++++ 4 files changed, 116 insertions(+), 5 deletions(-) diff --git a/crates/moon-ui-components/src/moon/hotkey_input.rs b/crates/moon-ui-components/src/moon/hotkey_input.rs index de19285..72fa95b 100644 --- a/crates/moon-ui-components/src/moon/hotkey_input.rs +++ b/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()), } diff --git a/crates/moon-ui-components/src/moon/hotkey_input/tests.rs b/crates/moon-ui-components/src/moon/hotkey_input/tests.rs index 59f6388..ac094b8 100644 --- a/crates/moon-ui-components/src/moon/hotkey_input/tests.rs +++ b/crates/moon-ui-components/src/moon/hotkey_input/tests.rs @@ -4,6 +4,79 @@ use gpui::{Capslock, Keystroke, Modifiers}; use super::{MoonHotkeyCapture, MoonHotkeyModifierWatch, moon_hotkey_capture}; +/// Renders the production field at fixed bounds and records its change callbacks. +struct HotkeyHarness { + changes: std::rc::Rc>>>, +} + +impl gpui::Render for HotkeyHarness { + /// Returns a field whose fixed bounds let the test press its body and clear button. + fn render( + &mut self, + _window: &mut gpui::Window, + _cx: &mut gpui::Context, + ) -> impl gpui::IntoElement { + let changes = self.changes.clone(); + super::MoonHotkeyInput::new("hotkey-harness") + .bounds(crate::moon::MoonRect::new(0.0, 0.0, 240.0, 30.0)) + .default_value(Keystroke::parse("ctrl-k").unwrap()) + .on_change(move |value, _, _| changes.borrow_mut().push(value)) + } +} + +/// Catches disconnecting capture from the rendered field or the clear button: deletion keys +/// must record after activation, clear while idle, and remain recordable after clicking clear. +#[gpui::test] +fn hotkey_input_records_and_clears_through_rendered_handlers(cx: &mut gpui::TestAppContext) { + cx.update(crate::init); + let changes = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + let window = cx.add_window({ + let changes = changes.clone(); + move |_, _| HotkeyHarness { changes } + }); + let mut cx = gpui::VisualTestContext::from_window(window.into(), cx); + cx.update(|window, _| window.refresh()); + cx.run_until_parked(); + + for key in ["delete", "backspace"] { + cx.simulate_click( + gpui::point(gpui::px(20.0), gpui::px(15.0)), + Modifiers::none(), + ); + cx.simulate_keystrokes(key); + cx.run_until_parked(); + assert_eq!( + changes.borrow().last(), + Some(&Some(Keystroke::parse(key).unwrap())) + ); + + // Clearing during recording must stop it without bubbling back into body activation. + cx.simulate_click( + gpui::point(gpui::px(20.0), gpui::px(15.0)), + Modifiers::none(), + ); + cx.simulate_click( + gpui::point(gpui::px(222.0), gpui::px(15.0)), + Modifiers::none(), + ); + cx.run_until_parked(); + assert_eq!(changes.borrow().last(), Some(&None)); + + cx.simulate_keystrokes("enter"); + cx.simulate_keystrokes(key); + cx.run_until_parked(); + assert_eq!( + changes.borrow().last(), + Some(&Some(Keystroke::parse(key).unwrap())) + ); + + cx.simulate_keystrokes(key); + cx.run_until_parked(); + assert_eq!(changes.borrow().last(), Some(&None)); + } + assert_eq!(changes.borrow().len(), 8); +} + /// Catches handling ordinary chords while idle in `hotkey_input.rs:moon_hotkey_capture`, which /// would steal global shortcuts before the user activates the field. #[test] @@ -54,9 +127,9 @@ fn hotkey_input_commits_full_chord_while_recording() { } /// Catches merging cancel and clear handling in `hotkey_input.rs:moon_hotkey_capture`, which -/// would either erase a shortcut on Escape or leave Delete unable to clear it. +/// would either erase a shortcut on Escape or leave idle Delete unable to clear it. #[test] -fn hotkey_input_escape_cancels_and_delete_clears() { +fn hotkey_input_escape_cancels_and_idle_delete_clears() { let escape = Keystroke::parse("escape").unwrap(); let delete = Keystroke::parse("delete").unwrap(); let backspace = Keystroke::parse("backspace").unwrap(); @@ -64,13 +137,29 @@ fn hotkey_input_escape_cancels_and_delete_clears() { moon_hotkey_capture(&escape, true), MoonHotkeyCapture::Cancel ); - assert_eq!(moon_hotkey_capture(&delete, true), MoonHotkeyCapture::Clear); + assert_eq!( + moon_hotkey_capture(&delete, false), + MoonHotkeyCapture::Clear + ); assert_eq!( moon_hotkey_capture(&backspace, false), MoonHotkeyCapture::Clear ); } +/// Catches treating bare deletion keys as Clear during recording, preventing factory bindings +/// from being entered again after a user clears them. +#[test] +fn hotkey_input_records_bare_delete_and_backspace() { + for source in ["delete", "backspace"] { + let stroke = Keystroke::parse(source).unwrap(); + assert_eq!( + moon_hotkey_capture(&stroke, true), + MoonHotkeyCapture::Commit(stroke) + ); + } +} + /// Catches classifying modified control keys as field controls in /// `hotkey_input.rs:moon_hotkey_capture`, which would prevent users from recording those chords. #[test] diff --git a/crates/moon-ui-components/src/moon/kbd.rs b/crates/moon-ui-components/src/moon/kbd.rs index 49ad607..64d2415 100644 --- a/crates/moon-ui-components/src/moon/kbd.rs +++ b/crates/moon-ui-components/src/moon/kbd.rs @@ -63,6 +63,7 @@ impl MoonKbd { Self::new(Self::format_keystroke(&stroke)) } + /// Formats a keystroke with platform labels, distinguishing forward-delete from backspace. pub fn format_keystroke(key: &Keystroke) -> String { #[cfg(target_os = "macos")] const SEPARATOR: &str = ""; @@ -140,7 +141,7 @@ impl MoonKbd { #[cfg(not(target_os = "macos"))] "backspace" => keys.push_str("Backspace"), #[cfg(target_os = "macos")] - "delete" => keys.push('⌫'), + "delete" => keys.push('⌦'), #[cfg(not(target_os = "macos"))] "delete" => keys.push_str("Delete"), #[cfg(target_os = "macos")] diff --git a/crates/moon-ui-components/src/moon/kbd/tests.rs b/crates/moon-ui-components/src/moon/kbd/tests.rs index e39400a..743c4af 100644 --- a/crates/moon-ui-components/src/moon/kbd/tests.rs +++ b/crates/moon-ui-components/src/moon/kbd/tests.rs @@ -3,6 +3,25 @@ use super::{MoonKbd, MoonKbdSize}; use gpui::Keystroke; +/// Catches conflating forward-delete and backspace labels, making distinct bindings look alike. +#[test] +fn kbd_distinguishes_delete_and_backspace() { + #[cfg(target_os = "macos")] + let cases = [("delete", "⌦"), ("backspace", "⌫"), ("shift-delete", "⇧⌦")]; + #[cfg(not(target_os = "macos"))] + let cases = [ + ("delete", "Delete"), + ("backspace", "Backspace"), + ("shift-delete", "Shift+Delete"), + ]; + for (source, expected) in cases { + assert_eq!( + MoonKbd::format_keystroke(&Keystroke::parse(source).unwrap()), + expected + ); + } +} + /// Catches using pointer-target height or legacy font scaling, which enlarges shortcut chips. #[test] fn kbd_metrics_match_designer_reference() {