Merge pull request #7 from oposs/fix/form-scroll-and-window-layout

fix(ui): reveal read-only form fields and stop window overlapping the footer

Author: oetiker

CHANGES.md

@@ -12,6 +12,21 @@ All notable changes to eDAPtor are documented here. The format follows
 
 ### Fixed
 
+- **The main window no longer sits one row too low with its bottom edge overlaid
+  by the footer.** The window's bounds were built in screen-absolute coordinates
+  but applied as desktop-local ones, shifting the whole window down a row — a
+  blank strip appeared under the menu bar and the window's bottom border landed on
+  the status-line row. The window is now positioned in desktop-local space, so it
+  lines up correctly.
+- **Read-only fields at the top or bottom of a long entry form can now be
+  scrolled into view.** Read-only fields cannot take focus, and the entry form's
+  scroll follows the focused field — so a run of read-only fields *below* the
+  last editable field (or *above* the first) used to be stranded off-screen on a
+  form taller than the pane: the last field was never visible. Up/Down on the
+  first/last editable field now walks the viewport through that read-only
+  head/tail one line at a time before focus moves on, and the reveal stays put
+  instead of snapping back.
+
 ## 1.5.0 - 2026-07-24
 
 ### Changed

Cargo.lock

@@ -1055,7 +1055,17 @@ fn init_desktop(r: Rect, state: Shared) -> Option<Box<dyn View>> {
     r.b.y -= 1; // above status line
     let mut desktop = Desktop::new(r, |br| Some(Desktop::init_background(br)));
 
-    let win_rect = Rect::new(r.a.x + 1, r.a.y, r.b.x - 1, r.b.y);
+    // The window is a desktop CHILD, so its bounds are in desktop-LOCAL
+    // coordinates (relative to the desktop's own origin at screen row `r.a.y`),
+    // NOT screen-absolute. The desktop's local extent is `(0, 0, width, height)`.
+    // Reusing the absolute `r.a.y` / `r.b.y` here treated row 1 as a local offset,
+    // shifting the whole window one row down: a blank row appeared under the menu
+    // and the window's bottom edge landed on the footer row. Build the rect in
+    // local space — fill the desktop, inset one column left/right (the former
+    // horizontal inset) — so it lines up before the pump flips it to fullscreen.
+    let win_w = r.b.x - r.a.x;
+    let win_h = r.b.y - r.a.y;
+    let win_rect = Rect::new(1, 0, win_w - 1, win_h);
     let mut win = Window::new(win_rect, Some("edaptor".to_string()), 1);
     // No drop shadow: as a desktop-filling frameless window it has nothing to cast
     // onto, and the shadow would otherwise paint a one-cell strip over the desktop
@@ -1360,4 +1370,84 @@ mod tests {
             "an empty attribute must not be sent"
         );
     }
+
+    /// The 80×24 headless screen rows (index 0 = menu row) of a freshly built
+    /// program, pumped a few times WITH events queued so no idle pass fires — i.e.
+    /// the pre-fullscreen bordered window the base `win_rect` governs.
+    #[cfg(test)]
+    fn bordered_screen_rows() -> Vec<String> {
+        use crate::ldap::worker::RawSubschema;
+        use crate::schema::SchemaModel;
+        use crate::ui::state::UiState;
+        use crate::workflows::structure::Structure;
+        let structure = Structure::build("dc=x", vec![]);
+        let schema = SchemaModel::from_raw(&RawSubschema::default());
+        let st = UiState::new_for_test(structure, schema, "dc=x".into(), Vec::new(), Vec::new());
+        let shared: Shared = std::rc::Rc::new(std::cell::RefCell::new(st));
+
+        let (backend, handle) = tv::HeadlessBackend::new(80, 24);
+        let mut prog = build_program(Box::new(backend), shared);
+        // Keep the event queue non-empty every pass so no idle pass fires the pump's
+        // fullscreen timer: this pins the bordered layout the base win_rect drives.
+        for _ in 0..5 {
+            handle.push_event(tv::Event::KeyDown(tv::KeyEvent::from(tv::Key::F(9))));
+            prog.pump_once();
+        }
+        // Extract the `text:` block of the snapshot: 24 rows each framed by `|…|`.
+        handle
+            .snapshot()
+            .lines()
+            .skip_while(|l| *l != "text:")
+            .skip(1)
+            .take_while(|l| l.starts_with('|'))
+            .map(|l| l.trim_start_matches('|').trim_end_matches('|').to_string())
+            .collect()
+    }
+
+    /// Regression: the main window must sit in desktop-LOCAL coordinates. A
+    /// screen-absolute `win_rect` shifted it one row down — leaving a blank
+    /// desktop row under the menu and pushing the window's bottom border onto the
+    /// footer row. Assert the window frame starts on row 1 (right under the menu)
+    /// and the footer alone occupies the last row.
+    #[test]
+    fn main_window_is_not_shifted_onto_the_footer() {
+        let rows = bordered_screen_rows();
+        assert_eq!(rows.len(), 24, "80x24 screen has 24 text rows");
+
+        // Row 0 is the menu bar.
+        assert!(
+            rows[0].contains("File"),
+            "row 0 is the menu bar: {:?}",
+            rows[0]
+        );
+
+        // Row 1 must be the window's top border (frame glyphs), NOT a blank desktop
+        // row. Before the fix this row was all background (`░`) + spaces.
+        assert!(
+            rows[1].contains('═') || rows[1].contains("edaptor"),
+            "row 1 must be the window top border, not a blank gap under the menu: {:?}",
+            rows[1]
+        );
+
+        // The last row is the footer/status line, and it must not be overlaid by
+        // the window's bottom border — no box-drawing frame glyphs on it.
+        let footer = &rows[23];
+        assert!(
+            footer.contains("Alt-N"),
+            "last row is the status line: {:?}",
+            footer
+        );
+        assert!(
+            !footer.contains('═') && !footer.contains('┘') && !footer.contains('╝'),
+            "the window bottom border must not overlap the footer row: {:?}",
+            footer
+        );
+
+        // The window's bottom border lives on the second-to-last row instead.
+        assert!(
+            rows[22].contains('┘') || rows[22].contains('═'),
+            "row 22 holds the window bottom border: {:?}",
+            rows[22]
+        );
+    }
 }

src/ui/app.rs

@@ -786,6 +786,13 @@ impl FormPane {
             self.group.focus_child(scroll_id, ctx);
             if let Some(sg) = self.scroll_mut() {
                 sg.focus_child(id, ctx);
+                // Actively snap the landed field into view. The per-event settle
+                // (region-aware) would otherwise leave a first/last focusable field
+                // off-screen on open when it sits below a tall read-only head (its
+                // whole region is a scroll fixed point), so land it explicitly.
+                if let Some(logical) = sg.logical_of(id) {
+                    sg.ensure_visible(logical, ctx);
+                }
             }
             // Always land with the caret homed to the start of the value (not the
             // select-all-to-end block Turbo Vision leaves on focus). This covers a
@@ -860,12 +867,22 @@ impl FormPane {
             None => ids.len() - 1,
         };
         let next_id = ids[next];
+        let moved = cur != Some(next_id);
         if let Some(sg) = self.scroll_mut() {
             sg.focus_child(next_id, ctx);
-            // Scroll immediately so the newly focused field — and thus the
-            // hardware cursor — is on screen this frame, not one pump tick later.
-            if let Some(logical) = sg.logical_of(next_id) {
-                sg.ensure_visible(logical, ctx);
+            if moved {
+                // Focus actually advanced: snap the newly focused field fully into
+                // view this frame (so the hardware cursor is on screen now, not one
+                // pump tick later).
+                if let Some(logical) = sg.logical_of(next_id) {
+                    sg.ensure_visible(logical, ctx);
+                }
+            } else {
+                // Clamped at an end (Down on the last / Up on the first focusable
+                // field): don't re-snap to the bare field rect — that would yank a
+                // deliberately scrolled-in read-only head/tail back off screen. Use
+                // the region-aware settle, which leaves an in-band scroll alone.
+                sg.ensure_focused_visible(ctx);
             }
         }
         // Enter the field with the caret at the start, not a select-all block.
@@ -1374,8 +1391,21 @@ impl View for FormPane {
                 self.group.handle_event(ev, ctx);
             }
         } else if nav {
+            // Plain (non-list, non-launch) focused field. If it is the FIRST or
+            // LAST focusable field and the form has a non-focusable read-only
+            // head/tail, Up/Down first walks the viewport through that hidden
+            // region (read-only cells never take focus, so this is the only way to
+            // bring a trailing/leading read-only field into view); only once the
+            // region's near edge is visible does focus advance to the prev/next
+            // field.
             let down = matches!(ev, Event::KeyDown(k) if k.key == Key::Down);
-            self.focus_field(if down { 1 } else { -1 }, ctx);
+            let scrolled = self
+                .scroll_mut()
+                .map(|sg| sg.scroll_focus_region_edge(down, 1, ctx))
+                .unwrap_or(false);
+            if !scrolled {
+                self.focus_field(if down { 1 } else { -1 }, ctx);
+            }
             ev.clear();
         } else if is_keydown && self.focused_is_launch_view() {
             // Action key on a read-only launch/list block: route it to the focused
@@ -2610,6 +2640,139 @@ mod tests {
         );
     }
 
+    #[test]
+    fn trailing_readonly_tail_scrolls_into_view() {
+        // Reported bug: read-only fields can never take focus, and the form's
+        // scroll anchors on the focused field — so a run of read-only fields
+        // BELOW the last focusable field (past the last visible row of a long
+        // form) can never be scrolled into view. Pressing Down on the last
+        // focusable field must walk the viewport through that trailing read-only
+        // tail one line at a time, instead of clamping in place.
+        let mut fields = vec![ef("cn", "z", true)]; // the only focusable field
+        for i in 0..10 {
+            fields.push(ef(&format!("ro{i}"), &format!("v{i}"), false));
+        }
+        let (_shared, mut pane) = build_pane_with_form(fields);
+        // 6-row viewport (rows 2..8); content is 11 rows → max_top 5.
+        <FormPane as View>::change_bounds(&mut pane, Rect::new(0, 0, 80, 8));
+        let mut out = VecDeque::new();
+        let mut timers = tv::timer::TimerQueue::new();
+        let mut deferred = Vec::new();
+        let mut tick = Event::Broadcast {
+            command: REFRESH,
+            source: None,
+        };
+        pane.handle_event(
+            &mut tick,
+            &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+        );
+
+        assert_eq!(
+            pane.focusable_value_ids().len(),
+            1,
+            "cn is the only focusable field; the ro* tail is read-only"
+        );
+        let max_top = pane.scroll_mut().unwrap().max_top();
+        assert_eq!(max_top, 5, "content (11) overflows the 6-row viewport");
+
+        // Down past the last focusable field scrolls the read-only tail into view
+        // one line at a time (before the fix this did nothing).
+        for expected_top in 1..=max_top {
+            let mut d = Event::KeyDown(tv::KeyEvent::from(tv::Key::Down));
+            pane.handle_event(
+                &mut d,
+                &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+            );
+            assert_eq!(
+                pane.scroll_mut().unwrap().top_for_test(),
+                expected_top,
+                "Down scrolls the read-only tail into view one line at a time"
+            );
+        }
+
+        // A passive event (a REFRESH tick) must NOT yank the scroll back to the
+        // focused field: the tail stays revealed.
+        let mut tick2 = Event::Broadcast {
+            command: REFRESH,
+            source: None,
+        };
+        pane.handle_event(
+            &mut tick2,
+            &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+        );
+        assert_eq!(
+            pane.scroll_mut().unwrap().top_for_test(),
+            max_top,
+            "the revealed tail is stable across passive re-anchor events"
+        );
+
+        // At the bottom, further Down is a no-op (nothing left to reveal).
+        let mut d = Event::KeyDown(tv::KeyEvent::from(tv::Key::Down));
+        pane.handle_event(
+            &mut d,
+            &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+        );
+        assert_eq!(pane.scroll_mut().unwrap().top_for_test(), max_top);
+    }
+
+    #[test]
+    fn leading_readonly_head_scrolls_into_view() {
+        // Symmetric to the trailing tail: a run of read-only fields ABOVE the
+        // first focusable field. On open the focused field must be visible (the
+        // scroll snaps to show it, head partly above), and Up must then walk the
+        // viewport up through the read-only head one line at a time.
+        let mut fields: Vec<EditField> = (0..10)
+            .map(|i| ef(&format!("ro{i}"), &format!("v{i}"), false))
+            .collect();
+        fields.push(ef("cn", "z", true)); // the only focusable field, at the bottom
+        let (_shared, mut pane) = build_pane_with_form(fields);
+        // 6-row viewport (rows 2..8); content is 11 rows → max_top 5.
+        <FormPane as View>::change_bounds(&mut pane, Rect::new(0, 0, 80, 8));
+        let mut out = VecDeque::new();
+        let mut timers = tv::timer::TimerQueue::new();
+        let mut deferred = Vec::new();
+        let mut tick = Event::Broadcast {
+            command: REFRESH,
+            source: None,
+        };
+        pane.handle_event(
+            &mut tick,
+            &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+        );
+
+        assert_eq!(pane.focusable_value_ids().len(), 1, "only cn is focusable");
+        // cn is the last content row (row 10); to show it the 6-row viewport must
+        // scroll to top=5 (rows 5..11) on open — not sit at top=0 hiding it.
+        assert_eq!(
+            pane.scroll_mut().unwrap().top_for_test(),
+            5,
+            "the focused field is snapped into view on open, not hidden by the head"
+        );
+
+        // Up past the first focusable field walks the read-only head into view one
+        // line at a time (top counts back down to 0).
+        for expected_top in (0..=4).rev() {
+            let mut u = Event::KeyDown(tv::KeyEvent::from(tv::Key::Up));
+            pane.handle_event(
+                &mut u,
+                &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+            );
+            assert_eq!(
+                pane.scroll_mut().unwrap().top_for_test(),
+                expected_top,
+                "Up scrolls the read-only head into view one line at a time"
+            );
+        }
+
+        // At the top, further Up is a no-op.
+        let mut u = Event::KeyDown(tv::KeyEvent::from(tv::Key::Up));
+        pane.handle_event(
+            &mut u,
+            &mut headless_ctx(&mut out, &mut timers, &mut deferred),
+        );
+        assert_eq!(pane.scroll_mut().unwrap().top_for_test(), 0);
+    }
+
     #[test]
     fn short_launch_block_advances_focus_immediately() {
         // A Launch block that already fits the viewport must behave exactly as