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

Much nicer looking error and warning messages #8127

Merged
merged 13 commits into from
Nov 14, 2024
6 changes: 3 additions & 3 deletions crates/viewer/re_data_ui/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use egui::NumExt;
use re_chunk_store::UnitChunkShared;
use re_entity_db::InstancePath;
use re_log_types::{ComponentPath, Instance, TimeInt};
use re_ui::{ContextExt as _, SyntaxHighlighting as _};
use re_ui::{ContextExt as _, SyntaxHighlighting as _, UiExt};
use re_viewer_context::{UiLayout, ViewerContext};

use super::DataUi;
Expand Down Expand Up @@ -81,11 +81,11 @@ impl<'a> DataUi for ComponentPathLatestAtResults<'a> {
*component_name,
);
if temporal_message_count > 0 {
ui.label(ui.ctx().error_text(format!(
ui.error_label(&format!(
"Static component has {} event{} logged on timelines",
temporal_message_count,
if temporal_message_count > 1 { "s" } else { "" }
)))
))
.on_hover_text(
"Components should be logged either as static or on timelines, but \
never both. Values for static components logged to timelines cannot be \
Expand Down
7 changes: 2 additions & 5 deletions crates/viewer/re_data_ui/src/component_path.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use re_log_types::ComponentPath;
use re_ui::ContextExt as _;
use re_ui::UiExt;
use re_viewer_context::{UiLayout, ViewerContext};

use super::DataUi;
Expand Down Expand Up @@ -47,10 +47,7 @@ impl DataUi for ComponentPath {
));
}
} else {
ui.label(
ui.ctx()
.error_text(format!("Unknown component path: {self}")),
);
ui.error_label(&format!("Unknown component path: {self}"));
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions crates/viewer/re_data_ui/src/instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use re_types::{
image::ImageKind,
static_assert_struct_has_fields, Archetype, ComponentName, Loggable,
};
use re_ui::{ContextExt as _, UiExt as _};
use re_ui::UiExt as _;
use re_viewer_context::{
gpu_bridge::image_data_range_heuristic, ColormapWithRange, HoverHighlight, ImageInfo,
ImageStatsCache, Item, UiLayout, ViewerContext,
Expand Down Expand Up @@ -46,11 +46,7 @@ impl DataUi for InstancePath {
.store()
.all_components_on_timeline(&query.timeline(), entity_path)
} else {
ui_layout.label(
ui,
ui.ctx()
.error_text(format!("Unknown entity: {entity_path:?}")),
);
ui.error_label(&format!("Unknown entity: {entity_path:?}"));
return;
};
let Some(components) = component else {
Expand Down
30 changes: 23 additions & 7 deletions crates/viewer/re_space_view_spatial/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use re_types::{
archetypes::Pinhole, blueprint::components::VisualBounds2D, components::ViewCoordinates,
image::ImageKind,
};
use re_ui::{ContextExt as _, UiExt as _};
use re_ui::UiExt as _;
use re_viewer_context::{
HoverHighlight, SelectionHighlight, SpaceViewHighlights, SpaceViewState, ViewerContext,
};
Expand Down Expand Up @@ -214,11 +214,12 @@ pub fn create_labels(
};

let font_id = egui::TextStyle::Body.resolve(parent_ui.style());
let format = match label.style {
UiLabelStyle::Color(color) => egui::TextFormat::simple(font_id, color),
UiLabelStyle::Error => parent_ui.ctx().error_text_format(),
let is_error = matches!(label.style, UiLabelStyle::Error);
let text_color = match label.style {
UiLabelStyle::Color(color) => color,
UiLabelStyle::Error => parent_ui.style().visuals.strong_text_color(),
};
let text_color = format.color;
let format = egui::TextFormat::simple(font_id, text_color);

let galley = parent_ui.fonts(|fonts| {
fonts.layout_job({
Expand Down Expand Up @@ -249,7 +250,13 @@ pub fn create_labels(
.index_highlight(label.labeled_instance.instance);
let background_color = match highlight.hover {
HoverHighlight::None => match highlight.selection {
SelectionHighlight::None => parent_ui.style().visuals.panel_fill,
SelectionHighlight::None => {
if is_error {
parent_ui.error_label_background_color()
} else {
parent_ui.style().visuals.panel_fill
}
}
SelectionHighlight::SiblingSelection => {
parent_ui.style().visuals.widgets.active.bg_fill
}
Expand All @@ -258,7 +265,16 @@ pub fn create_labels(
HoverHighlight::Hovered => parent_ui.style().visuals.widgets.hovered.bg_fill,
};

label_shapes.push(egui::Shape::rect_filled(bg_rect, 3.0, background_color));
let rect_stroke = if is_error {
egui::Stroke::new(1.0, parent_ui.style().visuals.error_fg_color)
} else {
egui::Stroke::NONE
};

label_shapes.push(
egui::epaint::RectShape::new(bg_rect.expand(4.0), 4.0, background_color, rect_stroke)
.into(),
);
label_shapes.push(egui::Shape::galley(
text_rect.center_top(),
galley,
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_space_view_tensor/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use re_types::{
datatypes::{TensorData, TensorDimension},
SpaceViewClassIdentifier, View,
};
use re_ui::{list_item, ContextExt as _, UiExt as _};
use re_ui::{list_item, UiExt as _};
use re_viewer_context::{
gpu_bridge, ApplicableEntities, ColormapWithRange, IdentifiedViewSystem as _,
IndicatedEntities, PerVisualizer, SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId,
Expand Down Expand Up @@ -282,7 +282,7 @@ impl TensorSpaceView {
if let Err(err) =
self.tensor_slice_ui(ctx, ui, state, view_id, dimension_labels, &slice_selection)
{
ui.label(ui.ctx().error_text(err.to_string()));
ui.error_label(&err.to_string());
}
});

Expand Down
21 changes: 13 additions & 8 deletions crates/viewer/re_ui/examples/re_ui_example/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,14 +457,19 @@ impl egui_tiles::Behavior<Tab> for MyTileTreeBehavior {
_tile_id: egui_tiles::TileId,
_pane: &mut Tab,
) -> egui_tiles::UiResponse {
egui::warn_if_debug_build(ui);
ui.label("Hover me for a tooltip")
.on_hover_text("This is a tooltip");

ui.label(
egui::RichText::new("Welcome to the ReUi example")
.text_style(DesignTokens::welcome_screen_h1()),
);
egui::Frame::none().inner_margin(4.0).show(ui, |ui| {
egui::warn_if_debug_build(ui);
ui.label("Hover me for a tooltip")
.on_hover_text("This is a tooltip");

ui.label(
egui::RichText::new("Welcome to the ReUi example")
.text_style(DesignTokens::welcome_screen_h1()),
);

ui.error_label("This is an example of a long error label.");
ui.warning_label("This is an example of a long warning label.");
});

Default::default()
}
Expand Down
9 changes: 8 additions & 1 deletion crates/viewer/re_ui/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,14 @@ impl UICommand {
Self::ToggleSelectionPanel => Some(ctrl_shift(Key::S)),
Self::ToggleTimePanel => Some(ctrl_shift(Key::T)),
Self::ToggleChunkStoreBrowser => Some(ctrl_shift(Key::D)),
Self::Settings => None,
Self::Settings => {
if cfg!(target_os = "macos") {
Some(KeyboardShortcut::new(Modifiers::MAC_CMD, Key::Comma))
} else {
// TODO(emilk): shortcut for web and non-mac too
None
}
}

#[cfg(debug_assertions)]
Self::ToggleBlueprintInspectionPanel => Some(ctrl_shift(Key::I)),
Expand Down
21 changes: 9 additions & 12 deletions crates/viewer/re_ui/src/context_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,35 +59,32 @@ pub trait ContextExt {
// egui::Stroke::new(stroke_width, color)
}

/// Text colored to indicate success.
#[must_use]
fn success_text(&self, text: impl Into<String>) -> egui::RichText {
egui::RichText::new(text).color(SUCCESS_COLOR)
}

/// Text colored to indicate a warning.
///
/// For most cases, you should use [`crate::UiExt::warning_label`] instead,
/// which has a nice fat border around it.
#[must_use]
fn warning_text(&self, text: impl Into<String>) -> egui::RichText {
let style = self.ctx().style();
egui::RichText::new(text).color(style.visuals.warn_fg_color)
}

/// NOTE: duplicated in [`Self::error_text_format`]
/// Text colored to indicate an error.
///
/// For most cases, you should use [`crate::UiExt::error_label`] instead,
/// which has a nice fat border around it.
#[must_use]
fn error_text(&self, text: impl Into<String>) -> egui::RichText {
let style = self.ctx().style();
egui::RichText::new(text).color(style.visuals.error_fg_color)
}

/// NOTE: duplicated in [`Self::error_text`]
fn error_text_format(&self) -> egui::TextFormat {
let style = self.ctx().style();
let font_id = egui::TextStyle::Body.resolve(&style);
egui::TextFormat {
font_id,
color: style.visuals.error_fg_color,
..Default::default()
}
}

fn top_bar_style(&self, style_like_web: bool) -> TopBarStyle {
let egui_zoom_factor = self.ctx().zoom_factor();
let fullscreen = self
Expand Down
3 changes: 3 additions & 0 deletions crates/viewer/re_ui/src/design_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ impl DesignTokens {

egui_style.visuals.image_loading_spinners = false;

egui_style.visuals.error_fg_color = egui::Color32::from_rgb(0xAB, 0x01, 0x16);
egui_style.visuals.warn_fg_color = egui::Color32::from_rgb(0xFF, 0x7A, 0x0C);

ctx.set_style(egui_style);
}

Expand Down
94 changes: 66 additions & 28 deletions crates/viewer/re_ui/src/ui_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,52 +8,90 @@ use egui::{
use crate::{
design_tokens, icons, list_item,
list_item::{LabelContent, ListItem},
ContextExt, DesignTokens, Icon, LabelStyle,
DesignTokens, Icon, LabelStyle,
};

static FULL_SPAN_TAG: &str = "rerun_full_span";

fn error_label_bg_color(fg_color: Color32) -> Color32 {
fg_color.gamma_multiply(0.35)
}

fn warning_or_error_label(
ui: &mut egui::Ui,
fg_color: Color32,
visible_text: &str,
full_text: &str,
) -> egui::Response {
egui::Frame::none()
.stroke((1.0, fg_color))
.fill(error_label_bg_color(fg_color))
.rounding(4.0)
.inner_margin(4.0)
.show(ui, |ui| {
ui.horizontal(|ui| {
ui.spacing_mut().item_spacing.x = 4.0;
ui.colored_label(fg_color, "⚠");
ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Wrap);
let response = ui.strong(visible_text).on_hover_ui(|ui| {
if visible_text != full_text {
ui.label(full_text);
ui.add_space(8.0);
}
ui.label("Click to copy text.");
});
if response.clicked() {
ui.ctx().copy_text(full_text.to_owned());
};
});
})
.response
}

/// Rerun custom extensions to [`egui::Ui`].
pub trait UiExt {
fn ui(&self) -> &egui::Ui;
fn ui_mut(&mut self) -> &mut egui::Ui;

/// Show the label with a success color.
fn success_label(&mut self, text: &str) -> egui::Response {
let label = egui::Label::new(self.ui().ctx().success_text(text));
self.ui_mut().add(label)
}

/// Shows a warning label.
/// Shows a warning label with a large border.
///
/// If you don't want a border, use [`crate::ContextExt::warning_text`].
fn warning_label(&mut self, warning_text: &str) -> egui::Response {
let label = egui::Label::new(self.ui().ctx().warning_text(warning_text));
self.ui_mut().add(label)
let ui = self.ui_mut();
warning_or_error_label(
ui,
ui.style().visuals.warn_fg_color,
warning_text,
warning_text,
)
}

/// Shows a small error label with the given text on hover and copies the text to the clipboard on click.
/// Shows a small error label with the given text on hover and copies the text to the clipboard on click with a large border.
///
/// This has a large border! If you don't want a border, use [`crate::ContextExt::error_text`].
fn error_with_details_on_hover(&mut self, error_text: &str) -> egui::Response {
let label = egui::Label::new(self.ui().ctx().error_text("Error"))
.selectable(false)
.sense(egui::Sense::click());
let response = self.ui_mut().add(label);
if response.clicked() {
self.ui().ctx().copy_text(error_text.to_owned());
}
response.on_hover_text(error_text)
let ui = self.ui_mut();
warning_or_error_label(ui, ui.style().visuals.error_fg_color, "Error", error_text)
}

fn error_label_background_color(&self) -> egui::Color32 {
error_label_bg_color(self.ui().style().visuals.error_fg_color)
}

/// Shows an error label with the entire error text and copies the text to the clipboard on click.
///
/// Use this only if you have a lot of space to spare.
/// Use this only if the error message is short, or you have a lot of room.
/// Otherwise, use [`Self::error_with_details_on_hover`].
///
/// This has a large border! If you don't want a border, use [`crate::ContextExt::error_text`].
fn error_label(&mut self, error_text: &str) -> egui::Response {
let label = egui::Label::new(self.ui().ctx().error_text(error_text))
.selectable(true)
.sense(egui::Sense::click());
let response = self.ui_mut().add(label).on_hover_text("Click to copy.");
if response.clicked() {
self.ui().ctx().copy_text(error_text.to_owned());
}
response
let ui = self.ui_mut();
warning_or_error_label(
ui,
ui.style().visuals.error_fg_color,
error_text,
error_text,
)
}

fn small_icon_button(&mut self, icon: &Icon) -> egui::Response {
Expand Down
Loading
Loading