Much nicer looking error and warning messages (#8127)

emilk · web-flow · commit 95b118f1ded4 · 2024-11-14T10:03:29.000+01:00
### What * Closes #8091 ## Before ![image](https://github.com/user-attachments/assets/e7df583a-0032-41e9-87b7-0a2a8a84579c) ![image](https://github.com/user-attachments/assets/8ca98429-e00d-4925-96c0-edfe0d986e11) ![image](https://github.com/user-attachments/assets/345d7023-066f-4741-a121-8ab372589ed6) ## After ![image](https://github.com/user-attachments/assets/1c86d97f-3fac-4f70-9ac5-d67b3418cf3d) ![image](https://github.com/user-attachments/assets/03f6d1e7-6474-43d3-aae3-d4573c416ee5) ![image](https://github.com/user-attachments/assets/07260f8b-a8a2-4223-92e5-da67521e88a6) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/8127?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/8127?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/8127) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. To deploy documentation changes immediately after merging this PR, add the `deploy docs` label.
diff --git a/crates/viewer/re_data_ui/src/component.rs b/crates/viewer/re_data_ui/src/component.rs
@@ -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;
@@ -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 \
diff --git a/crates/viewer/re_data_ui/src/component_path.rs b/crates/viewer/re_data_ui/src/component_path.rs
@@ -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;
@@ -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}"));
             }
         }
     }
diff --git a/crates/viewer/re_data_ui/src/instance_path.rs b/crates/viewer/re_data_ui/src/instance_path.rs
@@ -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,
@@ -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 {
diff --git a/crates/viewer/re_space_view_spatial/src/ui.rs b/crates/viewer/re_space_view_spatial/src/ui.rs
@@ -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,
 };
@@ -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({
@@ -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
                 }
@@ -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,
diff --git a/crates/viewer/re_space_view_tensor/src/space_view_class.rs b/crates/viewer/re_space_view_tensor/src/space_view_class.rs
@@ -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,
@@ -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());
             }
         });
 
diff --git a/crates/viewer/re_ui/examples/re_ui_example/main.rs b/crates/viewer/re_ui/examples/re_ui_example/main.rs
@@ -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()
     }
diff --git a/crates/viewer/re_ui/src/command.rs b/crates/viewer/re_ui/src/command.rs
@@ -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)),
diff --git a/crates/viewer/re_ui/src/context_ext.rs b/crates/viewer/re_ui/src/context_ext.rs
@@ -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
diff --git a/crates/viewer/re_ui/src/design_tokens.rs b/crates/viewer/re_ui/src/design_tokens.rs
@@ -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);
     }
 
diff --git a/crates/viewer/re_ui/src/toasts.rs b/crates/viewer/re_ui/src/toasts.rs
@@ -5,9 +5,7 @@ use std::collections::HashMap;
 use egui::Color32;
 
 pub const INFO_COLOR: Color32 = Color32::from_rgb(0, 155, 255);
-pub const WARNING_COLOR: Color32 = Color32::from_rgb(255, 212, 0);
-pub const ERROR_COLOR: Color32 = Color32::from_rgb(255, 32, 0);
-pub const SUCCESS_COLOR: Color32 = Color32::from_rgb(0, 255, 32);
+pub const SUCCESS_COLOR: Color32 = Color32::from_rgb(0, 240, 32);
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub enum ToastKind {
@@ -139,8 +137,8 @@ fn default_toast_contents(ui: &mut egui::Ui, toast: &Toast) -> egui::Response {
 
                 if toast.options.show_icon {
                     let (icon, icon_color) = match toast.kind {
-                        ToastKind::Warning => ("⚠", WARNING_COLOR),
-                        ToastKind::Error => ("❗", ERROR_COLOR),
+                        ToastKind::Warning => ("⚠", ui.style().visuals.warn_fg_color),
+                        ToastKind::Error => ("❗", ui.style().visuals.error_fg_color),
                         ToastKind::Success => ("✔", SUCCESS_COLOR),
                         _ => ("ℹ", INFO_COLOR),
                     };
diff --git a/crates/viewer/re_ui/src/ui_ext.rs b/crates/viewer/re_ui/src/ui_ext.rs
diff --git a/crates/viewer/re_viewer/src/ui/settings_screen.rs b/crates/viewer/re_viewer/src/ui/settings_screen.rs

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`use re_log_types::ComponentPath;`
`2`		`-use re_ui::ContextExt as _;`
	`2`	`+use re_ui::UiExt;`
`3`	`3`	`use re_viewer_context::{UiLayout, ViewerContext};`
`4`	`4`
`5`	`5`	`use super::DataUi;`
`@@ -47,10 +47,7 @@ impl DataUi for ComponentPath {`
`47`	`47`	`));`
`48`	`48`	`}`
`49`	`49`	`} else {`
`50`		`- ui.label(`
`51`		`- ui.ctx()`
`52`		`- .error_text(format!("Unknown component path: {self}")),`
`53`		`- );`
	`50`	`+ ui.error_label(&format!("Unknown component path: {self}"));`
`54`	`51`	`}`
`55`	`52`	`}`
`56`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -242,6 +242,9 @@ impl DesignTokens {`
`242`	`242`
`243`	`243`	`egui_style.visuals.image_loading_spinners = false;`
`244`	`244`
	`245`	`+ egui_style.visuals.error_fg_color = egui::Color32::from_rgb(0xAB, 0x01, 0x16);`
	`246`	`+ egui_style.visuals.warn_fg_color = egui::Color32::from_rgb(0xFF, 0x7A, 0x0C);`
	`247`	`+`
`245`	`248`	`ctx.set_style(egui_style);`
`246`	`249`	`}`
`247`	`250`