-
Notifications
You must be signed in to change notification settings - Fork 370
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
Conversation
d9d0781
to
d8f8f2b
Compare
This ensures tooltips with `ListItem`s don't get unnecessarily wide
c8cc940
to
805811f
Compare
# Conflicts: # crates/re_data_ui/src/item_ui.rs # crates/re_time_panel/src/lib.rs # crates/re_ui/src/lib.rs # crates/re_ui/src/list_item/label_content.rs # crates/re_ui/src/list_item/list_item.rs # crates/re_ui/src/list_item/mod.rs
// avoid the list item to max the tooltip width | ||
if ui_layout == UiLayout::Tooltip { | ||
content = content.exact_width(true); | ||
} |
There was a problem hiding this comment.
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
@@ -626,7 +626,7 @@ impl TimePanel { | |||
ctx, | |||
&InstancePath::from(tree.path.clone()), | |||
)) | |||
.exact_width(true), | |||
.truncate(false), |
There was a problem hiding this comment.
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
@@ -417,8 +417,7 @@ pub fn component_path_button_to( | |||
} else { | |||
"Temporal component" | |||
}) | |||
.with_icon(icon) | |||
.exact_width(true), |
There was a problem hiding this comment.
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
ListItem
in tooltipsListItem
in tooltips and other popups
crates/re_ui/src/lib.rs
Outdated
/// Used as a heuristic to figure out if it is safe to truncate text. | ||
/// | ||
/// If this returns false, we should never truncate. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
What
ListItem
in tooltips #6516This PR lets us put any
ListItem
anywhere without trouble.This is because of two things:
ListItem
understands the concept of an egui sizing pass, so won't expand to become huge when in a tooltipLabelContent
andPropertyContent
can tell when it is in a tooltip, and in these cases won't truncate the contentsChecklist
main
build: rerun.io/viewernightly
build: rerun.io/viewerTo run all checks from
main
, comment on the PR with@rerun-bot full-check
.