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

Move Visible Time Range higher in the Selection Panel #5036

Merged
merged 6 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,15 @@ fn blueprint_ui_for_space_view(
.cloned()
.unwrap_or(resolved_entity_props.clone());

let cursor = ui.cursor();
visible_history_ui(
ctx,
ui,
&space_view_class,
true,
None,
&mut props.visible_history,
&resolved_entity_props.visible_history,
);

space_view
.class(ctx.space_view_class_registry)
Expand All @@ -912,22 +920,6 @@ fn blueprint_ui_for_space_view(
&mut props,
);

if cursor != ui.cursor() {
// add some space if something was rendered by selection_ui
//TODO(ab): use design token
ui.add_space(16.0);
}

visible_history_ui(
ctx,
ui,
&space_view_class,
true,
None,
&mut props.visible_history,
&resolved_entity_props.visible_history,
);

root_data_result.save_override(Some(props), ctx);
}
}
Expand Down
64 changes: 41 additions & 23 deletions crates/re_viewer/src/ui/visible_history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use re_log_types::{EntityPath, TimeType, TimeZone};
use re_space_view_spatial::{SpatialSpaceView2D, SpatialSpaceView3D};
use re_space_view_time_series::TimeSeriesSpaceView;
use re_types_core::ComponentName;
use re_ui::{markdown_ui, ReUi};
use re_viewer_context::{SpaceViewClass, SpaceViewClassIdentifier, TimeControl, ViewerContext};

/// These space views support the Visible History feature.
Expand Down Expand Up @@ -91,7 +92,7 @@ pub fn visible_history_ui(

let mut interacting_with_controls = false;

let collapsing_response = re_ui.collapsing_header(ui, "Visible Time Range", true, |ui| {
let collapsing_response = re_ui.collapsing_header(ui, "Visible Time Range", false, |ui| {
ui.horizontal(|ui| {
re_ui
.radio_value(ui, &mut visible_history_prop.enabled, false, "Default")
Expand Down Expand Up @@ -173,6 +174,8 @@ pub fn visible_history_ui(
)
})
.inner;

current_range_ui(ctx, ui, current_time, is_sequence_timeline, visible_history);
} else {
resolved_visible_history_boundary_ui(
ctx,
Expand All @@ -181,35 +184,35 @@ pub fn visible_history_ui(
is_sequence_timeline,
true,
);

resolved_visible_history_boundary_ui(
ctx,
ui,
&resolved_visible_history.to,
is_sequence_timeline,
false,
);
}

current_range_ui(ctx, ui, current_time, is_sequence_timeline, visible_history);

ui.add(
egui::Label::new(
egui::RichText::new(if is_sequence_timeline {
"These settings apply to all sequence timelines."
} else {
"These settings apply to all temporal timelines."
})
.italics()
.weak(),
)
.wrap(true),
)
.on_hover_text(
"Visible Time Range properties are stored separately for each types of timelines. \
They may differ depending on whether the current timeline is temporal or a sequence.",
);
current_range_ui(
ctx,
ui,
current_time,
is_sequence_timeline,
resolved_visible_history,
);
}
});

// Add spacer after the visible history section.
//TODO(ab): figure out why `item_spacing.y` is added _only_ in collapsed state.
if collapsing_response.body_response.is_some() {
ui.add_space(ui.spacing().item_spacing.y / 2.0);
} else {
ui.add_space(-ui.spacing().item_spacing.y / 2.0);
}
ReUi::full_span_separator(ui);
ui.add_space(ui.spacing().item_spacing.y / 2.0);

// Decide when to show the visible history highlight in the timeline. The trick is that when
// interacting with the controls, the mouse might end up outside the collapsing header rect,
// so we must track these interactions specifically.
Expand All @@ -235,10 +238,25 @@ pub fn visible_history_ui(
}
}

collapsing_response.header_response.on_hover_text(
"Controls the time range used to display data in the Space View.\n\n\
Note that the data current as of the time range starting time is included.",
let markdown = format!("# Visible Time Range\n
This feature controls the time range used to display data in the Space View.

The settings are inherited from parent Group(s) or enclosing Space View if not overridden.

Visible Time Range properties are stored separately of each _types_ of timelines. The may differ depending on \
abey79 marked this conversation as resolved.
Show resolved Hide resolved
whether the current timeline is temporal or a sequence. The current settings apply to all {} timelines.
abey79 marked this conversation as resolved.
Show resolved Hide resolved

Notes that the data current as of the time range starting time is included.",
Copy link
Member

Choose a reason for hiding this comment

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

I vote to completely remove that part

if is_sequence_timeline {
"sequence"
} else {
"temporal"
}
);

collapsing_response.header_response.on_hover_ui(|ui| {
markdown_ui(ui, egui::Id::new(markdown.as_str()), &markdown);
});
}

fn current_range_ui(
Expand Down
Loading