Interactive Ui:s: add UiBuilder::sense and Ui::response - #5054
Conversation
18ad272 to
6a3cd72
Compare
|
@emilk I wonder if it would make sense to return a Response based on Ui::interact_bg from Ui::scope_dyn? Then the Ui responses would contain the right interactions based on UiBuilder::sense. /// Create a child, add content to it, and then allocate only what was used in the parent `Ui`.
pub fn scope_dyn<'c, R>(
&mut self,
ui_builder: UiBuilder,
add_contents: Box<dyn FnOnce(&mut Ui) -> R + 'c>,
) -> InnerResponse<R> {
let next_auto_id_source = self.next_auto_id_source;
let mut child_ui = self.new_child(ui_builder);
self.next_auto_id_source = next_auto_id_source; // HACK: we want `scope` to only increment this once, so that `ui.scope` is equivalent to `ui.allocate_space`.
let ret = add_contents(&mut child_ui);
self.allocate_rect(child_ui.min_rect(), Sense::hover());
let response = child_ui.interact_bg();
InnerResponse::new(ret, response)
} |
emilk
left a comment
There was a problem hiding this comment.
This is cool stuff!
I'd love to have an official example or test of this though.
And yes, I think having scope_dyn actually return a correct response based on UiBuilder::sense is better than adding a new interact_scope function
Ui::interact_bg focus and add Ui::interact_scopeUi::interact_bg focus and add Ui::interact_scope
The interact_scope function would still be required to read the response early and pass it to the ui closure.
Then the new Ui::interact_scope wouldn't be needed anymore and we could read the response early in all kinds of Uis. |
4613e51 to
556de04
Compare
aa943e6 to
3556b72
Compare
|
@emilk I have updated the PR to implement the ideas outlined above, I think I like this even more. I was able to implement this without having to store the response in the Ui struct, by inversing the logic in Ui::read_response, making this even cleaner. Also, I added a small example (I fixed the Title after the Video was recorded): Bildschirmaufnahme.2024-09-02.um.12.58.35.mov |
66e23db to
22466f9
Compare
emilk
left a comment
There was a problem hiding this comment.
This is utterly awesome ⭐
This is something I've been wanting to do in egui for a long time
| let response = self.allocate_rect(child_ui.min_rect(), Sense::hover()); | ||
| #[allow(deprecated)] | ||
| let response = child_ui.interact_bg(); | ||
| child_ui.should_interact_bg_on_drop = false; |
There was a problem hiding this comment.
Yeah, I think it's not very elegant but I couldn't come up with a better way. I'll add a comment clarifying that it's an optimization.
Ui::interact_bg focus and add Ui::interact_scopeUi:s: add UiBuilder::sense and Ui::response
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
6165cec to
aca4a0a
Compare
| let response = self.allocate_rect(child_ui.min_rect(), Sense::hover()); | ||
| #[allow(deprecated)] | ||
| let response = child_ui.interact_bg(); | ||
| child_ui.should_interact_bg_on_drop = false; |
There was a problem hiding this comment.
Yeah, I think it's not very elegant but I couldn't come up with a better way. I'll add a comment clarifying that it's an optimization.
| #[allow(deprecated)] | ||
| let response = child_ui.interact_bg(); | ||
| child_ui.should_interact_bg_on_drop = false; | ||
| self.allocate_rect(child_ui.min_rect(), Sense::hover()); |
There was a problem hiding this comment.
I also wonder if we should replace this with Ui::advance_cursor_after_rect since we don't need this response?
|
The widget id used for the We should instead use an To see the problem with the current code, just apply this diff: --- a/crates/egui_demo_lib/src/demo/interactive_container.rs
+++ b/crates/egui_demo_lib/src/demo/interactive_container.rs
@@ -21,6 +21,7 @@ impl crate::Demo for InteractiveContainerDemo {
.show(ctx, |ui| {
use crate::View as _;
self.ui(ui);
+ self.ui(ui);
});
}
}I've opened a new issue for this: |
…5054) <!-- Please read the "Making a PR" section of [`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md) before opening a Pull Request! * Keep your PR:s small and focused. * The PR title is what ends up in the changelog, so make it descriptive! * If applicable, add a screenshot or gif. * If it is a non-trivial addition, consider adding a demo for it to `egui_demo_lib`, or a new example. * Do NOT open PR:s from your `master` branch, as that makes it hard for maintainers to test and add commits to your PR. * Remember to run `cargo fmt` and `cargo clippy`. * Open the PR as a draft until you have self-reviewed it and run `./scripts/check.sh`. * When you have addressed a PR comment, mark it as resolved. Please be patient! I will review your PR, but my time is limited! --> * Closes emilk#5053 * [x] I have followed the instructions in the PR template This fixes emilk#5053 by adding a Sense parameter to UiBuilder, using that in Context::create_widget, so the Widget is registered with the right Sense / focusable. Additionally, I've added a ignore_focus param to create_widget, so the focus isn't surrendered / reregistered on Ui::interact_bg. The example from emilk#5053 now works correctly: https://github.com/user-attachments/assets/a8a04b5e-7635-4e05-9ed8-e17d64910a35 <details><summary>Updated example code</summary> <p> ```rust ui.button("I can focus"); ui.scope_builder( UiBuilder::new() .sense(Sense::click()) .id_source("focus_test"), |ui| { ui.label("I can focus for a single frame"); let response = ui.interact_bg(); let t = if response.has_focus() { "has focus" } else { "doesn't have focus" }; ui.label(t); }, ); ui.button("I can't focus :("); ``` </p> </details> --- Also, I've added `Ui::interact_scope` to make it easier to read a Ui's response in advance, without having to know about the internals of how the Ui Ids get created. This makes it really easy to created interactive container elements or custom buttons, without having to use Galleys or Painter::add(Shape::Noop) to style based on the interaction. <details><summary> Example usage to create a simple button </summary> <p> ```rust use eframe::egui; use eframe::egui::{Frame, InnerResponse, Label, RichText, UiBuilder, Widget}; use eframe::NativeOptions; use egui::{CentralPanel, Sense, WidgetInfo}; pub fn main() -> eframe::Result { eframe::run_simple_native("focus test", NativeOptions::default(), |ctx, _frame| { CentralPanel::default().show(ctx, |ui| { ui.button("Regular egui Button"); custom_button(ui, |ui| { ui.label("Custom Button"); }); if custom_button(ui, |ui| { ui.label("You can even have buttons inside buttons:"); if ui.button("button inside button").clicked() { println!("Button inside button clicked!"); } }) .response .clicked() { println!("Custom button clicked!"); } }); }) } fn custom_button<R>( ui: &mut egui::Ui, content: impl FnOnce(&mut egui::Ui) -> R, ) -> InnerResponse<R> { let auto_id = ui.next_auto_id(); ui.skip_ahead_auto_ids(1); let response = ui.interact_scope( Sense::click(), UiBuilder::new().id_source(auto_id), |ui, response| { ui.style_mut().interaction.selectable_labels = false; let visuals = response .map(|r| ui.style().interact(&r)) .unwrap_or(&ui.visuals().noninteractive()); let text_color = visuals.text_color(); Frame::none() .fill(visuals.bg_fill) .stroke(visuals.bg_stroke) .rounding(visuals.rounding) .inner_margin(ui.spacing().button_padding) .show(ui, |ui| { ui.visuals_mut().override_text_color = Some(text_color); content(ui) }) .inner }, ); response .response .widget_info(|| WidgetInfo::new(egui::WidgetType::Button)); response } ``` </p> </details> https://github.com/user-attachments/assets/281bd65f-f616-4621-9764-18fd0d07698b --------- Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
|
I am experiencing this when using egui-file-dialog and WGPU together, upon opening a file dialog. I do not get this error with EFRAME directly (no WGPU) |
Context::interact_bgbreaks focus #5053This fixes #5053 by adding a Sense parameter to UiBuilder, using that in Context::create_widget, so the Widget is registered with the right Sense / focusable. Additionally, I've added a ignore_focus param to create_widget, so the focus isn't surrendered / reregistered on Ui::interact_bg.
The example from #5053 now works correctly:
Bildschirmaufnahme.2024-09-01.um.20.45.25.mov
Updated example code
Also, I've added
Ui::interact_scopeto make it easier to read a Ui's response in advance, without having to know about the internals of how the Ui Ids get created.This makes it really easy to created interactive container elements or custom buttons, without having to use Galleys or Painter::add(Shape::Noop) to style based on the interaction.
Example usage to create a simple button
Bildschirmaufnahme.2024-09-01.um.21.05.29.mov