irongit
Sign inRegister
huncholanehuncholane/jvPublic

fix: flashing ui bug when scrolling in jv

huncholanehuncholaneauthored
parent b8a8ebbcommit 8bef318d2f449126e971aa4c08aa14dab916197cBrowse files

2 files changed, +173 -172

+152-159src/views/browser.rs
@@ -38,8 +38,9 @@ struct Entry {
3838 pub struct BrowserView {
3939 path: Vec<PathSegment>,
4040 selection: usize,
41- scroll_to_selection: bool,
4241 restore_key: Option<String>,
42+ // Scrollable list for the center column
43+ current_list: crate::widgets::scrollable_list::ScrollableList,
4344 // jq bar
4445 jq_bar: crate::widgets::jq_bar::JqBar,
4546 jq_synced: bool,
@@ -53,8 +54,9 @@ impl BrowserView {
5354 Self {
5455 path: Vec::new(),
5556 selection: 0,
56- scroll_to_selection: false,
57+
5758 restore_key: None,
59+ current_list: crate::widgets::scrollable_list::ScrollableList::new(),
5860 jq_bar: crate::widgets::jq_bar::JqBar::new(),
5961 jq_synced: true,
6062 jq_result: None,
@@ -95,7 +97,7 @@ impl BrowserView {
9597 if let Some(idx) = display_names.iter().position(|n| n == display_name) {
9698 self.selection = idx;
9799 }
98- self.scroll_to_selection = true;
100+ // selection changed
99101 self.sync_jq_from_path();
100102 }
101103
@@ -207,7 +209,7 @@ impl BrowserView {
207209 if !jq_has_focus {
208210 let action = crate::widgets::read_miller_keys(ui, false);
209211 if crate::widgets::apply_selection(&mut self.selection, action, current_entries.len()) {
210- self.scroll_to_selection = true;
212+ // selection changed
211213 self.sync_jq_from_path();
212214 }
213215 if action == crate::widgets::MillerAction::Enter {
@@ -217,7 +219,7 @@ impl BrowserView {
217219 // Root: enter a file by name
218220 self.path.push(PathSegment::Key(entry.label.clone()));
219221 self.selection = 0;
220- self.scroll_to_selection = true;
222+ // selection changed
221223 self.jq_synced = true;
222224 self.sync_jq_from_path();
223225 } else {
@@ -312,7 +314,7 @@ impl BrowserView {
312314 // Handle click actions
313315 if let Some(idx) = clicked_entry {
314316 self.selection = idx;
315- self.scroll_to_selection = false;
317+ // scroll handled by ScrollableList
316318 self.sync_jq_from_path();
317319 }
318320 if let Some(idx) = dbl_clicked_entry {
@@ -322,7 +324,7 @@ impl BrowserView {
322324 if self.path.is_empty() {
323325 self.path.push(PathSegment::Key(entry.label.clone()));
324326 self.selection = 0;
325- self.scroll_to_selection = true;
327+ // selection changed
326328 self.jq_synced = true;
327329 self.sync_jq_from_path();
328330 } else {
@@ -373,7 +375,7 @@ impl BrowserView {
373375 self.jq_synced = true;
374376 self.jq_result = None;
375377 self.jq_error = None;
376- self.scroll_to_selection = true;
378+ // selection changed
377379 }
378380 }
379381 }
@@ -386,7 +388,7 @@ impl BrowserView {
386388 self.jq_synced = true;
387389 self.jq_result = None;
388390 self.jq_error = None;
389- self.scroll_to_selection = true;
391+ // selection changed
390392 }
391393 } else if resp.run {
392394 // Manual Enter (no completion) — try as path, then as jq query
@@ -397,7 +399,7 @@ impl BrowserView {
397399 self.jq_synced = true;
398400 self.jq_result = None;
399401 self.jq_error = None;
400- self.scroll_to_selection = true;
402+ // selection changed
401403 return;
402404 }
403405 }
@@ -499,9 +501,6 @@ impl BrowserView {
499501 current_value: &serde_json::Value,
500502 height: f32,
501503 ) -> (Option<usize>, Option<usize>) {
502- let mut clicked = None;
503- let mut dbl_clicked = None;
504-
505504 if entries.is_empty() {
506505 ui.centered_and_justified(|ui| {
507506 ui.label(
@@ -513,149 +512,137 @@ impl BrowserView {
513512 return (None, None);
514513 }
515514
516- let scroll_sel = self.scroll_to_selection;
517- self.scroll_to_selection = false;
518- let selection = self.selection;
519-
520- egui::ScrollArea::vertical()
521- .id_salt("browser_current")
522- .auto_shrink(false)
523- .max_height(height)
524- .show(ui, |ui| {
525- for i in 0..entries.len() {
526- let entry = &entries[i];
527- let is_selected = i == selection;
528-
529- let (is_hovered, row_id) = crate::widgets::prev_frame_hover(ui.ctx(), ui.id(), i);
530-
531- let bg = if is_selected {
532- CatppuccinMocha::SURFACE0
533- } else if is_hovered {
534- egui::Color32::from_rgba_unmultiplied(
535- entry.color.r(), entry.color.g(), entry.color.b(), 15,
536- )
537- } else {
538- egui::Color32::TRANSPARENT
539- };
540-
541- let font_size = if is_hovered && !is_selected { 12.5 } else { 12.0 };
542- let type_size = if is_hovered && !is_selected { 11.5 } else { 11.0 };
543- let preview_size = if is_hovered && !is_selected { 11.5 } else { 11.0 };
515+ let row_height = ui.text_style_height(&egui::TextStyle::Monospace) + 6.0;
516+ let mut clicked = None;
517+ let mut dbl_clicked: Option<usize> = None;
518+
519+ self.current_list.selection = self.selection;
520+ self.current_list.show(
521+ ui,
522+ "browser_current",
523+ entries.len(),
524+ row_height,
525+ Some(height),
526+ &mut |ui, i, is_selected| {
527+ let entry = &entries[i];
528+ let (is_hovered, row_id) = crate::widgets::prev_frame_hover(ui.ctx(), ui.id(), i);
529+
530+ let bg = if is_selected {
531+ CatppuccinMocha::SURFACE0
532+ } else if is_hovered {
533+ egui::Color32::from_rgba_unmultiplied(
534+ entry.color.r(), entry.color.g(), entry.color.b(), 15,
535+ )
536+ } else {
537+ egui::Color32::TRANSPARENT
538+ };
544539
545- let mut copy_clicked = false;
540+ let font_size = if is_hovered && !is_selected { 12.5 } else { 12.0 };
541+ let type_size = if is_hovered && !is_selected { 11.5 } else { 11.0 };
542+ let preview_size = if is_hovered && !is_selected { 11.5 } else { 11.0 };
543+
544+ // Hit-test copy button from previous frame
545+ let copy_btn_id = ui.id().with(("copy_btn", i));
546+ if let Some(prev_rect) = ui.ctx().data(|d| d.get_temp::<egui::Rect>(copy_btn_id)) {
547+ if ui.rect_contains_pointer(prev_rect) {
548+ ui.painter().rect_filled(prev_rect, 3.0, CatppuccinMocha::SURFACE1);
549+ ui.painter().text(
550+ prev_rect.center(),
551+ egui::Align2::CENTER_CENTER,
552+ egui_phosphor::regular::COPY,
553+ egui::FontId::proportional(12.0),
554+ CatppuccinMocha::TEXT,
555+ );
556+ if ui.input(|inp| inp.pointer.any_click()) {
557+ if let Some(v) = child_value(current_value, i, &entry.label) {
558+ ui.ctx().copy_text(copy_value_str(v));
559+ }
560+ }
561+ }
562+ }
546563
547- // Hit-test copy button from previous frame's stored rect
548- let copy_btn_id = ui.id().with(("copy_btn", i));
549- if let Some(prev_rect) = ui.ctx().data(|d| d.get_temp::<egui::Rect>(copy_btn_id)) {
550- if ui.rect_contains_pointer(prev_rect) {
551- ui.painter().rect_filled(prev_rect, 3.0, CatppuccinMocha::SURFACE1);
564+ let r = egui::Frame::new()
565+ .fill(bg)
566+ .corner_radius(4.0)
567+ .inner_margin(egui::Margin::symmetric(6, 1))
568+ .show(ui, |ui| {
569+ ui.horizontal(|ui| {
570+ // Copy icon
571+ let icon_rect = egui::Rect::from_min_size(
572+ ui.cursor().left_top(),
573+ egui::vec2(16.0, ui.min_rect().height().max(14.0)),
574+ );
552575 ui.painter().text(
553- prev_rect.center(),
576+ icon_rect.center(),
554577 egui::Align2::CENTER_CENTER,
555578 egui_phosphor::regular::COPY,
556579 egui::FontId::proportional(12.0),
557- CatppuccinMocha::TEXT,
580+ CatppuccinMocha::OVERLAY0,
581+ );
582+ ui.add_space(16.0);
583+ ui.ctx().data_mut(|d| d.insert_temp(copy_btn_id, icon_rect));
584+
585+ // Type icon
586+ ui.label(
587+ RichText::new(entry.type_icon)
588+ .color(entry.color)
589+ .family(egui::FontFamily::Monospace)
590+ .size(font_size),
558591 );
559- egui::show_tooltip_at_pointer(
560- ui.ctx(),
561- ui.layer_id(),
562- ui.id().with(("copy_tip", i)),
592+ // Label
593+ let label_color = if is_selected || is_hovered {
594+ entry.color
595+ } else {
596+ CatppuccinMocha::SUBTEXT0
597+ };
598+ ui.label(
599+ RichText::new(&entry.label)
600+ .color(label_color)
601+ .family(egui::FontFamily::Monospace)
602+ .size(font_size),
603+ );
604+ // Type label
605+ ui.label(
606+ RichText::new(&entry.type_label)
607+ .color(CatppuccinMocha::OVERLAY0)
608+ .family(egui::FontFamily::Monospace)
609+ .size(type_size),
610+ );
611+ // Preview (right-aligned)
612+ ui.with_layout(
613+ egui::Layout::right_to_left(egui::Align::Center),
563614 |ui| {
564- ui.label("Copy value (c or Ctrl+C)");
615+ let preview = if entry.preview.len() > 24 {
616+ format!("{}...", &entry.preview[..21])
617+ } else {
618+ entry.preview.clone()
619+ };
620+ ui.label(
621+ RichText::new(preview)
622+ .color(entry.color)
623+ .family(egui::FontFamily::Monospace)
624+ .size(preview_size),
625+ );
565626 },
566627 );
567- if ui.input(|inp| inp.pointer.any_click()) {
568- copy_clicked = true;
569- if let Some(v) = child_value(current_value, i, &entry.label) {
570- ui.ctx().copy_text(copy_value_str(v));
571- }
572- }
573- }
574- }
575-
576- let r = egui::Frame::new()
577- .fill(bg)
578- .corner_radius(4.0)
579- .inner_margin(egui::Margin::symmetric(6, 1))
580- .show(ui, |ui| {
581- ui.horizontal(|ui| {
582- // Copy icon — paint only, no space allocation
583- let icon_rect = egui::Rect::from_min_size(
584- ui.cursor().left_top(),
585- egui::vec2(16.0, ui.min_rect().height().max(14.0)),
586- );
587- ui.painter().text(
588- icon_rect.center(),
589- egui::Align2::CENTER_CENTER,
590- egui_phosphor::regular::COPY,
591- egui::FontId::proportional(12.0),
592- CatppuccinMocha::OVERLAY0,
593- );
594- ui.add_space(16.0);
595- ui.ctx().data_mut(|d| d.insert_temp(copy_btn_id, icon_rect));
596-
597- // Type icon
598- ui.label(
599- RichText::new(entry.type_icon)
600- .color(entry.color)
601- .family(egui::FontFamily::Monospace)
602- .size(font_size),
603- );
604- // Label (key or index)
605- let label_color = if is_selected || is_hovered {
606- entry.color
607- } else {
608- CatppuccinMocha::SUBTEXT0
609- };
610- ui.label(
611- RichText::new(&entry.label)
612- .color(label_color)
613- .family(egui::FontFamily::Monospace)
614- .size(font_size),
615- );
616- // Type label
617- ui.label(
618- RichText::new(&entry.type_label)
619- .color(CatppuccinMocha::OVERLAY0)
620- .family(egui::FontFamily::Monospace)
621- .size(type_size),
622- );
623- // Value preview (right-aligned)
624- ui.with_layout(
625- egui::Layout::right_to_left(egui::Align::Center),
626- |ui| {
627- let preview = if entry.preview.len() > 24 {
628- format!("{}...", &entry.preview[..21])
629- } else {
630- entry.preview.clone()
631- };
632- ui.label(
633- RichText::new(preview)
634- .color(entry.color)
635- .family(egui::FontFamily::Monospace)
636- .size(preview_size),
637- );
638- },
639- );
640- });
641628 });
629+ });
642630
643- let response = r.response.interact(egui::Sense::click());
644- crate::widgets::store_hover(ui.ctx(), row_id, response.hovered());
645-
646- if response.clicked() && !copy_clicked {
647- clicked = Some(i);
648- }
649- if response.double_clicked() && !copy_clicked {
650- dbl_clicked = Some(i);
651- }
631+ let response = r.response.interact(egui::Sense::click());
632+ crate::widgets::store_hover(ui.ctx(), row_id, response.hovered());
652633
653- if is_selected && scroll_sel {
654- response.scroll_to_me(Some(egui::Align::Center));
655- }
634+ if response.double_clicked() {
635+ dbl_clicked = Some(i);
656636 }
657- });
658637
638+ response
639+ },
640+ &mut |idx| {
641+ clicked = Some(idx);
642+ },
643+ );
644+
645+ self.selection = self.current_list.selection;
659646 (clicked, dbl_clicked)
660647 }
661648
@@ -936,7 +923,7 @@ impl BrowserView {
936923 _ => return,
937924 }
938925 self.selection = 0;
939- self.scroll_to_selection = true;
926+ // selection changed
940927 self.sync_jq_from_path();
941928 }
942929 }
@@ -953,7 +940,7 @@ impl BrowserView {
953940 self.restore_key = Some(k);
954941 }
955942 }
956- self.scroll_to_selection = true;
943+ // selection changed
957944 self.sync_jq_from_path();
958945 }
959946 }
@@ -1008,7 +995,7 @@ fn resolve_from_files<'a>(
1008995
1009996 /// Build entries for the root file list (no JSON cloning).
1010997 fn build_file_entries(files: &[(String, serde_json::Value)]) -> Vec<Entry> {
1011- files.iter().map(|(name, val)| {
998+ let mut entries: Vec<Entry> = files.iter().map(|(name, val)| {
1012999 let display = name.strip_suffix(".json").unwrap_or(name);
10131000 let (icon, color) = type_icon_color(val);
10141001 Entry {
@@ -1019,7 +1006,9 @@ fn build_file_entries(files: &[(String, serde_json::Value)]) -> Vec<Entry> {
10191006 color,
10201007 is_container: true,
10211008 }
1022- }).collect()
1009+ }).collect();
1010+ entries.sort_by(|a, b| a.label.to_lowercase().cmp(&b.label.to_lowercase()));
1011+ entries
10231012 }
10241013
10251014 fn resolve_path<'a>(
@@ -1125,20 +1114,24 @@ fn copy_value_str(v: &serde_json::Value) -> String {
11251114
11261115 fn build_entries(value: &serde_json::Value) -> Vec<Entry> {
11271116 match value {
1128- serde_json::Value::Object(map) => map
1129- .iter()
1130- .map(|(k, v)| {
1131- let (icon, color) = type_icon_color(v);
1132- Entry {
1133- label: k.clone(),
1134- type_icon: icon,
1135- type_label: type_label(v),
1136- preview: value_preview(v),
1137- color,
1138- is_container: v.is_object() || v.is_array(),
1139- }
1140- })
1141- .collect(),
1117+ serde_json::Value::Object(map) => {
1118+ let mut entries: Vec<Entry> = map
1119+ .iter()
1120+ .map(|(k, v)| {
1121+ let (icon, color) = type_icon_color(v);
1122+ Entry {
1123+ label: k.clone(),
1124+ type_icon: icon,
1125+ type_label: type_label(v),
1126+ preview: value_preview(v),
1127+ color,
1128+ is_container: v.is_object() || v.is_array(),
1129+ }
1130+ })
1131+ .collect();
1132+ entries.sort_by(|a, b| a.label.to_lowercase().cmp(&b.label.to_lowercase()));
1133+ entries
1134+ }
11421135 serde_json::Value::Array(arr) => arr
11431136 .iter()
11441137 .enumerate()
+21-13src/widgets/scrollable_list.rs
@@ -9,6 +9,8 @@ pub struct ScrollableList {
99 pub selection: usize,
1010 prev_selection: usize,
1111 force_scroll: bool,
12+ /// When set, force this scroll offset on next show() to bring an off-screen selection into view.
13+ pending_offset: Option<f32>,
1214 }
1315
1416 impl ScrollableList {
@@ -17,6 +19,7 @@ impl ScrollableList {
1719 selection: 0,
1820 prev_selection: usize::MAX,
1921 force_scroll: false,
22+ pending_offset: None,
2023 }
2124 }
2225
@@ -62,38 +65,43 @@ impl ScrollableList {
6265 self.prev_selection = self.selection;
6366 self.force_scroll = false;
6467
65- let visible_h = max_height.unwrap_or(f32::MAX);
66-
67- // Set scroll offset directly so show_rows renders the selected row.
68- let scroll_offset = if needs_scroll && visible_h < f32::MAX {
69- let target_y = self.selection as f32 * row_height;
70- let centered = (target_y - visible_h / 2.0 + row_height / 2.0).max(0.0);
71- Some(centered)
72- } else {
73- None
74- };
75-
7668 let mut area = egui::ScrollArea::vertical()
7769 .id_salt(id_salt)
7870 .auto_shrink(false);
7971 if let Some(h) = max_height {
8072 area = area.max_height(h);
8173 }
82- if let Some(offset) = scroll_offset {
74+ // Apply pending offset from a previous out-of-range jump
75+ if let Some(offset) = self.pending_offset.take() {
8376 area = area.vertical_scroll_offset(offset);
8477 }
8578
8679 let mut clicked_idx: Option<usize> = None;
80+ let selection = self.selection;
8781
8882 area.show_rows(ui, row_height, count, |ui, range| {
83+ let in_range = range.contains(&selection);
84+
8985 for i in range {
90- let is_selected = i == self.selection;
86+ let is_selected = i == selection;
9187 let r = render_row(ui, i, is_selected);
9288
89+ // scroll_to_me works when the item is in/near the rendered range
90+ if needs_scroll && is_selected {
91+ r.scroll_to_me(Some(egui::Align::Center));
92+ }
9393 if r.clicked() {
9494 clicked_idx = Some(i);
9595 }
9696 }
97+
98+ // If selection was outside rendered range, queue an offset jump for next frame
99+ if needs_scroll && !in_range {
100+ if let Some(h) = max_height {
101+ let target_y = selection as f32 * row_height;
102+ self.pending_offset = Some((target_y - h / 2.0 + row_height / 2.0).max(0.0));
103+ }
104+ }
97105 });
98106
99107 if let Some(idx) = clicked_idx {