ListItem 2.0 (part 3): PropertyContent column auto-sizing (#6182)

### What This PR: - Implements column auto-sizing for `PropertyContent`. The current heuristics is rather simple and will likely need improvement when it's battle tested. Currently, the content requests a minimum width of 200px (there really isn't a way to properly deal with two-column-plus-button neatly all the way to 0 width), and allocate a maximum of 70% of the available width to the label column. - Improves the way `ListItem` handles content capturing the hover state (which happen for interactive content). There is no longer a requirement for `ListItemContent::ui()` to return an `Option<Response>`, which simplifies a bunch of things. - Updates `re_ui_example` accordingly Note: still no change applied to the actual viewer code base so far. - Part of #6075 - Follow-up to #6174 <img width="502" alt="image" src="https://github.com/rerun-io/rerun/assets/49431240/28322870-997c-4bf4-997f-d0e7c8da2da9"> ### Limitations and todo - Improved heuristics for column sizing, when we have a better idea of the need. - More helpers are needed for various kinds of values. - There can be only 0 or 1 action button. This should be extended by using a `…` button with some kind of popup with all available actions in a future PR. - Right gutter space is reserved for the action button even if no list item in scope use them. The `list_item_scope` could track this and skip reserving that space if it's never used (e.g. component list in entity path selection panel): #6179 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6182?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/6182?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/6182) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`.
rerun-io · May 2, 2024 · d086044 · d086044
1 parent 9f6add2
commit d086044
Show file tree

Hide file tree

Showing 7 changed files with 242 additions and 161 deletions.
diff --git a/crates/re_ui/examples/re_ui_example/right_panel.rs b/crates/re_ui/examples/re_ui_example/right_panel.rs
@@ -186,47 +186,51 @@ impl RightPanel {
             list_item2::PropertyContent::new("PropertyContent features:")
                 .value_text("bunch of properties"),
             |re_ui, ui| {
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Bool").value_bool(self.boolean),
-                );
-
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Bool (editable)")
-                        .value_bool_mut(&mut self.boolean),
-                );
+                // By using an inner scope, we allow the nested properties to not align themselves
+                // to the parent property, which in this particular case looks better.
+                list_item2::list_item_scope(ui, "inner_scope", None, |ui| {
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Bool").value_bool(self.boolean),
+                    );
 
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Text").value_text(&self.text),
-                );
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Bool (editable)")
+                            .value_bool_mut(&mut self.boolean),
+                    );
 
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Text (editable)")
-                        .value_text_mut(&mut self.text),
-                );
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Text").value_text(&self.text),
+                    );
 
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Color")
-                        .with_icon(&re_ui::icons::SPACE_VIEW_TEXT)
-                        .action_button(&re_ui::icons::ADD, || {
-                            re_log::warn!("Add button clicked");
-                        })
-                        .value_color(&self.color),
-                );
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Text (editable)")
+                            .value_text_mut(&mut self.text),
+                    );
 
-                re_ui.list_item2().show_hierarchical(
-                    ui,
-                    list_item2::PropertyContent::new("Color (editable)")
-                        .with_icon(&re_ui::icons::SPACE_VIEW_TEXT)
-                        .action_button(&re_ui::icons::ADD, || {
-                            re_log::warn!("Add button clicked");
-                        })
-                        .value_color_mut(&mut self.color),
-                );
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Color")
+                            .with_icon(&re_ui::icons::SPACE_VIEW_TEXT)
+                            .action_button(&re_ui::icons::ADD, || {
+                                re_log::warn!("Add button clicked");
+                            })
+                            .value_color(&self.color),
+                    );
+
+                    re_ui.list_item2().show_hierarchical(
+                        ui,
+                        list_item2::PropertyContent::new("Color (editable)")
+                            .with_icon(&re_ui::icons::SPACE_VIEW_TEXT)
+                            .action_button(&re_ui::icons::ADD, || {
+                                re_log::warn!("Add button clicked");
+                            })
+                            .value_color_mut(&mut self.color),
+                    );
+                });
             },
         );
 
@@ -261,8 +265,6 @@ impl RightPanel {
                             egui::Color32::LIGHT_RED,
                             "CustomContent delegates to a closure",
                         );
-
-                        None
                     }),
                 )
             },

diff --git a/crates/re_ui/src/list_item2/label_content.rs b/crates/re_ui/src/list_item2/label_content.rs
@@ -126,12 +126,7 @@ impl<'a> LabelContent<'a> {
 }
 
 impl ListItemContent for LabelContent<'_> {
-    fn ui(
-        self: Box<Self>,
-        re_ui: &ReUi,
-        ui: &mut Ui,
-        context: &ContentContext<'_>,
-    ) -> Option<egui::Response> {
+    fn ui(self: Box<Self>, re_ui: &ReUi, ui: &mut Ui, context: &ContentContext<'_>) {
         let Self {
             mut text,
             subdued,
@@ -226,8 +221,6 @@ impl ListItemContent for LabelContent<'_> {
             .min;
 
         ui.painter().galley(text_pos, galley, visuals.text_color());
-
-        button_response
     }
 
     fn desired_width(&self, _re_ui: &ReUi, ui: &Ui) -> DesiredWidth {

diff --git a/crates/re_ui/src/list_item2/list_item.rs b/crates/re_ui/src/list_item2/list_item.rs
@@ -277,14 +277,14 @@ impl<'a> ListItem<'a> {
             if collapse_openness.is_some() {
                 content_rect.min.x += extra_indent + collapse_extra;
             }
+
             let content_ctx = ContentContext {
                 rect: content_rect,
                 bg_rect,
                 response: &style_response,
                 list_item: &self,
-                state: &state,
             };
-            let content_response = content.ui(re_ui, ui, &content_ctx);
+            content.ui(re_ui, ui, &content_ctx);
 
             // Draw background on interaction.
             if drag_target {
@@ -293,7 +293,7 @@ impl<'a> ListItem<'a> {
                     Shape::rect_stroke(bg_rect, 0.0, (1.0, ui.visuals().selection.bg_fill)),
                 );
             } else {
-                let bg_fill = if content_response.map_or(false, |r| r.hovered()) {
+                let bg_fill = if !response.hovered() && ui.rect_contains_pointer(bg_rect) {
                     // if some part of the content is active and hovered, our background should
                     // become dimmer
                     Some(visuals.bg_fill)

diff --git a/crates/re_ui/src/list_item2/mod.rs b/crates/re_ui/src/list_item2/mod.rs
@@ -31,9 +31,6 @@ pub struct ContentContext<'a> {
 
     /// The current list item.
     pub list_item: &'a ListItem<'a>,
-
-    /// The frame-over-frame state for this list item.
-    pub state: &'a State,
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -65,12 +62,7 @@ pub trait ListItemContent {
     /// If the content has some interactive elements, it should return its response. In particular,
     /// if the response is hovered, the list item will show a dimmer background highlight.
     //TODO(ab): could the return type be just a bool meaning "inner interactive widget was hovered"?
-    fn ui(
-        self: Box<Self>,
-        re_ui: &crate::ReUi,
-        ui: &mut egui::Ui,
-        context: &ContentContext<'_>,
-    ) -> Option<egui::Response>;
+    fn ui(self: Box<Self>, re_ui: &crate::ReUi, ui: &mut egui::Ui, context: &ContentContext<'_>);
 
     /// The desired width of the content.
     fn desired_width(&self, _re_ui: &crate::ReUi, _ui: &egui::Ui) -> DesiredWidth {

diff --git a/crates/re_ui/src/list_item2/other_contents.rs b/crates/re_ui/src/list_item2/other_contents.rs
@@ -11,34 +11,25 @@ impl ListItemContent for EmptyContent {
         _re_ui: &crate::ReUi,
         _ui: &mut egui::Ui,
         _context: &ContentContext<'_>,
-    ) -> Option<egui::Response> {
-        None
+    ) {
     }
 }
 
 /// [`ListItemContent`] that delegates to a closure.
 #[allow(clippy::type_complexity)]
-pub struct CustomContent {
-    ui: Box<dyn FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) -> Option<egui::Response>>,
+pub struct CustomContent<'a> {
+    ui: Box<dyn FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) + 'a>,
 }
 
-impl CustomContent {
-    pub fn new(
-        ui: impl FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) -> Option<egui::Response>
-            + 'static,
-    ) -> Self {
+impl<'a> CustomContent<'a> {
+    pub fn new(ui: impl FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) + 'a) -> Self {
         Self { ui: Box::new(ui) }
     }
 }
 
-impl ListItemContent for CustomContent {
-    fn ui(
-        self: Box<Self>,
-        re_ui: &crate::ReUi,
-        ui: &mut egui::Ui,
-        context: &ContentContext<'_>,
-    ) -> Option<egui::Response> {
-        (self.ui)(re_ui, ui, context)
+impl ListItemContent for CustomContent<'_> {
+    fn ui(self: Box<Self>, re_ui: &crate::ReUi, ui: &mut egui::Ui, context: &ContentContext<'_>) {
+        (self.ui)(re_ui, ui, context);
     }
 }
 
@@ -64,17 +55,10 @@ impl DebugContent {
 }
 
 impl ListItemContent for DebugContent {
-    fn ui(
-        self: Box<Self>,
-        _re_ui: &crate::ReUi,
-        ui: &mut egui::Ui,
-        context: &ContentContext<'_>,
-    ) -> Option<egui::Response> {
+    fn ui(self: Box<Self>, _re_ui: &crate::ReUi, ui: &mut egui::Ui, context: &ContentContext<'_>) {
         ui.ctx()
             .debug_painter()
             .debug_rect(context.rect, egui::Color32::DARK_GREEN, self.label);
-
-        None
     }
 
     fn desired_width(&self, _re_ui: &ReUi, _ui: &Ui) -> DesiredWidth {