Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs #6184

Merged
merged 7 commits into from
May 3, 2024
Merged
52 changes: 36 additions & 16 deletions crates/re_data_ui/src/editors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,28 +257,48 @@ fn edit_marker_shape_ui(

egui::ComboBox::from_id_source("marker_shape")
.selected_text(marker_text) // TODO(emilk): Show marker shape in the selected text
.width(100.0)
.width(ui.available_width().at_most(100.0))
.height(320.0)
.show_ui(ui, |ui| {
// no spacing between list items
ui.spacing_mut().item_spacing.y = 0.0;

// Hack needed for ListItem to click its highlight bg rect correctly:
ui.set_clip_rect(
ui.clip_rect()
.with_max_x(ui.max_rect().max.x + ui.spacing().menu_margin.right),
let item_width = 100.0;

// workaround to force `ui.max_rect()` to reflect the content size
ui.allocate_space(egui::vec2(item_width, 0.0));
Comment on lines +268 to +269
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// workaround to force `ui.max_rect()` to reflect the content size
ui.allocate_space(egui::vec2(item_width, 0.0));
// workaround to force `ui.max_rect()` to reflect the content size
ui.set_width(item_width);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the doscstring of ComboBox::width claims it sets the with of the menu… but it doesn't. We should fix that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's slightly more complicated than that, if I understand correctly. In my code, the width of the combobox itself can shrink a lot when squeezed: .width(ui.available_width().at_most(100.0)). The menu width correctly set to that size. The issue is when the menu content is larger than the provided width. In that case, the menu visually expends to accommodate its content, but doesn't reflect that in the closure's ui.available_width(), which remains stuck to combobox.width - inner_margins.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to have a repro and open an issue in egui.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abey79 marked this conversation as resolved.
Show resolved Hide resolved

let background_x_range = ui
.spacing()
.menu_margin
.expand_rect(ui.max_rect())
.x_range();

re_ui::list_item2::list_item_scope(
ui,
"marker_shape",
Some(background_x_range),
Comment on lines +271 to +280
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could make a re_ui::combo_box helper that also did this

Copy link
Member Author

@abey79 abey79 May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I want all of our popup menu, context menu, etc. to be reimplemented and styled after list item:

I would certainly welcome your input in this area, especially around the menu logics that currently relies on custom egui::Ui function IIRC.

|ui| {
for marker in MarkerShape::ALL {
let response = ctx
.re_ui
.list_item2()
.selected(edit_marker == marker)
.show_flat(
ui,
re_ui::list_item2::LabelContent::new(marker.to_string())
.min_desired_width(item_width)
.with_icon_fn(|_re_ui, ui, rect, visuals| {
paint_marker(ui, marker.into(), rect, visuals.text_color());
}),
);
Comment on lines +283 to +294
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exploded in line-count quite a bit… I wonder if there is anything we can do to improve this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't immediately think of one.

One area to think about is to have some DataUi like thing that would make our data objects somehow capable of producing impl ListItemContent. If MarkerShape had this behaviour, it could be passed directly (or as marker.as_content() or sth) to .show_flat. I think we do need some hindsight to be able to design such a system, though.


if response.clicked() {
edit_marker = marker;
}
}
},
);

for marker in MarkerShape::ALL {
let list_item = re_ui::ListItem::new(ctx.re_ui, marker.to_string())
.with_icon_fn(|_re_ui, ui, rect, visuals| {
paint_marker(ui, marker.into(), rect, visuals.text_color());
})
.selected(edit_marker == marker);
if list_item.show_flat(ui).clicked() {
edit_marker = marker;
}
}
});

if edit_marker != current_marker {
Expand Down
10 changes: 10 additions & 0 deletions crates/re_ui/examples/re_ui_example/right_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,16 @@ impl RightPanel {
},
),
);

re_ui.list_item2().show_hierarchical(
ui,
list_item2::LabelContent::new("LabelContent with buttons (always shown)")
.with_buttons(|re_ui, ui| {
re_ui.small_icon_button(ui, &re_ui::icons::ADD)
| re_ui.small_icon_button(ui, &re_ui::icons::REMOVE)
})
.always_show_buttons(true),
);
},
);

Expand Down
49 changes: 39 additions & 10 deletions crates/re_ui/src/list_item2/label_content.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::list_item2::{ContentContext, DesiredWidth, ListItemContent};
use egui::{text::TextWrapping, Align, Align2, NumExt, Ui};

use super::{ContentContext, DesiredWidth, ListItemContent};
use crate::{Icon, LabelStyle, ReUi};
use eframe::emath::{Align, Align2};
use eframe::epaint::text::TextWrapping;
use egui::Ui;

/// [`ListItemContent`] that displays a simple label with optional icon and buttons.
#[allow(clippy::type_complexity)]
Expand All @@ -17,7 +16,9 @@ pub struct LabelContent<'a> {
label_style: LabelStyle,
icon_fn: Option<Box<dyn FnOnce(&ReUi, &egui::Ui, egui::Rect, egui::style::WidgetVisuals) + 'a>>,
buttons_fn: Option<Box<dyn FnOnce(&ReUi, &mut egui::Ui) -> egui::Response + 'a>>,
always_show_buttons: bool,

min_desired_width: f32,
exact_width: bool,
}

Expand All @@ -31,6 +32,8 @@ impl<'a> LabelContent<'a> {
label_style: Default::default(),
icon_fn: None,
buttons_fn: None,
always_show_buttons: false,
min_desired_width: 0.0,
exact_width: false,
}
}
Expand Down Expand Up @@ -81,12 +84,23 @@ impl<'a> LabelContent<'a> {
/// By default, [`LabelContent`] uses the available width. By setting `exact_width` to true,
/// the exact width required by the label (and the icon if any) is allocated instead. See
/// [`super::DesiredWidth::Exact`].
///
/// Note that if [`Self::min_desired_width`] is set, it is used as a minimum value.
#[inline]
pub fn exact_width(mut self, exact_width: bool) -> Self {
self.exact_width = exact_width;
self
}

/// Set the minimum desired for the content.
///
/// This defaults to zero.
#[inline]
pub fn min_desired_width(mut self, min_desired_width: f32) -> Self {
self.min_desired_width = min_desired_width;
self
}

/// Provide an [`Icon`] to be displayed on the left of the item.
#[inline]
pub fn with_icon(self, icon: &'a Icon) -> Self {
Expand All @@ -108,7 +122,8 @@ impl<'a> LabelContent<'a> {

/// Provide a closure to display on-hover buttons on the right of the item.
///
/// Buttons also show when the item is selected, in order to support clicking them on touch screens.
/// Buttons also show when the item is selected, in order to support clicking them on touch
/// screens. The buttons can be set to be always shown with [`Self::always_show_buttons`].
///
/// Notes:
/// - If buttons are used, the item will allocate the full available width of the parent. If the
Expand All @@ -123,6 +138,16 @@ impl<'a> LabelContent<'a> {
self.buttons_fn = Some(Box::new(buttons));
self
}

/// Always show the buttons.
///
/// By default, buttons are only shown when the item is hovered or selected. By setting this to
/// `true`, the buttons are always shown.
#[inline]
pub fn always_show_buttons(mut self, always_show_buttons: bool) -> Self {
self.always_show_buttons = always_show_buttons;
self
}
}

impl ListItemContent for LabelContent<'_> {
Expand All @@ -135,6 +160,8 @@ impl ListItemContent for LabelContent<'_> {
label_style,
icon_fn,
buttons_fn,
always_show_buttons,
min_desired_width: _,
exact_width: _,
} = *self;

Expand Down Expand Up @@ -179,10 +206,12 @@ impl ListItemContent for LabelContent<'_> {
// We can't use `.hovered()` or the buttons disappear just as the user clicks,
// so we use `contains_pointer` instead. That also means we need to check
// that we aren't dragging anything.
let should_show_buttons = context.list_item.interactive
// By showing the buttons when selected, we allow users to find them on touch screens.
let should_show_buttons = (context.list_item.interactive
&& ui.rect_contains_pointer(context.bg_rect)
&& !egui::DragAndDrop::has_any_payload(ui.ctx())
|| context.list_item.selected; // by showing the buttons when selected, we allow users to find them on touch screens
&& !egui::DragAndDrop::has_any_payload(ui.ctx()))
|| context.list_item.selected
|| always_show_buttons;
let button_response = if should_show_buttons {
if let Some(buttons) = buttons_fn {
let mut ui =
Expand Down Expand Up @@ -244,9 +273,9 @@ impl ListItemContent for LabelContent<'_> {

// The `ceil()` is needed to avoid some rounding errors which leads to text being
// truncated even though we allocated enough space.
DesiredWidth::Exact(desired_width.ceil())
DesiredWidth::Exact(desired_width.ceil().at_least(self.min_desired_width))
} else {
DesiredWidth::default()
DesiredWidth::AtLeast(self.min_desired_width)
}
}
}
3 changes: 3 additions & 0 deletions crates/re_ui/src/list_item2/list_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ impl<'a> ListItem<'a> {
bg_rect.set_left(layout_info.background_x_range.min);
bg_rect.set_right(layout_info.background_x_range.max);

// Record the max allocated width.
layout_info.register_max_item_width(ui.ctx(), rect.right() - layout_info.left_x);

// We want to be able to select/hover the item across its full span, so we interact over the
// entire background rect. But…
let mut response = ui.interact(bg_rect, allocated_id, sense);
Expand Down
27 changes: 19 additions & 8 deletions crates/re_ui/src/list_item2/property_content.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::list_item2::{ContentContext, DesiredWidth, ListItemContent};
use egui::{text::TextWrapping, Align, Align2, NumExt as _, Ui};

use super::{ContentContext, DesiredWidth, ListItemContent};
use crate::{Icon, ReUi};
use eframe::emath::{Align, Align2};
use eframe::epaint::text::TextWrapping;
use egui::{NumExt, Ui};

/// Closure to draw an icon left of the label.
type IconFn<'a> = dyn FnOnce(&ReUi, &mut egui::Ui, egui::Rect, egui::style::WidgetVisuals) + 'a;
Expand All @@ -20,6 +19,7 @@ struct PropertyActionButton<'a> {
/// value (which may be editable).
pub struct PropertyContent<'a> {
label: egui::WidgetText,
min_desired_width: f32,
icon_fn: Option<Box<IconFn<'a>>>,
show_only_when_collapsed: bool,
value_fn: Option<Box<PropertyValueFn<'a>>>,
Expand All @@ -36,13 +36,24 @@ impl<'a> PropertyContent<'a> {
pub fn new(label: impl Into<egui::WidgetText>) -> Self {
Self {
label: label.into(),
min_desired_width: 200.0,
icon_fn: None,
show_only_when_collapsed: true,
value_fn: None,
action_buttons: None,
}
}

/// Set the minimum desired width for the entire content.
///
/// Since there is no possibly way to meaningfully collapse two to three columns worth of
/// content, this is set to 200.0 by default.
#[inline]
pub fn min_desired_width(mut self, min_desired_width: f32) -> Self {
self.min_desired_width = min_desired_width;
self
}

/// Provide an [`Icon`] to be displayed on the left of the label.
#[inline]
pub fn with_icon(self, icon: &'a Icon) -> Self {
Expand Down Expand Up @@ -134,7 +145,7 @@ impl<'a> PropertyContent<'a> {
#[inline]
pub fn value_text(self, text: impl Into<egui::WidgetText> + 'a) -> Self {
self.value_fn(move |_, ui, _| {
ui.label(text.into());
ui.add(egui::Label::new(text.into()).truncate(true));
})
}

Expand Down Expand Up @@ -172,6 +183,7 @@ impl ListItemContent for PropertyContent<'_> {
fn ui(self: Box<Self>, re_ui: &ReUi, ui: &mut Ui, context: &ContentContext<'_>) {
let Self {
label,
min_desired_width: _,
icon_fn,
show_only_when_collapsed,
value_fn,
Expand All @@ -192,7 +204,7 @@ impl ListItemContent for PropertyContent<'_> {
// │ │ │ │ │││ │ │ │ │
// │ └ ─ ─ ─ ─ ┴ ─ ─ ─ ─ ┴ ┴────────┴─┴─────────────┴─┴─────────────┴─┴─────────┘ │
// │ ▲ ▲ ▲ │ ▲ │
// │ └──layout_info.left │ └───────────────────────────────┤ │
// │ └──layout_info.left_x │ └───────────────────────────────┤ │
// │ │ ▲ │ │
// │ content_left_x──┘ mid_point_x───┘ text_to_icon_padding() │
// │ │
Expand Down Expand Up @@ -325,7 +337,6 @@ impl ListItemContent for PropertyContent<'_> {
}

fn desired_width(&self, _re_ui: &ReUi, _ui: &Ui) -> DesiredWidth {
// really no point having a two-column widget collapsed to 0 width
super::DesiredWidth::AtLeast(200.0)
DesiredWidth::AtLeast(self.min_desired_width)
}
}
54 changes: 47 additions & 7 deletions crates/re_ui/src/list_item2/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,35 @@ use egui::NumExt;
/// stored in this structure (via [`LayoutInfo`] methods). Then, it is saved in egui temporary memory
/// against the scope id. On frame `n+1`, the accumulated values are used by [`list_item_scope`] to
/// set up the [`LayoutInfo`] and the accumulator is reset to restart the process.
///
/// Here is an illustration of the layout statistics that are gathered:
/// ```text
/// │◀─────────────────────background_x_range───────────────────▶│
/// │ │
/// │ ┌──left_x │
/// │ ▼ │
/// │ │ │ │ │
/// │ ┌───────────────────────────────────────────┐ │
/// │ │ │ │ │ │
/// │ └───┬────────────────────────────────────┬──┘ │
/// │ │ ▼ │ │ │ │ │
/// │ └───┬─────────────────────────┬──────┘ │
/// │ │ │ │ │ │ │
/// │ ├─────────────────────────┴────┐ │
/// │ │ ▼ │ │ │ │ │
/// │ └───┬──────────────────────────┴─────────┐ │
/// │ │ │ │ │ │
/// │ ├─────────────────────┬──────────────┘ │
/// │ │ ▶ │ │ │ │ │
/// │ ┌───────────┴─────────────────────┴──┐ │
/// │ │ │ │ │ │
/// │ └────────────────────────────────────┘ │
/// │ │ │ │ │
/// │ │
/// │ │◀──────────────────────▶ max_desired_left_column_width │
/// │ │
/// │ │◀───────────────max_item_width─────────────────▶│ │
/// ```
#[derive(Debug, Clone)]
struct LayoutStatistics {
/// Maximum desired column width.
Expand All @@ -17,6 +46,11 @@ struct LayoutStatistics {
///
/// If so, space for a right-aligned gutter should be reserved.
is_action_button_used: bool,

/// Max item width.
///
/// The width is calculated from [`LayoutInfo::left_x`] to the right edge of the item.
max_item_width: f32,
}

impl Default for LayoutStatistics {
Expand All @@ -25,6 +59,7 @@ impl Default for LayoutStatistics {
Self {
max_desired_left_column_width: f32::NEG_INFINITY,
is_action_button_used: false,
max_item_width: f32::NEG_INFINITY,
abey79 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -117,23 +152,28 @@ impl LayoutInfo {
///
/// All [`super::ListItemContent`] implementation that attempt to align on the two-column system should
/// call this function once in their [`super::ListItemContent::ui`] method.
pub(crate) fn register_desired_left_column_width(
&self,
ctx: &egui::Context,
desired_width: f32,
) {
pub fn register_desired_left_column_width(&self, ctx: &egui::Context, desired_width: f32) {
LayoutStatistics::update(ctx, self.scope_id, |stats| {
stats.max_desired_left_column_width =
stats.max_desired_left_column_width.max(desired_width);
});
}

/// Indicate whether right-aligned space should be reserved for the action button.
pub(crate) fn reserve_action_button_space(&self, ctx: &egui::Context, reserve: bool) {
pub fn reserve_action_button_space(&self, ctx: &egui::Context, reserve: bool) {
LayoutStatistics::update(ctx, self.scope_id, |stats| {
stats.is_action_button_used |= reserve;
});
}

/// Register the maximum width of the item.
///
/// Should only be set by [`super::ListItem`].
pub(crate) fn register_max_item_width(&self, ctx: &egui::Context, width: f32) {
LayoutStatistics::update(ctx, self.scope_id, |stats| {
stats.max_item_width = stats.max_item_width.max(width);
});
}
}

/// Stack of [`LayoutInfo`]s.
Expand Down Expand Up @@ -232,7 +272,7 @@ pub fn list_item_scope<R>(
// from real-world usage.
layout_stats
.max_desired_left_column_width
.at_most(0.7 * ui.max_rect().width()),
.at_most(0.7 * layout_stats.max_item_width),
)
} else {
None
Expand Down
Loading
Loading