emilk / egui

egui: an easy-to-use immediate mode GUI in Rust that runs on both web and native
https://www.egui.rs/
Apache License 2.0
20.61k stars 1.49k forks source link

Add Into bound into id methods #4680

Open creativcoder opened 1 week ago

creativcoder commented 1 week ago

I've felt the need for this several times, and assigning ids adds a little bit of friction by having the user call into() when passing a String or &str.

This PR adds the Into trait bound on the id method that reduces this small friction from the end user.

emilk commented 5 days ago

I haven't written this down before (will do after writing this here first), but there is a reason for the current design:

An Id is supposed to be globally unique. This is ensured by having Ui construct a tree of locally unique "ID sources" (anything implementing Hash, but often string literals in practice).

So when constructing a child-region of some kind (Grid, CollapsingHeader, …) you specify a locally unique ID source. When constructing a top-level thing (e.g. an Area or Window) you specify a globally unique Id.

I want to make this somewhat clear in the API (though obviously I haven't succeeded) so the rough idea is this: The locally unique ID sources are more common, so we optimize the ergonomics for those. So for those we take anything impl Hash. For the globally unique Ids however, we force the user to explicitly type Id::new(…) or Id::from(…) or ui.id().with(…) so they think through the promise that they make to egui.

Does that make sense?


For the plot items the only use of the Id is to match against PlotResponse::hovered_plot_item, which is an Option<Id>, so you want an Id there somehow anyway, I believe.

creativcoder commented 5 days ago

Ah I see

It's a little less clear that we want to create a unique global id when we have to create them from Id::new() or Id::from(), because I still ended up creating an Id passing a string lying around in my scope in one of my codebase.

Maybe a change in method name like global_id for containers and top level ui elements would make the intention more clear. Does that work?

Also a few places, some of the methods that assign an id have different names like: collapsing_header, they can be renamed to just id() like others.