Skip to content

Commit

Permalink
Response.context_menu now returns the response of the context menu,…
Browse files Browse the repository at this point in the history
… if open (emilk#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<InnerResponse<()>>` 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](emilk#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 ?
  • Loading branch information
AufarZakiev authored and hacknus committed Oct 30, 2024
1 parent c999237 commit fe21c7f
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions crates/egui/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<InnerResponse<()>> {
menu::context_menu(self, add_contents)
}
}

Expand Down

0 comments on commit fe21c7f

Please sign in to comment.