From fe21c7f4e329530d34c76aab68bd067c522c8214 Mon Sep 17 00:00:00 2001 From: Aufar Zakiev Date: Tue, 30 Jan 2024 01:07:16 +0300 Subject: [PATCH] `Response.context_menu` now returns the response of the context menu, if open (#3904) Hi everyone! It's a great pleasure to work with such a library. It feels like a breath of fresh air! **Problem:** The current implementation of `context_menu` consumes `self` and returns it. However, the underlying `menu::context_menu` requires only a reference to `self`, so it is safe to change the `context_menu` signature. **Fix:** 1. Change signature to take a ref to `self`. 2. Return the `Option>` from the underlying method call **Pros:** 1. No `let response = response.context_menu(|| {...})` pattern 2. Better consistency with other `show`-like methods, as it is in the `Window::show` 3. Less ownership gymnastics **Cons:** 1. Breaking change 2. Smth else what I don't see ? **Additional info:** - This method is also addressed in [this PR](https://github.com/emilk/egui/pull/857). - `check.sh` fails only on `cargo check --quiet -p eframe --no-default-features --features wgpu` with `"The platform you're compiling for is not supported by winit"` error, should it be launched at all ? --- crates/egui/src/response.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 027938fb18c..bc252052ae4 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -770,9 +770,8 @@ impl Response { /// ``` /// /// See also: [`Ui::menu_button`] and [`Ui::close_menu`]. - pub fn context_menu(self, add_contents: impl FnOnce(&mut Ui)) -> Self { - menu::context_menu(&self, add_contents); - self + pub fn context_menu(&self, add_contents: impl FnOnce(&mut Ui)) -> Option> { + menu::context_menu(self, add_contents) } }