Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ui::response only works if UiBuilder::id_salt is uniquely set #5190

Closed
emilk opened this issue Sep 30, 2024 · 0 comments · Fixed by #5192
Closed

Ui::response only works if UiBuilder::id_salt is uniquely set #5190

emilk opened this issue Sep 30, 2024 · 0 comments · Fixed by #5192
Labels
bug Something is broken

Comments

@emilk
Copy link
Owner

emilk commented Sep 30, 2024

The widget id used for the interact/response of Ui is currently Ui::id, which has a problem: is not necessarily unique.

We should instead use an Id that is globally unique, i.e. based on where in the hierarchy the Ui is, i.e. based on next_auto_id_salt. I believe this means we need to store this in Ui so we can use the same Id in remember_min_rect.

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);
             });
     }
 }

This also means the drag-and-drop demo uses the same Id for all drag-sources, which exsasserbates

@emilk emilk added the bug Something is broken label Sep 30, 2024
@emilk emilk changed the title Ui::response requires unique Id Ui::response only works if UiBuilder::id_salt is uniquely set Sep 30, 2024
@emilk emilk closed this as completed in 448e12d Sep 30, 2024
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
This adds a new `Ui::unique_id` used in `Ui::response`.

I'll make a follow-up PR where the old `id` is renamed `stable_id`, and
deprecate `fn id` to force users to think through which `id` they want.

* Closes emilk#5190
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant