From 06760e1b0869459ef78840c2a380f594e7a9d1e3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 16 Jun 2025 08:30:46 +0200 Subject: [PATCH] Change API of `Tooltip` slightly (#7151) We try to be consistent with our parameter order to reduce surprise for users. I also renamed a few things to clarify what is what --- CONTRIBUTING.md | 3 +- crates/egui/src/containers/old_popup.rs | 6 ++-- crates/egui/src/containers/tooltip.rs | 39 ++++++++++++++----------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 613de20c..7ddedc37 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,7 +35,7 @@ You can test your code locally by running `./scripts/check.sh`. There are snapshots test that might need to be updated. Run the tests with `UPDATE_SNAPSHOTS=true cargo test --workspace --all-features` to update all of them. If CI keeps complaining about snapshots (which could happen if you don't use macOS, snapshots in CI are currently -rendered with macOS), you can instead run `./scripts/update_snapshots_from_ci.sh` to update your local snapshots from +rendered with macOS), you can instead run `./scripts/update_snapshots_from_ci.sh` to update your local snapshots from the last CI run of your PR (which will download the `test_results` artefact). For more info about the tests see [egui_kittest](./crates/egui_kittest/README.md). Snapshots and other big files are stored with git lfs. See [Working with git lfs](#working-with-git-lfs) for more info. @@ -125,6 +125,7 @@ While using an immediate mode gui is simple, implementing one is a lot more tric * Flip `if !condition {} else {}` * Sets of things should be lexicographically sorted (e.g. crate dependencies in `Cargo.toml`) * Put each type in their own file, unless they are trivial (e.g. a `struct` with no `impl`) +* Put most generic arguments first (e.g. `Context`), and most specific last * Break the above rules when it makes sense diff --git a/crates/egui/src/containers/old_popup.rs b/crates/egui/src/containers/old_popup.rs index 3e5de650..cc75494e 100644 --- a/crates/egui/src/containers/old_popup.rs +++ b/crates/egui/src/containers/old_popup.rs @@ -61,7 +61,7 @@ pub fn show_tooltip_at_pointer( widget_id: Id, add_contents: impl FnOnce(&mut Ui) -> R, ) -> Option { - Tooltip::new(widget_id, ctx.clone(), PopupAnchor::Pointer, parent_layer) + Tooltip::new(ctx.clone(), parent_layer, widget_id, PopupAnchor::Pointer) .gap(12.0) .show(add_contents) .map(|response| response.inner) @@ -78,7 +78,7 @@ pub fn show_tooltip_for( widget_rect: &Rect, add_contents: impl FnOnce(&mut Ui) -> R, ) -> Option { - Tooltip::new(widget_id, ctx.clone(), *widget_rect, parent_layer) + Tooltip::new(ctx.clone(), parent_layer, widget_id, *widget_rect) .show(add_contents) .map(|response| response.inner) } @@ -94,7 +94,7 @@ pub fn show_tooltip_at( suggested_position: Pos2, add_contents: impl FnOnce(&mut Ui) -> R, ) -> Option { - Tooltip::new(widget_id, ctx.clone(), suggested_position, parent_layer) + Tooltip::new(ctx.clone(), parent_layer, widget_id, suggested_position) .show(add_contents) .map(|response| response.inner) } diff --git a/crates/egui/src/containers/tooltip.rs b/crates/egui/src/containers/tooltip.rs index c85739d7..a6cb3199 100644 --- a/crates/egui/src/containers/tooltip.rs +++ b/crates/egui/src/containers/tooltip.rs @@ -7,26 +7,31 @@ use emath::Vec2; pub struct Tooltip<'a> { pub popup: Popup<'a>, - layer_id: LayerId, - widget_id: Id, + + /// The layer of the parent widget. + parent_layer: LayerId, + + /// The id of the widget that owns this tooltip. + parent_widget: Id, } impl Tooltip<'_> { - /// Show a tooltip that is always open + /// Show a tooltip that is always open. pub fn new( - widget_id: Id, ctx: Context, + parent_layer: LayerId, + parent_widget: Id, anchor: impl Into, - layer_id: LayerId, ) -> Self { + let width = ctx.style().spacing.tooltip_width; Self { - // TODO(lucasmerlin): Set width somehow (we're missing context here) - popup: Popup::new(widget_id, ctx, anchor.into(), layer_id) + popup: Popup::new(parent_widget, ctx, anchor.into(), parent_layer) .kind(PopupKind::Tooltip) .gap(4.0) + .width(width) .sense(Sense::hover()), - layer_id, - widget_id, + parent_layer, + parent_widget, } } @@ -39,8 +44,8 @@ impl Tooltip<'_> { .sense(Sense::hover()); Self { popup, - layer_id: response.layer_id, - widget_id: response.id, + parent_layer: response.layer_id, + parent_widget: response.id, } } @@ -96,8 +101,8 @@ impl Tooltip<'_> { pub fn show(self, content: impl FnOnce(&mut crate::Ui) -> R) -> Option> { let Self { mut popup, - layer_id: parent_layer, - widget_id, + parent_layer, + parent_widget, } = self; if !popup.is_open() { @@ -111,11 +116,11 @@ impl Tooltip<'_> { fs.layers .entry(parent_layer) .or_default() - .widget_with_tooltip = Some(widget_id); + .widget_with_tooltip = Some(parent_widget); fs.tooltips .widget_tooltips - .get(&widget_id) + .get(&parent_widget) .copied() .unwrap_or(PerWidgetTooltipState { bounding_rect: rect, @@ -123,7 +128,7 @@ impl Tooltip<'_> { }) }); - let tooltip_area_id = Self::tooltip_id(widget_id, state.tooltip_count); + let tooltip_area_id = Self::tooltip_id(parent_widget, state.tooltip_count); popup = popup.anchor(state.bounding_rect).id(tooltip_area_id); let response = popup.show(|ui| { @@ -144,7 +149,7 @@ impl Tooltip<'_> { response .response .ctx - .pass_state_mut(|fs| fs.tooltips.widget_tooltips.insert(widget_id, state)); + .pass_state_mut(|fs| fs.tooltips.widget_tooltips.insert(parent_widget, state)); Self::remember_that_tooltip_was_shown(&response.response.ctx); }