From eb777c73169a3c1ed51525461b38c3d048788186 Mon Sep 17 00:00:00 2001
From: Antoine Beyeler <antoine@rerun.io>
Date: Thu, 2 May 2024 12:29:35 +0200
Subject: [PATCH] Addressed review comments from #6182

---
 .../re_ui/src/list_item2/property_content.rs  | 31 +++++++++----------
 crates/re_ui/src/list_item2/scope.rs          |  6 +++-
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/crates/re_ui/src/list_item2/property_content.rs b/crates/re_ui/src/list_item2/property_content.rs
index e9fb1a9fc1f8e..e5cbe325adc30 100644
--- a/crates/re_ui/src/list_item2/property_content.rs
+++ b/crates/re_ui/src/list_item2/property_content.rs
@@ -30,6 +30,7 @@ pub struct PropertyContent<'a> {
 }
 
 impl<'a> PropertyContent<'a> {
+    /// Spacing used between the two main columns
     const COLUMN_SPACING: f32 = 12.0;
 
     pub fn new(label: impl Into<egui::WidgetText>) -> Self {
@@ -182,7 +183,7 @@ impl ListItemContent for PropertyContent<'_> {
         // │                                                                              │
         // │ ◀───────────state.left_column_width────────────▶│┌──COLUMN_SPACING           │
         // │                                                  ▼                           │
-        // │                       ◀──────────────CONTENT────┼──────────────────────────▶ │
+        // │                       ◀─────────────────────────┼────────context.rect──────▶ │
         // │ ┌ ─ ─ ─ ─ ┬ ─ ─ ─ ─ ┬ ┬────────┬─┬─────────────┬─┬─────────────┬─┬─────────┐ │
         // │                       │        │ │             │││             │ │         │ │
         // │ │         │         │ │        │ │             │ │             │ │         │ │
@@ -195,9 +196,6 @@ impl ListItemContent for PropertyContent<'_> {
         // │                       │                         ▲               │            │
         // │       content_left_x──┘           mid_point_x───┘     text_to_icon_padding   │
         // │                                                                              │
-        //
-        // content_indent = content_left_x - state.left_x
-        // left_column_width = content_indent + icon_extra + label_width + COLUMN_SPACING/2
 
         let state = super::StateStack::top(ui.ctx());
 
@@ -215,22 +213,13 @@ impl ListItemContent for PropertyContent<'_> {
             0.0
         };
 
-        let icon_rect = egui::Rect::from_center_size(
-            context.rect.left_center() + egui::vec2(ReUi::small_icon_size().x / 2., 0.0),
-            ReUi::small_icon_size(),
-        );
-
         // Based on egui::ImageButton::ui()
-        let button_dimension = ReUi::small_icon_size().x + 2.0 * ui.spacing().button_padding.x;
-        let action_button_rect = egui::Rect::from_center_size(
-            context.rect.right_center() - egui::vec2(button_dimension / 2.0, 0.0),
-            egui::Vec2::splat(button_dimension),
-        );
-
+        let action_button_dimension =
+            ReUi::small_icon_size().x + 2.0 * ui.spacing().button_padding.x;
         let reserve_action_button_space =
             action_buttons.is_some() || state.reserve_action_button_space;
         let action_button_extra = if reserve_action_button_space {
-            action_button_rect.width() + ReUi::text_to_icon_padding()
+            action_button_dimension + ReUi::text_to_icon_padding()
         } else {
             0.0
         };
@@ -252,6 +241,11 @@ impl ListItemContent for PropertyContent<'_> {
 
         // Draw icon
         if let Some(icon_fn) = icon_fn {
+            let icon_rect = egui::Rect::from_center_size(
+                context.rect.left_center() + egui::vec2(ReUi::small_icon_size().x / 2., 0.0),
+                ReUi::small_icon_size(),
+            );
+
             icon_fn(re_ui, ui, icon_rect, visuals);
         }
 
@@ -311,6 +305,11 @@ impl ListItemContent for PropertyContent<'_> {
 
         // Draw action button
         if let Some(action_button) = action_buttons {
+            let action_button_rect = egui::Rect::from_center_size(
+                context.rect.right_center() - egui::vec2(action_button_dimension / 2.0, 0.0),
+                egui::Vec2::splat(action_button_dimension),
+            );
+
             // the right to left layout is used to mimic LabelContent's buttons behavior and get a
             // better alignment
             let mut child_ui = ui.child_ui(
diff --git a/crates/re_ui/src/list_item2/scope.rs b/crates/re_ui/src/list_item2/scope.rs
index 8a9b9782ecd89..5d0627877ad41 100644
--- a/crates/re_ui/src/list_item2/scope.rs
+++ b/crates/re_ui/src/list_item2/scope.rs
@@ -17,7 +17,7 @@ pub struct State {
     /// based on `ui.max_rect()`.
     pub(crate) left_x: f32,
 
-    /// Column width to be used this frame.
+    /// Column width to be read this frame.
     ///
     /// The column width has `left_x` as reference, so it includes:
     /// - All the indentation on the left side of the list item.
@@ -26,6 +26,10 @@ pub struct State {
     ///
     /// The effective left column width for a given [`super::ListItemContent`] implementation can be
     /// calculated as `left_column_width - (context.rect.left() - left_x)`.
+    ///
+    /// This value is set to `None` during the first frame, when [`list_item_scope`] isn't able to
+    /// determine a suitable value. In that case, implementations should devise a suitable default
+    /// value.
     pub(crate) left_column_width: Option<f32>,
 
     /// Maximum desired column width, to be updated this frame.