Skip to content

Commit 70b3256

Browse files
authored
Unify button handling in LabelContent, PropertyContent and SectionHeader (#11164)
### Related * closes #6191 * closes #6203 * related to #10762 (RR-1532) ### What Unify button handling via new `ItemButtons` struct and introduce ext trait to unify convenience helper fns. This changes the default behavior of the PropertyContent to only show icon buttons on hover. In some places this may be desired (usually if there are a lot of items with the same buttons (e.g. blueprint panel)), sometimes not (e.g. if buttons are only on select items in a list, they should always be visible for discoverability). We should check that this should make sense for all property content buttons.
1 parent edd8c72 commit 70b3256

File tree

20 files changed

+384
-352
lines changed

20 files changed

+384
-352
lines changed

crates/viewer/re_blueprint_tree/src/blueprint_tree.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use re_data_ui::item_ui::guess_instance_path_icon;
88
use re_entity_db::InstancePath;
99
use re_log_types::{ApplicationId, EntityPath};
1010
use re_ui::filter_widget::format_matching_text;
11+
use re_ui::list_item::ListItemContentButtonsExt as _;
1112
use re_ui::{
1213
ContextExt as _, DesignTokens, UiExt as _, drag_and_drop::DropTarget, filter_widget, list_item,
1314
};
@@ -300,15 +301,12 @@ impl BlueprintTree {
300301
.label_style(contents_name_style(&container_data.name))
301302
.with_icon(icon_for_container_kind(&container_data.kind))
302303
.with_buttons(|ui| {
303-
let vis_response = visibility_button_ui(ui, parent_visible, &mut visible);
304+
visibility_button_ui(ui, parent_visible, &mut visible);
304305

305-
let remove_response = remove_button_ui(ui, "Remove container");
306-
if remove_response.clicked() {
306+
if remove_button_ui(ui, "Remove container").clicked() {
307307
viewport_blueprint.mark_user_interaction(ctx);
308308
viewport_blueprint.remove_contents(content);
309309
}
310-
311-
remove_response | vis_response
312310
});
313311

314312
// Globally unique id - should only be one of these in view at one time.
@@ -391,15 +389,12 @@ impl BlueprintTree {
391389
.with_icon(class.icon())
392390
.subdued(!view_visible)
393391
.with_buttons(|ui| {
394-
let vis_response = visibility_button_ui(ui, container_visible, &mut visible);
392+
visibility_button_ui(ui, container_visible, &mut visible);
395393

396-
let response = remove_button_ui(ui, "Remove view from the viewport");
397-
if response.clicked() {
394+
if remove_button_ui(ui, "Remove view from the viewport").clicked() {
398395
viewport_blueprint.mark_user_interaction(ctx);
399396
viewport_blueprint.remove_contents(Contents::View(view_data.id));
400397
}
401-
402-
response | vis_response
403398
});
404399

405400
// Globally unique id - should only be one of these in view at one time.
@@ -518,22 +513,20 @@ impl BlueprintTree {
518513
.subdued(!view_visible || !data_result_data.visible)
519514
.with_buttons(|ui: &mut egui::Ui| {
520515
let mut visible_after = data_result_data.visible;
521-
let vis_response =
522-
visibility_button_ui(ui, view_visible, &mut visible_after);
516+
visibility_button_ui(ui, view_visible, &mut visible_after);
523517
if visible_after != data_result_data.visible {
524518
data_result_data.update_visibility(ctx, visible_after);
525519
}
526520

527-
let response = remove_button_ui(
521+
if remove_button_ui(
528522
ui,
529523
"Remove this entity and all its children from the view",
530-
);
531-
if response.clicked() {
524+
)
525+
.clicked()
526+
{
532527
data_result_data
533528
.remove_data_result_from_view(ctx, viewport_blueprint);
534529
}
535-
536-
response | vis_response
537530
})
538531
}
539532
}

crates/viewer/re_data_ui/src/item_ui.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use re_types::{
99
archetypes::RecordingInfo,
1010
components::{Name, Timestamp},
1111
};
12+
use re_ui::list_item::ListItemContentButtonsExt as _;
1213
use re_ui::{SyntaxHighlighting as _, UiExt as _, icons, list_item};
1314
use re_viewer_context::{
1415
HoverHighlight, Item, SystemCommand, SystemCommandSender as _, UiLayout, ViewId, ViewerContext,
@@ -740,7 +741,6 @@ pub fn entity_db_button_ui(
740741
store_id.clone().into(),
741742
));
742743
}
743-
resp
744744
});
745745
}
746746

@@ -812,7 +812,6 @@ pub fn table_id_button_ui(
812812
table_id.clone().into(),
813813
));
814814
}
815-
resp
816815
});
817816
}
818817

crates/viewer/re_recording_panel/src/recording_panel_ui.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use re_data_ui::item_ui::{entity_db_button_ui, table_id_button_ui};
77
use re_log_types::TableId;
88
use re_redap_browser::{Command, EXAMPLES_ORIGIN, LOCAL_ORIGIN, RedapServers};
99
use re_smart_channel::SmartChannelSource;
10-
use re_ui::list_item::{ItemButton as _, ItemMenuButton, LabelContent};
10+
use re_ui::list_item::{ItemMenuButton, LabelContent, ListItemContentButtonsExt as _};
1111
use re_ui::{UiExt as _, UiLayout, icons, list_item};
1212
use re_viewer_context::{
1313
DisplayMode, Item, RecordingOrTable, SystemCommand, SystemCommandSender as _, ViewerContext,
@@ -219,9 +219,9 @@ fn server_section_ui(
219219
} = server_data;
220220

221221
let content = list_item::LabelContent::header(origin.host.to_string())
222-
.always_show_buttons(true)
222+
.with_always_show_buttons(true)
223223
.with_buttons(|ui| {
224-
Box::new(ItemMenuButton::new(&icons::MORE, "Actions", move |ui| {
224+
ItemMenuButton::new(&icons::MORE, "Actions", move |ui| {
225225
if icons::RESET
226226
.as_button_with_label(ui.tokens(), "Refresh")
227227
.ui(ui)
@@ -243,8 +243,8 @@ fn server_section_ui(
243243
{
244244
servers.send_command(Command::RemoveServer(origin.clone()));
245245
}
246-
}))
247-
.ui(ui)
246+
})
247+
.ui(ui);
248248
});
249249

250250
let item_response = ui
@@ -358,8 +358,6 @@ fn dataset_entry_ui(
358358
));
359359
}
360360
}
361-
362-
resp
363361
});
364362
}
365363

@@ -498,7 +496,6 @@ fn app_id_section_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, local_app_id: &
498496
ctx.command_sender()
499497
.send_system(SystemCommand::CloseApp(app_id.clone()));
500498
}
501-
resp
502499
});
503500
}
504501

@@ -565,8 +562,6 @@ fn receiver_ui(
565562
if resp.clicked() {
566563
ctx.connected_receivers.remove(receiver);
567564
}
568-
569-
resp
570565
});
571566

572567
if show_hierarchal {

crates/viewer/re_selection_panel/src/defaults_ui.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use re_data_ui::{DataUi as _, archetype_label_list_item_ui};
99
use re_log_types::EntityPath;
1010
use re_types_core::ComponentDescriptor;
1111
use re_types_core::reflection::ComponentDescriptorExt as _;
12+
use re_ui::list_item::ListItemContentButtonsExt as _;
1213
use re_ui::{SyntaxHighlighting as _, UiExt as _, list_item::LabelContent};
1314
use re_viewer_context::{
1415
ComponentUiTypes, QueryContext, SystemCommand, SystemCommandSender as _, UiLayout, ViewContext,
@@ -90,8 +91,8 @@ Click on the `+` button to add a new default value.";
9091
active_default_ui(ctx, ui, &active_defaults, view, query, db);
9192
};
9293
ui.section_collapsing_header("Component defaults")
93-
.button(add_button)
94-
.help_markdown(markdown)
94+
.with_button(add_button)
95+
.with_help_markdown(markdown)
9596
.show(ui, body);
9697
}
9798

@@ -146,7 +147,7 @@ fn active_default_ui(
146147
let response = ui.list_item_flat_noninteractive(
147148
re_ui::list_item::PropertyContent::new(component_descr.archetype_field_name())
148149
.min_desired_width(150.0)
149-
.action_button(&re_ui::icons::CLOSE, "Clear blueprint component", || {
150+
.with_action_button(&re_ui::icons::CLOSE, "Clear blueprint component", || {
150151
ctx.clear_blueprint_component(
151152
view.defaults_path.clone(),
152153
component_descr.clone(),

crates/viewer/re_selection_panel/src/selection_panel.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use re_data_ui::{
99
use re_entity_db::{EntityPath, InstancePath};
1010
use re_log_types::{ComponentPath, EntityPathFilter, EntityPathSubs, ResolvedEntityPathFilter};
1111
use re_types::ComponentDescriptor;
12+
use re_ui::list_item::ListItemContentButtonsExt as _;
1213
use re_ui::{
1314
SyntaxHighlighting as _, UiExt as _, icons,
1415
list_item::{self, PropertyContent},
@@ -428,7 +429,7 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included.
428429

429430
if let Some(view) = viewport.view(view_id) {
430431
ui.section_collapsing_header("Entity path filter")
431-
.button(
432+
.with_button(
432433
list_item::ItemActionButton::new(
433434
&re_ui::icons::EDIT,
434435
"Add new entity…",
@@ -438,7 +439,7 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included.
438439
)
439440
.hover_text("Modify the entity query using the editor"),
440441
)
441-
.help_markdown(markdown)
442+
.with_help_markdown(markdown)
442443
.show(ui, |ui| {
443444
// TODO(#6075): Because `list_item_scope` changes it. Temporary until everything is `ListItem`.
444445
ui.spacing_mut().item_spacing.y = ui.ctx().style().spacing.item_spacing.y;
@@ -710,7 +711,7 @@ fn container_children(
710711
};
711712

712713
ui.section_collapsing_header("Contents")
713-
.button(
714+
.with_button(
714715
list_item::ItemActionButton::new(&re_ui::icons::ADD, "Add to container", || {
715716
show_add_view_or_container_modal(*container_id);
716717
})
@@ -999,8 +1000,6 @@ fn show_list_item_for_container_child(
9991000
if response.clicked() {
10001001
remove_contents = true;
10011002
}
1002-
1003-
response
10041003
}),
10051004
)
10061005
}
@@ -1025,8 +1024,6 @@ fn show_list_item_for_container_child(
10251024
if response.clicked() {
10261025
remove_contents = true;
10271026
}
1028-
1029-
response
10301027
}),
10311028
)
10321029
}

crates/viewer/re_selection_panel/src/visible_time_range_ui.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use re_types::{
77
blueprint::{archetypes as blueprint_archetypes, components::VisibleTimeRange},
88
datatypes::{TimeInt, TimeRange, TimeRangeBoundary},
99
};
10+
use re_ui::list_item::ListItemContentButtonsExt as _;
1011
use re_ui::{TimeDragValue, UiExt as _};
1112
use re_viewer_context::{QueryRange, ViewClass, ViewState, ViewerContext};
1213
use re_viewport_blueprint::{ViewBlueprint, entity_path_for_view_property};
@@ -173,7 +174,7 @@ Notes:
173174
let collapsing_response = ui
174175
.section_collapsing_header("Visible time range")
175176
.default_open(true)
176-
.help_markdown(markdown)
177+
.with_help_markdown(markdown)
177178
.show(ui, |ui| {
178179
ui.horizontal(|ui| {
179180
ui.re_radio_value(has_individual_time_range, false, "Default")

crates/viewer/re_selection_panel/src/visualizer_ui.rs

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use re_log_types::{ComponentPath, EntityPath};
88
use re_types::blueprint::archetypes::VisualizerOverrides;
99
use re_types::{ComponentDescriptor, reflection::ComponentDescriptorExt as _};
1010
use re_types_core::external::arrow::array::ArrayRef;
11+
use re_ui::list_item::ListItemContentButtonsExt as _;
1112
use re_ui::{UiExt as _, design_tokens_of_visuals, list_item};
1213
use re_view::latest_at_with_blueprint_resolved_data;
1314
use re_viewer_context::{
@@ -74,8 +75,8 @@ in the blueprint or in the UI by selecting the view.
7475
specific to the visualizer and the current view type.";
7576

7677
ui.section_collapsing_header("Visualizers")
77-
.button(button)
78-
.help_markdown(markdown)
78+
.with_button(button)
79+
.with_help_markdown(markdown)
7980
.show(ui, |ui| {
8081
visualizer_ui_impl(ctx, ui, &data_result, &active_visualizers, &all_visualizers);
8182
});
@@ -129,17 +130,21 @@ pub fn visualizer_ui_impl(
129130
),
130131
)
131132
.min_desired_width(150.0)
132-
.with_buttons(|ui| remove_visualizer_button(ui, visualizer_id))
133-
.always_show_buttons(true),
133+
.with_buttons(|ui| {
134+
remove_visualizer_button(ui, visualizer_id);
135+
})
136+
.with_always_show_buttons(true),
134137
);
135138
visualizer_components(ctx, ui, data_result, visualizer);
136139
} else {
137140
ui.list_item_flat_noninteractive(
138141
list_item::LabelContent::new(format!("{visualizer_id} (unknown visualizer)"))
139142
.weak(true)
140143
.min_desired_width(150.0)
141-
.with_buttons(|ui| remove_visualizer_button(ui, visualizer_id))
142-
.always_show_buttons(true),
144+
.with_buttons(|ui| {
145+
remove_visualizer_button(ui, visualizer_id);
146+
})
147+
.with_always_show_buttons(true),
143148
);
144149
}
145150
}
@@ -406,22 +411,21 @@ fn visualizer_components(
406411
)
407412
.value_fn(value_fn)
408413
.show_only_when_collapsed(false)
409-
.menu_button(
410-
&re_ui::icons::MORE,
411-
"More options",
412-
|ui: &mut egui::Ui| {
413-
menu_more(
414-
ctx,
415-
ui,
416-
component_descr.clone(),
417-
override_path,
418-
&raw_override.clone().map(|(_, raw_override)| raw_override),
419-
raw_default.clone().map(|(_, raw_override)| raw_override),
420-
raw_fallback.clone(),
421-
raw_current_value.clone(),
422-
);
423-
},
424-
),
414+
.with_menu_button(&re_ui::icons::MORE, "More options", |ui: &mut egui::Ui| {
415+
menu_more(
416+
ctx,
417+
ui,
418+
component_descr.clone(),
419+
override_path,
420+
&raw_override.clone().map(|(_, raw_override)| raw_override),
421+
raw_default.clone().map(|(_, raw_override)| raw_override),
422+
raw_fallback.clone(),
423+
raw_current_value.clone(),
424+
);
425+
})
426+
// TODO(emilk/egui#7531): Ideally we would hide the button unless hovered, but this
427+
// currently breaks the menu.
428+
.with_always_show_buttons(true),
425429
add_children,
426430
)
427431
.item_response;
@@ -460,7 +464,7 @@ fn editable_blueprint_component_list_item(
460464
allow_multiline,
461465
);
462466
})
463-
.action_button(&re_ui::icons::CLOSE, "Clear blueprint component", || {
467+
.with_action_button(&re_ui::icons::CLOSE, "Clear blueprint component", || {
464468
query_ctx
465469
.viewer_ctx()
466470
.clear_blueprint_component(blueprint_path, component_descr.clone());

crates/viewer/re_ui/examples/re_ui_example/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod hierarchical_drag_and_drop;
33
mod right_panel;
44

55
use egui::{Modifiers, os};
6+
use re_ui::list_item::ListItemContentButtonsExt as _;
67
use re_ui::{
78
CommandPalette, CommandPaletteAction, CommandPaletteUrl, ContextExt as _, DesignTokens, Help,
89
IconText, UICommand, UICommandSender, UiExt as _,
@@ -250,7 +251,7 @@ impl eframe::App for ExampleApp {
250251
// ---
251252

252253
ui.section_collapsing_header("Data")
253-
.button(list_item::ItemMenuButton::new(
254+
.with_button(list_item::ItemMenuButton::new(
254255
&re_ui::icons::ADD,
255256
"Add",
256257
|ui| {

0 commit comments

Comments
 (0)