Skip to content

Commit

Permalink
Much nicer looking error and warning messages (#8127)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
emilk authored Nov 14, 2024
1 parent fe840c0 commit 95b118f
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 102 deletions.
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
8 changes: 3 additions & 5 deletions crates/viewer/re_ui/src/toasts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
};
Expand Down
Loading

0 comments on commit 95b118f

Please sign in to comment.