Add `Popup` and `Tooltip`, unifying the previous behaviours #5713

lucasmerlin · 2025-02-12T09:56:22Z

This introduces new Tooltip and Popup structs that unify and extend the old popups and tooltips.

Popup handles the positioning and optionally stores state on whether the popup is open (for click based popups like ComboBox, menus, context menus).
Tooltip is based on Popup and handles state of whether the tooltip should be shown (which turns out to be quite complex to handles all the edge cases).

Both Popup and Tooltip can easily be constructed from a Response and then customized via builder methods.

This also introduces PositionAlign, for aligning something outside of a Rect (in contrast to Align2 for aligning inside a Rect). But I don't like the name, any suggestions? Inspired by mui's tooltip positioning.

Part of Unify menus with tooltips/popups #4607
I have followed the instructions in the PR template

TODOs:

Automatic tooltip positioning based on available space
Review / fix / remove all code TODOs
~~Update the helper fns on Response to be consistent in naming and parameters (Some use tooltip, some hover_ui, some take &self, some take self)~~ actually, I think the naming and parameter make sense on second thought
Make sure all old code is marked deprecated

For discussion during review:

the following check in show_tooltip_for still necessary?:

     let is_touch_screen = ctx.input(|i| i.any_touches());
     let allow_placing_below = !is_touch_screen; // There is a finger below. TODO: Needed?

github-actions · 2025-02-12T10:00:27Z

Preview available at https://egui-pr-preview.github.io/pr/5713-lucasunify-tooltip-popup
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

lucasmerlin · 2025-02-12T17:37:50Z

While I think it's nice that the ComboBoxes indicate the direction they will open in, it can look somewhat confusing in edge cases:

(the second last combobox is bigger so it will wrap while the last one doesn't yet wrap)

Since this is also difficult to implement with the new Popup positioning, I'll remove this behavior and always point the arrow down.

…opup

Mingun · 2025-02-13T05:37:31Z

crates/egui/src/containers/popup.rs

+/// Similar to [`Align2`] but for aligning something to the outside of some rect.
+/// ```text
+///              ┌───────────┐  ┌────────┐  ┌─────────┐              
+///              │ TOP_START │  │  TOP   │  │ TOP_END │              
+///              └───────────┘  └────────┘  └─────────┘               
+/// ┌──────────┐ ┌────────────────────────────────────┐ ┌───────────┐
+/// │LEFT_START│ │                                    │ │RIGHT_START│
+/// └──────────┘ │                                    │ └───────────┘
+/// ┌──────────┐ │                                    │ ┌───────────┐
+/// │   LEFT   │ │             some_rect              │ │   RIGHT   │
+/// └──────────┘ │                                    │ └───────────┘
+/// ┌──────────┐ │                                    │ ┌───────────┐
+/// │ LEFT_END │ │                                    │ │ RIGHT_END │
+/// └──────────┘ └────────────────────────────────────┘ └───────────┘
+///              ┌────────────┐  ┌──────┐  ┌──────────┐              
+///              │BOTTOM_START│  │BOTTOM│  │BOTTOM_END│              
+///              └────────────┘  └──────┘  └──────────┘              
 /// ```
-/// # egui::__run_test_ui(|ui| {
-/// if ui.ui_contains_pointer() {
-///     egui::show_tooltip_text(ui.ctx(), ui.layer_id(), egui::Id::new("my_tooltip"), "Helpful text");
-/// }
-/// # });
-/// ```
-pub fn show_tooltip_text(
-    ctx: &Context,
-    parent_layer: LayerId,
-    widget_id: Id,
-    text: impl Into<WidgetText>,
-) -> Option<()> {
-    show_tooltip(ctx, parent_layer, widget_id, |ui| {
-        crate::widgets::Label::new(text).ui(ui);
-    })
-}
+// TODO: Find a better name for Position and PositionAlign
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct PositionAlign(pub Position, pub Align);


The great gojs library models similar conception as two properties of a Spot type in the widget (which they called GraphObject):

alignment

alignmentFocus

For examples of positioning see the Panels examples, especially a Panel.Spot type.

Nice, that is a nice way to think about it! I've changed my PositionAlign to be based on two Align2, which feels nicer and allows for more alignments. It's called Align4 now 😄

emilk

There's a lot of code to review here 😅 I've only looked at a small portion of it so far. I think I need a walk-through

crates/egui/src/containers/area.rs

crates/egui/src/containers/combo_box.rs

crates/emath/src/align.rs

crates/egui/src/memory/mod.rs

crates/egui/src/containers/old_popup.rs

crates/egui/src/containers/popup.rs

crates/emath/src/rect_relation.rs

crates/egui_kittest/tests/popup.rs

emilk · 2025-02-18T10:36:49Z

crates/egui/src/containers/popup.rs


-    if parent_ui.input(|i| i.key_pressed(Key::Escape)) || should_close {
-        parent_ui.memory_mut(|mem| mem.close_popup());
+        Some(response)


As you said: it would be nice to have a response.closed() flag the user can check. That could also be set by Area, CollapsingPanel, ColorPickerButton etc

Created another issue for this: #5727

emilk · 2025-02-18T10:39:04Z

crates/egui/src/containers/popup.rs

+        match open_kind {
+            OpenKind::Open | OpenKind::Closed => {}
+            OpenKind::Bool(open, close_behavior) => {
+                if should_close(close_behavior) {
+                    *open = false;
+                }
+            }
+            OpenKind::Memory { close_behavior, .. } => {
+                if should_close(close_behavior) {
+                    ctx.memory_mut(|mem| mem.close_popup());
+                }
+            }
        }


It would be nice with something like

if should_close(close_behavior) { open_kind.set_open(false); }

…thought maybe for a later PR

crates/egui/src/containers/popup.rs

crates/egui/src/containers/tooltip.rs

emilk · 2025-02-18T10:44:58Z

crates/egui/src/containers/tooltip.rs

+            fs.layers
+                .entry(parent_layer)
+                .or_default()
+                .widget_with_tooltip = Some(widget_id);


We could maybe get rid of LayerState::open_popups if we also stored which widgets has a popup open (attached to them). That would allow us to simplify and improve the "don't show a tooltip if this widget has a popup open" check in

egui/crates/egui/src/response.rs

Lines 615 to 619 in b36311a

let any_open_popups = self.ctx.prev_pass_state(|fs| {

fs.layers

.get(&self.layer_id)

.map_or(false, |layer| !layer.open_popups.is_empty())

});

(can be saved for a future PR though)

Add Popup and Tooltip, unifying the previous behaviours

254e444

lucasmerlin marked this pull request as draft February 12, 2025 09:56

Small fixes

199db96

lucasmerlin force-pushed the lucas/unify-tooltip-popup branch from 12cf1b7 to 199db96 Compare February 12, 2025 10:27

lucasmerlin added 5 commits February 12, 2025 12:32

Mark all deprecated

d977cee

Smart popup positioning based on available space

315e2ae

PositionAlign docs

915a4ab

Remove old tooltip positioning

be1b0f8

Cleanup

37be3f0

Allow specifying fallback positions and implement combobox with new p…

ba81bb5

…opup

lucasmerlin added feature New feature or request egui labels Feb 12, 2025

lucasmerlin added 3 commits February 12, 2025 19:57

Update popup example

acdd41d

Fixes and improvements

d32a0ac

Remove some uncommented code and todos

df33197

Mingun reviewed Feb 13, 2025

View reviewed changes

lucasmerlin added 7 commits February 13, 2025 11:08

Base PositionAlign on two Align2 and rename to Align4

fb64785

Remember the pointer position for context menus

efedb6a

Clippy

47a50bd

Move Align4 to emath

cb2ca4e

Small improvements

b31571e

Allow passing layout to area, popup and tooltip

76d4db6

Resolve todos

36a0c0f

lucasmerlin marked this pull request as ready for review February 13, 2025 13:31

Lints

fc9821e

lucasmerlin force-pushed the lucas/unify-tooltip-popup branch from 34b082b to fc9821e Compare February 13, 2025 13:32

lucasmerlin added 2 commits February 13, 2025 14:38

Update snapshot

5ce3ed1

Fix doc tests

bc5a750

lucasmerlin mentioned this pull request Feb 13, 2025

Improved menu based on egui::Popup #5716

Draft

1 task

emilk reviewed Feb 17, 2025

View reviewed changes

lucasmerlin added 4 commits February 18, 2025 09:56

Rename Align4 to RectRelation and pivot and focus to parent and child

575d580

Move to own file

c3f150f

Docs

a9dd2f3

Remove todo

d3a65b9

lucasmerlin commented Feb 18, 2025

View reviewed changes

crates/egui/src/containers/popup.rs Outdated Show resolved Hide resolved

lucasmerlin commented Feb 18, 2025

View reviewed changes

crates/egui/src/containers/popup.rs Outdated Show resolved Hide resolved

emilk approved these changes Feb 18, 2025

View reviewed changes

lucasmerlin mentioned this pull request Feb 18, 2025

Add Response::closed and investigate generalizing popup::OpenKind as something that could be used with Window, Collapsing, Area etc... #5727

Open

lucasmerlin added 5 commits February 18, 2025 14:23

More docs, rename rect_relation to rect_align, store ctx in Popup

740e56c

Rename gap vector to gap factor

54f4a7b

Fixes

5ae1975

Fixes

b922cf6

Fixes

f3d1ad8

lucasmerlin merged commit a8e98d3 into master Feb 18, 2025
46 checks passed

lucasmerlin deleted the lucas/unify-tooltip-popup branch February 18, 2025 14:53

lucasmerlin mentioned this pull request Feb 18, 2025

Support on hover tooltip that is noninteractable even with interactable content #5543

Draft

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Popup` and `Tooltip`, unifying the previous behaviours #5713

Add `Popup` and `Tooltip`, unifying the previous behaviours #5713

lucasmerlin commented Feb 12, 2025 •

edited

Loading

github-actions bot commented Feb 12, 2025

lucasmerlin commented Feb 12, 2025

Mingun Feb 13, 2025

lucasmerlin Feb 13, 2025

emilk left a comment

emilk Feb 18, 2025

lucasmerlin Feb 18, 2025

emilk Feb 18, 2025

emilk Feb 18, 2025

	let any_open_popups = self.ctx.prev_pass_state(\|fs\| {
	fs.layers
	.get(&self.layer_id)
	.map_or(false, \|layer\| !layer.open_popups.is_empty())
	});

Add Popup and Tooltip, unifying the previous behaviours #5713

Add Popup and Tooltip, unifying the previous behaviours #5713

Conversation

lucasmerlin commented Feb 12, 2025 • edited Loading

github-actions bot commented Feb 12, 2025

lucasmerlin commented Feb 12, 2025

Mingun Feb 13, 2025

Choose a reason for hiding this comment

lucasmerlin Feb 13, 2025

Choose a reason for hiding this comment

emilk left a comment

Choose a reason for hiding this comment

emilk Feb 18, 2025

Choose a reason for hiding this comment

lucasmerlin Feb 18, 2025

Choose a reason for hiding this comment

emilk Feb 18, 2025

Choose a reason for hiding this comment

emilk Feb 18, 2025

Choose a reason for hiding this comment

Add `Popup` and `Tooltip`, unifying the previous behaviours #5713

Add `Popup` and `Tooltip`, unifying the previous behaviours #5713

lucasmerlin commented Feb 12, 2025 •

edited

Loading