Response.context_menu now returns the response of the context menu, if open

[AufarZakiev]

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.
• 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 ?

[YgorSouza]

Another downside is that it makes it harder to use the BitOr operator.

[AufarZakiev]

With the fix BitOr could be used on original response itself, isn't it?

I see the point in another downside: method chaining becomes impossible. We won't be able to call smth like my_response.context_menu(...).highlight().
However, I think, such chaining is OK on builder pattern, when the object does not exist yet (e.g. Label, TextEdit, etc.). It is not so necessary when dealing with an already constructed object, such as Response

[YgorSouza]

I meant if you wanted to union the responses from multiple widgets as a single expression while inserting context menus and tooltips for each of them, like this:

fn my_composite_widget(ui: &mut Ui) -> Response {
    let ctx_menu = |ui: &mut Ui, option| {
        ui.label(option);
    };
    ui.button("Button 1")
        .context_menu(|ui| ctx_menu(ui, "Menu 1"))
        | ui.button("Button 2")
            .on_hover_text("Tooltip 2")
            .context_menu(|ui| ctx_menu(ui, "Menu 2"))
        | ui.button("Button 3")
            .context_menu(|ui| ctx_menu(ui, "Menu 3"))
}

Though I suppose this could be solved by adding an inspect method like in the standard library. And it would be easy for anyone to create a trait containing a method like this and implement it for Response.

[emilk]

I think this makes sense, since the context_menu method needs to return something to the user

[AufarZakiev]

@YgorSouza, does it this code fit your needs?

fn my_composite_widget(ui: &mut Ui) -> egui::Response {
    let ctx_menu = |ui: &mut Ui, option| {
        ui.label(option);
    };
    let btn1 = ui.button("Button 1");
    let btn2 = ui.button("Button 2").on_hover_text("Tooltip 2");
    let btn3 = ui.button("Button 3");

    btn1.context_menu(|ui| ctx_menu(ui, "Menu 1"));
    btn2.context_menu(|ui| ctx_menu(ui, "Menu 2"));
    btn3.context_menu(|ui| ctx_menu(ui, "Menu 3"));

    return btn1 | btn2 | btn3;
}

I see it a little wordy but more consistent. If there is a way to make it both very laconic and avoid let response = pattern - it would be great.

Thinking out loud:
I feel that context_menu, on_hover_text, etc. should be applicable to the Response object AND to the Button itself.

• Response's methods may take a ref
• Button's methods may consume self, as it is done with shortcut_text, rounding, etc.

I will play around a bit trying to implement this idea.

[AufarZakiev]

@emilk , I have merged last changes from the master branch. Could you please set labels for this PR? I guess I have no rights for that

[emilk]

@YgorSouza you can also use https://crates.io/crates/tap