Skip to content

Commit

Permalink
Addressed review comments from #6161
Browse files Browse the repository at this point in the history
  • Loading branch information
abey79 committed May 2, 2024
1 parent 87a58b5 commit f807b6f
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 42 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions crates/re_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,15 @@ re_format.workspace = true
re_log.workspace = true
re_log_types.workspace = true # syntax-highlighting for EntityPath

egui.workspace = true
egui_commonmark = { workspace = true, features = ["pulldown_cmark"] }
egui_extras.workspace = true
egui.workspace = true
parking_lot.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
strum_macros.workspace = true
strum.workspace = true
strum_macros.workspace = true
sublime_fuzzy.workspace = true
once_cell.workspace = true


eframe = { workspace = true, default-features = false, features = ["wgpu"] }
Expand Down
11 changes: 0 additions & 11 deletions crates/re_ui/examples/re_ui_example/right_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,17 +285,6 @@ impl RightPanel {
true,
list_item2::LabelContent::new("Other contents:"),
|re_ui, ui| {
re_ui.list_item2().show_hierarchical(
ui,
list_item2::LabelContent::new("next line is a EmptyContent:")
.subdued(true)
.italics(true),
);

re_ui
.list_item2()
.show_hierarchical(ui, list_item2::EmptyContent);

re_ui.list_item2().show_hierarchical(
ui,
list_item2::DebugContent::default()
Expand Down
2 changes: 1 addition & 1 deletion crates/re_ui/src/drag_and_drop.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Helpers for drag and drop support for reordering hierarchical lists.
//!
//! Works well in combination with [`crate::ListItem`].
//! Works well in combination with [`crate::list_item2::ListItem`].
pub enum ItemKind<ItemId: Copy> {
/// Root container item.
Expand Down
14 changes: 13 additions & 1 deletion crates/re_ui/src/list_item2/list_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,19 @@ pub struct ShowCollapsingResponse<R> {
pub body_response: Option<egui::InnerResponse<R>>,
}

/// Generic list item that delegates its content to a [`ListItemContent`] implementation.
/// Content-generic list item.
///
/// The following features are supported:
/// - Flat or collapsible hierarchical lists.
/// - Full-span background highlighting via [`super::list_item_scope`]. TODO(#6156): fix reference
/// - Interactive or not.
/// - Support for drag and drop with [`crate::drag_and_drop`].
///
/// Besides these core features, [`ListItem`] delegates all content to the [`ListItemContent`]
/// implementations, such as [`super::LabelContent`] and [`super::PropertyContent`].
///
/// Usage example can be found in `re_ui_example`.
#[derive(Debug, Clone)]
pub struct ListItem<'a> {
re_ui: &'a ReUi,
Expand Down
3 changes: 3 additions & 0 deletions crates/re_ui/src/list_item2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub struct ContentContext<'a> {
pub rect: egui::Rect,

/// Background area
///
/// This is the area covered by the full-span highlighting. Useful for testing if the cursor is
/// over the item.
pub bg_rect: egui::Rect,

/// List item response.
Expand Down
29 changes: 15 additions & 14 deletions crates/re_ui/src/list_item2/other_contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,36 @@ use crate::list_item2::{ContentContext, DesiredWidth, ListItemContent};
use crate::ReUi;
use egui::Ui;

/// Empty list item content.
pub struct EmptyContent;

impl ListItemContent for EmptyContent {
fn ui(
self: Box<Self>,
_re_ui: &crate::ReUi,
_ui: &mut egui::Ui,
_context: &ContentContext<'_>,
) {
}
}

/// [`ListItemContent`] that delegates to a closure.
#[allow(clippy::type_complexity)]
pub struct CustomContent<'a> {
ui: Box<dyn FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) + 'a>,
desired_width: DesiredWidth,
}

impl<'a> CustomContent<'a> {
pub fn new(ui: impl FnOnce(&crate::ReUi, &mut egui::Ui, &ContentContext<'_>) + 'a) -> Self {
Self { ui: Box::new(ui) }
Self {
ui: Box::new(ui),
desired_width: Default::default(),
}
}

#[inline]
pub fn with_desired_width(mut self, desired_width: DesiredWidth) -> Self {
self.desired_width = desired_width;
self
}
}

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);
}

fn desired_width(&self, _re_ui: &ReUi, _ui: &Ui) -> DesiredWidth {
self.desired_width
}
}

/// [`ListItemContent`] that displays the content rect.
Expand Down
18 changes: 7 additions & 11 deletions crates/re_ui/src/list_item2/scope.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use egui::NumExt;
use once_cell::sync::Lazy;

#[derive(Debug, Clone, PartialEq)]
pub struct State {
/// X coordinate span to use for hover/selection highlight.
///
/// Note: this field is not, strictly speaking, part of the state, as it's overwritten with each
/// call of `list_item_scope`. Still, it's convenient to have it here to pass it from the scope
/// to the inner `ListItem`.
/// Note: `list_item_scope` always overwrite this field before pushing the [`State`] to the
/// stack.
// TODO(#6156): this being here is a (temporary) hack (see docstring). In the future, this will
// be generalized to some `full_span_scope` mechanism to be used by all full-span widgets beyond
// `ListItem`.
Expand Down Expand Up @@ -78,19 +76,17 @@ impl State {
#[derive(Debug, Clone, Default)]
pub(crate) struct StateStack(Vec<State>);

static STATE_STACK_ID: Lazy<egui::Id> = Lazy::new(|| egui::Id::new("re_ui_list_item_state_stack"));

impl StateStack {
fn push(ctx: &egui::Context, state: State) {
ctx.data_mut(|writer| {
let stack: &mut StateStack = writer.get_temp_mut_or_default(*STATE_STACK_ID);
let stack: &mut StateStack = writer.get_temp_mut_or_default(egui::Id::NULL);
stack.0.push(state);
});
}

fn pop(ctx: &egui::Context) -> Option<State> {
ctx.data_mut(|writer| {
let stack: &mut StateStack = writer.get_temp_mut_or_default(*STATE_STACK_ID);
let stack: &mut StateStack = writer.get_temp_mut_or_default(egui::Id::NULL);
stack.0.pop()
})
}
Expand All @@ -102,7 +98,7 @@ impl StateStack {
/// [`super::list_item_scope`].
pub(crate) fn top(ctx: &egui::Context) -> State {
ctx.data_mut(|writer| {
let stack: &mut StateStack = writer.get_temp_mut_or_default(*STATE_STACK_ID);
let stack: &mut StateStack = writer.get_temp_mut_or_default(egui::Id::NULL);
let state = stack.0.last();
if state.is_none() {
re_log::warn_once!(
Expand All @@ -120,7 +116,7 @@ impl StateStack {
/// empty, the closure is not called and a warning is logged.
pub(crate) fn top_mut(ctx: &egui::Context, state_writer: impl FnOnce(&mut State)) {
ctx.data_mut(|writer| {
let stack: &mut StateStack = writer.get_temp_mut_or_default(*STATE_STACK_ID);
let stack: &mut StateStack = writer.get_temp_mut_or_default(egui::Id::NULL);
let state = stack.0.last_mut();
if let Some(state) = state {
state_writer(state);
Expand All @@ -135,7 +131,7 @@ impl StateStack {

fn peek(ctx: &egui::Context) -> Option<State> {
ctx.data_mut(|writer| {
let stack: &mut StateStack = writer.get_temp_mut_or_default(*STATE_STACK_ID);
let stack: &mut StateStack = writer.get_temp_mut_or_default(egui::Id::NULL);
stack.0.last().cloned()
})
}
Expand Down

0 comments on commit f807b6f

Please sign in to comment.