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

Better integration of ListItem in tooltips and other popups #6486

Merged
merged 14 commits into from
Jun 11, 2024
Merged
10 changes: 8 additions & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@
}
},
"args": [],
"cwd": "${workspaceFolder}"
"cwd": "${workspaceFolder}",
"env": {
"RUST_LOG": "debug"
}
},
{
"name": "Debug 'rerun ../data.rrd'",
Expand All @@ -81,7 +84,10 @@
"args": [
"../data.rrd"
],
"cwd": "${workspaceFolder}"
"cwd": "${workspaceFolder}",
"env": {
"RUST_LOG": "debug"
}
},
{
"name": "Debug 'rerun' colmap.rrd from url",
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5187,6 +5187,7 @@ dependencies = [
"re_format",
"re_log",
"re_log_types",
"re_tracing",
"serde",
"serde_json",
"strum",
Expand Down
7 changes: 1 addition & 6 deletions crates/re_data_ui/src/instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl DataUi for InstancePath {
continue; // no need to show components that are unset at this point in time
};

let mut content =
let content =
re_ui::list_item::PropertyContent::new(component_name.short_name())
.with_icon(icon)
.value_fn(|ui, _| {
Expand Down Expand Up @@ -145,11 +145,6 @@ impl DataUi for InstancePath {
}
});

// avoid the list item to max the tooltip width
if ui_layout == UiLayout::Tooltip {
content = content.exact_width(true);
}
Comment on lines -148 to -151
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now detected automagically by PropertyContent


list_item.show_flat(ui, content)
};

Expand Down
3 changes: 1 addition & 2 deletions crates/re_data_ui/src/item_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,7 @@ pub fn component_path_button_to(
} else {
"Temporal component"
})
.with_icon(icon)
.exact_width(true),
Copy link
Member Author

Choose a reason for hiding this comment

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

again, the contents magically knows not to truncate when in a tooltip

.with_icon(icon),
);

let component_name = component_path.component_name;
Expand Down
6 changes: 3 additions & 3 deletions crates/re_time_panel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ impl TimePanel {
ctx,
&InstancePath::from(tree.path.clone()),
))
.exact_width(true),
.truncate(false),
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't yet magically detect that we are in the time panel, so we still need to tell the contents not to truncate here

|ui| {
self.show_children(
ctx,
Expand Down Expand Up @@ -772,7 +772,7 @@ impl TimePanel {
} else {
&re_ui::icons::COMPONENT_TEMPORAL
})
.exact_width(true),
.truncate(false),
);

context_menu_ui_for_item(
Expand Down Expand Up @@ -816,7 +816,7 @@ impl TimePanel {
)
},
))
.exact_width(true)
.truncate(false)
.with_icon(if is_static {
&re_ui::icons::COMPONENT_STATIC
} else {
Expand Down
1 change: 1 addition & 0 deletions crates/re_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ re_entity_db.workspace = true # syntax-highlighting for InstancePath. TODO(emilk
re_format.workspace = true
re_log.workspace = true
re_log_types.workspace = true # syntax-highlighting for EntityPath
re_tracing.workspace = true

egui.workspace = true
egui_commonmark = { workspace = true, features = ["pulldown_cmark"] }
Expand Down
7 changes: 2 additions & 5 deletions crates/re_ui/examples/re_ui_example/right_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,13 @@ impl RightPanel {
format!("Some item {i}")
};

// Note: we use `exact_width(true)` here to force the item to allocate
// Note: we use `truncate(false)` here to force the item to allocate
// as much as needed for the label, which in turn will trigger the
// scroll area.
if ui
.list_item()
.selected(Some(i) == self.selected_list_item)
.show_flat(
ui,
list_item::LabelContent::new(&label).exact_width(true),
)
.show_flat(ui, list_item::LabelContent::new(&label).truncate(false))
.clicked()
{
self.selected_list_item = Some(i);
Expand Down
26 changes: 26 additions & 0 deletions crates/re_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,29 @@ pub fn apply_style_and_install_loaders(egui_ctx: &egui::Context) {

design_tokens().apply(egui_ctx);
}

/// Used as a heuristic to figure out if it is safe to truncate text.
///
/// If this returns false, we should never truncate.
Copy link
Member

Choose a reason for hiding this comment

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

If this returns false, we should never truncate.

Double-negative. Would be easier to follow as "If this returns true, we should truncate text when necessary."

The rationale coupling this description to the name of the function still isn't totally obvious. Even if a panel is not resizable, if the text is much longer than the available (non-resizable) panel size, it seems like truncation could be necessary.

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 expanded the docstring to clarify

fn is_in_resizable_area(ui: &egui::Ui) -> bool {
re_tracing::profile_function!();

let mut is_in_side_panel = false;

for frame in ui.stack().iter() {
if let Some(kind) = frame.kind() {
if kind.is_area() {
return false; // Our popups (tooltips etc) aren't resizable
}
if matches!(kind, egui::UiKind::LeftPanel | egui::UiKind::RightPanel) {
is_in_side_panel = true;
}
}
}

if is_in_side_panel {
true // Our side-panels are resizable
} else {
false // Safe fallback
}
}
64 changes: 45 additions & 19 deletions crates/re_ui/src/list_item/label_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,26 @@ pub struct LabelContent<'a> {
buttons_fn: Option<Box<dyn FnOnce(&mut egui::Ui) -> egui::Response + 'a>>,
always_show_buttons: bool,

min_desired_width: f32,
exact_width: bool,
text_wrap_mode: Option<egui::TextWrapMode>,
min_desired_width: Option<f32>,
}

impl<'a> LabelContent<'a> {
pub fn new(text: impl Into<egui::WidgetText>) -> Self {
Self {
text: text.into(),

subdued: false,
weak: false,
italics: false,

label_style: Default::default(),
icon_fn: None,
buttons_fn: None,
always_show_buttons: false,
min_desired_width: 0.0,
exact_width: false,

text_wrap_mode: None,
min_desired_width: None,
}
}

Expand Down Expand Up @@ -79,16 +82,14 @@ impl<'a> LabelContent<'a> {
self
}

/// Allocate the exact width required for the label.
///
/// 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.
/// Should we truncate text if it is too long?
#[inline]
pub fn exact_width(mut self, exact_width: bool) -> Self {
self.exact_width = exact_width;
pub fn truncate(mut self, truncate: bool) -> Self {
self.text_wrap_mode = Some(if truncate {
egui::TextWrapMode::Truncate
} else {
egui::TextWrapMode::Extend
});
self
}

Expand All @@ -97,7 +98,7 @@ impl<'a> LabelContent<'a> {
/// 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.min_desired_width = Some(min_desired_width);
self
}

Expand Down Expand Up @@ -148,10 +149,22 @@ impl<'a> LabelContent<'a> {
self.always_show_buttons = always_show_buttons;
self
}

fn get_text_wrap_mode(&self, ui: &egui::Ui) -> egui::TextWrapMode {
if let Some(text_wrap_mode) = self.text_wrap_mode {
text_wrap_mode
} else if crate::is_in_resizable_area(ui) {
egui::TextWrapMode::Truncate
} else {
egui::TextWrapMode::Extend
}
}
}

impl ListItemContent for LabelContent<'_> {
fn ui(self: Box<Self>, ui: &mut Ui, context: &ContentContext<'_>) {
let text_wrap_mode = self.get_text_wrap_mode(ui);

let Self {
mut text,
subdued,
Expand All @@ -161,8 +174,8 @@ impl ListItemContent for LabelContent<'_> {
icon_fn,
buttons_fn,
always_show_buttons,
text_wrap_mode: _,
min_desired_width: _,
exact_width: _,
} = *self;

let icon_rect = egui::Rect::from_center_size(
Expand Down Expand Up @@ -235,7 +248,7 @@ impl ListItemContent for LabelContent<'_> {

let mut layout_job =
text.into_layout_job(ui.style(), egui::FontSelection::Default, Align::LEFT);
layout_job.wrap = TextWrapping::truncate_at_width(text_rect.width());
layout_job.wrap = TextWrapping::from_wrap_mode_and_width(text_wrap_mode, text_rect.width());

let galley = ui.fonts(|fonts| fonts.layout_job(layout_job));

Expand All @@ -256,7 +269,9 @@ impl ListItemContent for LabelContent<'_> {
}

fn desired_width(&self, ui: &Ui) -> DesiredWidth {
if self.exact_width {
let text_wrap_mode = self.get_text_wrap_mode(ui);

let measured_width = {
//TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui`
let mut text = self.text.clone();
if self.italics || self.label_style == LabelStyle::Unnamed {
Expand All @@ -277,9 +292,20 @@ 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().at_least(self.min_desired_width))
desired_width.ceil()
};

if text_wrap_mode == egui::TextWrapMode::Extend {
let min_desired_width = self.min_desired_width.unwrap_or(0.0);
DesiredWidth::Exact(measured_width.at_least(min_desired_width))
} else {
DesiredWidth::AtLeast(self.min_desired_width)
// If the user set an explicit min-width, use it.
// Otherwise, show at least `default_min_width`, unless the text is even short.
let default_min_width = 64.0;
let min_desired_width = self
.min_desired_width
.unwrap_or_else(|| measured_width.min(default_min_width));
DesiredWidth::AtLeast(min_desired_width)
}
}
}
15 changes: 8 additions & 7 deletions crates/re_ui/src/list_item/list_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,15 @@ impl ListItem {
};

let desired_width = match content.desired_width(ui) {
// // content will use all available width
// None => ui.available_width().at_least(extra_indent + collapse_extra),
// // content will use the required width
// Some(desired_width) => extra_indent + collapse_extra + desired_width,
DesiredWidth::Exact(width) => extra_indent + collapse_extra + width,
DesiredWidth::AtLeast(width) => ui
.available_width()
.at_least(extra_indent + collapse_extra + width),
DesiredWidth::AtLeast(width) => {
let total_width = extra_indent + collapse_extra + width;
if ui.is_sizing_pass() {
total_width
} else {
ui.available_width().at_least(total_width)
}
}
};

let desired_size = egui::vec2(desired_width, height);
Expand Down
43 changes: 14 additions & 29 deletions crates/re_ui/src/list_item/property_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ struct PropertyActionButton<'a> {
pub struct PropertyContent<'a> {
label: egui::WidgetText,
min_desired_width: f32,
exact_width: bool,

icon_fn: Option<Box<IconFn<'a>>>,
show_only_when_collapsed: bool,
Expand All @@ -40,7 +39,6 @@ impl<'a> PropertyContent<'a> {
Self {
label: label.into(),
min_desired_width: 200.0,
exact_width: false,
icon_fn: None,
show_only_when_collapsed: true,
value_fn: None,
Expand All @@ -58,17 +56,6 @@ impl<'a> PropertyContent<'a> {
self
}

/// Allocate the exact width required for the entire content.
///
/// Note: this is done by tracking the maximum width in the current [`super::list_item_scope`]
/// during the previous frame, so this is effective on the second frame only. If the first frame
/// is actually rendered, this can lead to a flicker.
#[inline]
pub fn exact_width(mut self, exact_width: bool) -> Self {
self.exact_width = exact_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 @@ -218,7 +205,6 @@ impl ListItemContent for PropertyContent<'_> {
show_only_when_collapsed,
value_fn,
action_buttons,
exact_width: _,
} = *self;

// │ │
Expand Down Expand Up @@ -382,23 +368,22 @@ impl ListItemContent for PropertyContent<'_> {

fn desired_width(&self, ui: &Ui) -> DesiredWidth {
let layout_info = LayoutInfoStack::top(ui.ctx());
if self.exact_width {
if let Some(max_width) = layout_info.property_content_max_width {
let mut desired_width = max_width + layout_info.left_x - ui.max_rect().left();

// TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui`
let action_button_dimension =
DesignTokens::small_icon_size().x + 2.0 * ui.spacing().button_padding.x;
let reserve_action_button_space =
self.action_buttons.is_some() || layout_info.reserve_action_button_space;
if reserve_action_button_space {
desired_width += action_button_dimension + DesignTokens::text_to_icon_padding();
}

DesiredWidth::Exact(desired_width.ceil())
} else {
DesiredWidth::AtLeast(self.min_desired_width)
if crate::is_in_resizable_area(ui) {
DesiredWidth::AtLeast(self.min_desired_width)
} else if let Some(max_width) = layout_info.property_content_max_width {
let mut desired_width = max_width + layout_info.left_x - ui.max_rect().left();

// TODO(ab): ideally there wouldn't be as much code duplication with `Self::ui`
let action_button_dimension =
DesignTokens::small_icon_size().x + 2.0 * ui.spacing().button_padding.x;
let reserve_action_button_space =
self.action_buttons.is_some() || layout_info.reserve_action_button_space;
if reserve_action_button_space {
desired_width += action_button_dimension + DesignTokens::text_to_icon_padding();
}

DesiredWidth::AtLeast(desired_width.ceil())
} else {
DesiredWidth::AtLeast(self.min_desired_width)
}
Expand Down
Loading
Loading