Skip to content

Commit ba49160

Browse files
harryalbertoz-agent
andcommitted
emit LayoutChanged when selector mutations affect rendered height
Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent 986a9f0 commit ba49160

2 files changed

Lines changed: 129 additions & 21 deletions

File tree

crates/warp_tui/src/option_selector.rs

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ pub(crate) enum TuiOptionSelectorEvent {
5252
/// The selector asked to be dismissed (element-level Escape fallback for
5353
/// hosts without their own Escape binding).
5454
Dismissed,
55+
/// Something that affects the selector's rendered height changed: the
56+
/// viewport scrolled (overflow markers), the row catalog refreshed, or
57+
/// the custom-text error row toggled. Hosts whose measured height is
58+
/// cached re-measure on this event.
59+
LayoutChanged,
5560
}
5661

5762
/// User interactions dispatched from the selector's element tree.
@@ -164,6 +169,8 @@ impl TuiOptionSelector {
164169
.or_else(|| self.snapshot.selected_id.clone());
165170
self.highlight_id(target);
166171
self.sync_after_items_changed();
172+
// A refreshed catalog can change the row count and thus the height.
173+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
167174
ctx.notify();
168175
}
169176

@@ -173,6 +180,26 @@ impl TuiOptionSelector {
173180
self.custom_text.is_some()
174181
}
175182

183+
/// Scrolls to keep `selected` visible, announcing the scroll change (it
184+
/// toggles overflow markers, so the height may change) to the host.
185+
fn scroll_to_keep_visible(
186+
&mut self,
187+
items_len: usize,
188+
selected: usize,
189+
ctx: &mut ViewContext<Self>,
190+
) {
191+
let before = self.scroll_offset;
192+
keep_selected_visible(
193+
items_len,
194+
selected,
195+
MAX_VISIBLE_OPTION_ROWS,
196+
&mut self.scroll_offset,
197+
);
198+
if self.scroll_offset != before {
199+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
200+
}
201+
}
202+
176203
/// Confirms the highlighted item (Enter): enabled rows emit
177204
/// [`TuiOptionSelectorEvent::Confirmed`]; disabled rows are kept
178205
/// highlighted so their reason stays visible. While the
@@ -193,7 +220,10 @@ impl TuiOptionSelector {
193220
/// editing and reports whether the key was consumed, so the card only
194221
/// leaves the page when the selector had nothing to unwind.
195222
pub(crate) fn handle_back(&mut self, ctx: &mut ViewContext<Self>) -> bool {
196-
if self.custom_text.take().is_some() {
223+
if let Some(editor) = self.custom_text.take() {
224+
if editor.error.is_some() {
225+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
226+
}
197227
ctx.notify();
198228
return true;
199229
}
@@ -291,12 +321,7 @@ impl TuiOptionSelector {
291321
self.selection.select_previous(items_len, |_| true);
292322
}
293323
if let Some(selected) = self.selection.selected_index() {
294-
keep_selected_visible(
295-
items_len,
296-
selected,
297-
MAX_VISIBLE_OPTION_ROWS,
298-
&mut self.scroll_offset,
299-
);
324+
self.scroll_to_keep_visible(items_len, selected, ctx);
300325
}
301326
ctx.notify();
302327
}
@@ -309,12 +334,7 @@ impl TuiOptionSelector {
309334
return;
310335
};
311336
self.selection.select(index, items.len(), |_| true);
312-
keep_selected_visible(
313-
items.len(),
314-
index,
315-
MAX_VISIBLE_OPTION_ROWS,
316-
&mut self.scroll_offset,
317-
);
337+
self.scroll_to_keep_visible(items.len(), index, ctx);
318338
if !self.item_is_confirmable(item) {
319339
ctx.notify();
320340
return;
@@ -344,8 +364,14 @@ impl TuiOptionSelector {
344364
};
345365
let value = editor.buffer.trim().to_string();
346366
if value.is_empty() {
347-
editor.error = Some(CUSTOM_TEXT_EMPTY_ERROR.to_string());
367+
if editor.error.is_none() {
368+
editor.error = Some(CUSTOM_TEXT_EMPTY_ERROR.to_string());
369+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
370+
}
348371
} else {
372+
if editor.error.is_some() {
373+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
374+
}
349375
self.custom_text = None;
350376
self.snapshot.selected_id = Some(value.clone());
351377
self.custom_text_value = Some(value.clone());
@@ -359,10 +385,14 @@ impl TuiOptionSelector {
359385
fn scroll_by(&mut self, rows: isize, ctx: &mut ViewContext<Self>) {
360386
let items_len = self.items().len();
361387
let max_offset = items_len.saturating_sub(MAX_VISIBLE_OPTION_ROWS);
388+
let before = self.scroll_offset;
362389
self.scroll_offset = self
363390
.scroll_offset
364391
.saturating_add_signed(rows)
365392
.min(max_offset);
393+
if self.scroll_offset != before {
394+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
395+
}
366396
ctx.notify();
367397
}
368398

@@ -661,14 +691,18 @@ impl TypedActionView for TuiOptionSelector {
661691
TuiOptionSelectorAction::InsertChar(c) => {
662692
if let Some(editor) = &mut self.custom_text {
663693
editor.buffer.push(*c);
664-
editor.error = None;
694+
if editor.error.take().is_some() {
695+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
696+
}
665697
ctx.notify();
666698
}
667699
}
668700
TuiOptionSelectorAction::Backspace => {
669701
if let Some(editor) = &mut self.custom_text {
670702
editor.buffer.pop();
671-
editor.error = None;
703+
if editor.error.take().is_some() {
704+
ctx.emit(TuiOptionSelectorEvent::LayoutChanged);
705+
}
672706
ctx.notify();
673707
}
674708
}

crates/warp_tui/src/option_selector_tests.rs

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,17 @@ fn header() -> OptionSelectorHeader {
6464

6565
type CapturedEvents = Rc<RefCell<Vec<TuiOptionSelectorEvent>>>;
6666

67+
/// The captured events with `LayoutChanged` filtered out, for tests that
68+
/// assert on the primary confirmation flow.
69+
fn primary_events(events: &CapturedEvents) -> Vec<TuiOptionSelectorEvent> {
70+
events
71+
.borrow()
72+
.iter()
73+
.filter(|event| **event != TuiOptionSelectorEvent::LayoutChanged)
74+
.cloned()
75+
.collect()
76+
}
77+
6778
/// Adds a selector in a fresh TUI window and captures its emitted events.
6879
fn add_selector(app: &mut App) -> (ViewHandle<TuiOptionSelector>, CapturedEvents) {
6980
app.add_singleton_model(|_| Appearance::mock());
@@ -285,7 +296,7 @@ fn digits_are_viewport_relative_in_scrolled_lists() {
285296
TuiOptionSelectorAction::SelectNumberedOption(1),
286297
);
287298
assert_eq!(
288-
events.borrow().as_slice(),
299+
primary_events(&events),
289300
[TuiOptionSelectorEvent::Confirmed {
290301
id: "row-2".to_string()
291302
}],
@@ -438,7 +449,7 @@ fn custom_text_editor_trims_validates_and_submits() {
438449
TuiOptionSelectorAction::InsertChar(' '),
439450
);
440451
confirm(&mut app, &selector);
441-
assert!(events.borrow().is_empty());
452+
assert!(primary_events(&events).is_empty());
442453
assert!(render_lines(&app, &selector, 60)
443454
.iter()
444455
.any(|line| line.contains("Enter a value to continue.")));
@@ -450,7 +461,7 @@ fn custom_text_editor_trims_validates_and_submits() {
450461
act(&mut app, &selector, TuiOptionSelectorAction::Backspace);
451462
confirm(&mut app, &selector);
452463
assert_eq!(
453-
events.borrow().as_slice(),
464+
primary_events(&events),
454465
[TuiOptionSelectorEvent::CustomTextSubmitted {
455466
value: "my-host".to_string()
456467
}],
@@ -503,6 +514,69 @@ fn create_new_auth_secret_footer_is_ignored() {
503514
});
504515
}
505516

517+
#[test]
518+
fn layout_changed_is_emitted_only_when_overflow_markers_toggle() {
519+
App::test((), |mut app| async move {
520+
let (selector, events) = add_selector(&mut app);
521+
set_page(
522+
&mut app,
523+
&selector,
524+
snapshot(&["a", "b", "c", "d", "e", "f"], Some("a")),
525+
);
526+
events.borrow_mut().clear();
527+
528+
// Moves within the viewport do not scroll, so nothing is emitted.
529+
for _ in 0..3 {
530+
act(&mut app, &selector, TuiOptionSelectorAction::MoveDown);
531+
}
532+
assert!(!events
533+
.borrow()
534+
.contains(&TuiOptionSelectorEvent::LayoutChanged));
535+
536+
// Scrolling past the viewport reveals the `↑` marker: one event.
537+
act(&mut app, &selector, TuiOptionSelectorAction::MoveDown);
538+
assert_eq!(
539+
events
540+
.borrow()
541+
.iter()
542+
.filter(|event| **event == TuiOptionSelectorEvent::LayoutChanged)
543+
.count(),
544+
1,
545+
);
546+
});
547+
}
548+
549+
#[test]
550+
fn layout_changed_is_emitted_when_the_custom_text_error_row_toggles() {
551+
App::test((), |mut app| async move {
552+
let (selector, events) = add_selector(&mut app);
553+
let mut with_footer = snapshot(&["warp"], Some("warp"));
554+
with_footer.footer = Some(OptionFooter::CustomText {
555+
label: "Custom host…".to_string(),
556+
});
557+
set_page(&mut app, &selector, with_footer);
558+
act(&mut app, &selector, TuiOptionSelectorAction::SelectItem(1));
559+
events.borrow_mut().clear();
560+
561+
// An empty submit adds the validation-error row.
562+
confirm(&mut app, &selector);
563+
assert!(events
564+
.borrow()
565+
.contains(&TuiOptionSelectorEvent::LayoutChanged));
566+
events.borrow_mut().clear();
567+
568+
// Typing clears the error row.
569+
act(
570+
&mut app,
571+
&selector,
572+
TuiOptionSelectorAction::InsertChar('x'),
573+
);
574+
assert!(events
575+
.borrow()
576+
.contains(&TuiOptionSelectorEvent::LayoutChanged));
577+
});
578+
}
579+
506580
#[test]
507581
fn snapshot_refresh_preserves_the_highlighted_row() {
508582
App::test((), |mut app| async move {
@@ -517,7 +591,7 @@ fn snapshot_refresh_preserves_the_highlighted_row() {
517591
});
518592
confirm(&mut app, &selector);
519593
assert_eq!(
520-
events.borrow().as_slice(),
594+
primary_events(&events),
521595
[TuiOptionSelectorEvent::Confirmed {
522596
id: "c".to_string()
523597
}],
@@ -539,7 +613,7 @@ fn snapshot_refresh_falls_back_to_the_selected_value_when_the_highlight_vanishes
539613
});
540614
confirm(&mut app, &selector);
541615
assert_eq!(
542-
events.borrow().as_slice(),
616+
primary_events(&events),
543617
[TuiOptionSelectorEvent::Confirmed {
544618
id: "a".to_string()
545619
}],

0 commit comments

Comments
 (0)