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

🎋 Fix #11: Id clash when using multiple Trees #12

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/advanced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl Default for MyApp {

let root = tiles.insert_tab_tile(tabs);

let tree = egui_tiles::Tree::new(root, tiles);
let tree = egui_tiles::Tree::new("my_app_tree", root, tiles);

Self {
tree,
Expand Down
2 changes: 1 addition & 1 deletion examples/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ fn create_tree() -> egui_tiles::Tree<Pane> {

let root = tiles.insert_tab_tile(tabs);

egui_tiles::Tree::new(root, tiles)
egui_tiles::Tree::new("simple_tree", root, tiles)
}
2 changes: 1 addition & 1 deletion src/container/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ mod tests {
let mut tiles = Tiles::default();
let panes: Vec<TileId> = vec![tiles.insert_pane(Pane {}), tiles.insert_pane(Pane {})];
let root: TileId = tiles.insert_grid_tile(panes);
Tree::new(root, tiles)
Tree::new("test-tree", root, tiles)
};

let style = egui::Style::default();
Expand Down
2 changes: 1 addition & 1 deletion src/container/linear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ fn linear_drop_zones<Pane>(
let preview_thickness = 12.0;
let dragged_index = children
.iter()
.position(|&child| is_being_dragged(egui_ctx, child));
.position(|&child| is_being_dragged(egui_ctx, &tree.id, child));

let after_rect = |rect: Rect| match dir {
LinearDir::Horizontal => Rect::from_min_max(
Expand Down
6 changes: 3 additions & 3 deletions src/container/tabs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl Tabs {
.drag_started()
{
behavior.on_edit();
ui.memory_mut(|mem| mem.set_dragged_id(tile_id.egui_id()));
ui.memory_mut(|mem| mem.set_dragged_id(tile_id.egui_id(&tree.id)));
}
}

Expand All @@ -137,10 +137,10 @@ impl Tabs {
continue;
}

let is_being_dragged = is_being_dragged(ui.ctx(), child_id);
let is_being_dragged = is_being_dragged(ui.ctx(), &tree.id, child_id);

let selected = self.is_active(child_id);
let id = child_id.egui_id();
let id = child_id.egui_id(&tree.id);

let response =
behavior.tab_ui(&tree.tiles, ui, id, child_id, selected, is_being_dragged);
Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ fn is_possible_drag(ctx: &egui::Context) -> bool {
ctx.input(|input| input.pointer.is_decidedly_dragging())
}

fn is_being_dragged(ctx: &egui::Context, tile_id: TileId) -> bool {
ctx.memory(|mem| mem.is_being_dragged(tile_id.egui_id())) && is_possible_drag(ctx)
fn is_being_dragged(ctx: &egui::Context, tree_id: &egui::Id, tile_id: TileId) -> bool {
ctx.memory(|mem| mem.is_being_dragged(tile_id.egui_id(tree_id))) && is_possible_drag(ctx)
}

/// If this tile is currently being dragged, cover it with a semi-transparent overlay ([`Behavior::dragged_overlay_color`]).
Expand All @@ -278,7 +278,7 @@ fn cover_tile_if_dragged<Pane>(
ui: &mut egui::Ui,
tile_id: TileId,
) {
if is_being_dragged(ui.ctx(), tile_id) {
if is_being_dragged(ui.ctx(), &tree.id, tile_id) {
if let Some(child_rect) = tree.tiles.try_rect(tile_id) {
let overlay_color = behavior.dragged_overlay_color(ui.visuals());
ui.painter().rect_filled(child_rect, 0.0, overlay_color);
Expand Down
4 changes: 2 additions & 2 deletions src/tile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ impl TileId {
}

/// Corresponding [`egui::Id`], used for dragging.
pub fn egui_id(&self) -> egui::Id {
egui::Id::new(("egui_tile", self))
pub fn egui_id(&self, tree_id: &egui::Id) -> egui::Id {
tree_id.with(("tile", self))
}
}

Expand Down
53 changes: 29 additions & 24 deletions src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
/// let tabs: Vec<TileId> = vec![tiles.insert_pane(Pane { }), tiles.insert_pane(Pane { })];
/// let root: TileId = tiles.insert_tab_tile(tabs);
///
/// let tree = Tree::new(root, tiles);

Check failure on line 26 in src/tree.rs

View workflow job for this annotation

GitHub Actions / Format + check + test

the trait bound `egui::id::Id: From<TileId>` is not satisfied

Check failure on line 26 in src/tree.rs

View workflow job for this annotation

GitHub Actions / Format + check + test

this function takes 3 arguments but 2 arguments were supplied
/// ```
#[derive(Clone, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
Expand All @@ -33,16 +33,9 @@

/// All the tiles in the tree.
pub tiles: Tiles<Pane>,
}

impl<Pane> Default for Tree<Pane> {
// An empty tree
fn default() -> Self {
Self {
root: None,
tiles: Default::default(),
}
}
/// The previous id used to draw the tree.
pub(crate) id: egui::Id,
}

impl<Pane: std::fmt::Debug> std::fmt::Debug for Tree<Pane> {
Expand Down Expand Up @@ -92,48 +85,58 @@
// ----------------------------------------------------------------------------

impl<Pane> Tree<Pane> {
pub fn empty() -> Self {
Self::default()
pub fn empty(id: impl Into<egui::Id>) -> Self {
Self {
root: None,
tiles: Default::default(),
id: id.into(),
}
}

/// The most flexible constructor, allowing you to set up the tiles
/// however you want.
pub fn new(root: TileId, tiles: Tiles<Pane>) -> Self {
pub fn new(id: impl Into<egui::Id>, root: TileId, tiles: Tiles<Pane>) -> Self {
Self {
root: Some(root),
tiles,
id: id.into(),
}
}

/// Get the id used by this Tree.
pub fn id(&self) -> egui::Id {
self.id
}

/// Create a top-level [`crate::Tabs`] container with the given panes.
pub fn new_tabs(panes: Vec<Pane>) -> Self {
Self::new_container(ContainerKind::Tabs, panes)
pub fn new_tabs(id: impl Into<egui::Id>, panes: Vec<Pane>) -> Self {
Self::new_container(id, ContainerKind::Tabs, panes)
}

/// Create a top-level horizontal [`crate::Linear`] container with the given panes.
pub fn new_horizontal(panes: Vec<Pane>) -> Self {
Self::new_container(ContainerKind::Horizontal, panes)
pub fn new_horizontal(id: impl Into<egui::Id>, panes: Vec<Pane>) -> Self {
Self::new_container(id, ContainerKind::Horizontal, panes)
}

/// Create a top-level vertical [`crate::Linear`] container with the given panes.
pub fn new_vertical(panes: Vec<Pane>) -> Self {
Self::new_container(ContainerKind::Vertical, panes)
pub fn new_vertical(id: impl Into<egui::Id>, panes: Vec<Pane>) -> Self {
Self::new_container(id, ContainerKind::Vertical, panes)
}

/// Create a top-level [`crate::Grid`] container with the given panes.
pub fn new_grid(panes: Vec<Pane>) -> Self {
Self::new_container(ContainerKind::Grid, panes)
pub fn new_grid(id: impl Into<egui::Id>, panes: Vec<Pane>) -> Self {
Self::new_container(id, ContainerKind::Grid, panes)
}

/// Create a top-level container with the given panes.
pub fn new_container(kind: ContainerKind, panes: Vec<Pane>) -> Self {
pub fn new_container(id: impl Into<egui::Id>, kind: ContainerKind, panes: Vec<Pane>) -> Self {
let mut tiles = Tiles::default();
let tile_ids = panes
.into_iter()
.map(|pane| tiles.insert_pane(pane))
.collect();
let root = tiles.insert_new(Tile::Container(Container::new(kind, tile_ids)));
Self::new(root, tiles)
Self::new(id, root, tiles)
}

/// Check if [`Self::root`] is [`None`].
Expand Down Expand Up @@ -169,6 +172,8 @@
///
/// The tree will use upp all the available space - nothing more, nothing less.
pub fn ui(&mut self, behavior: &mut dyn Behavior<Pane>, ui: &mut Ui) {
self.id = ui.id();

Comment on lines +175 to +176
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a mistake

Copy link
Contributor Author

@tosti007 tosti007 Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at this wrongly you're right!

self.simplify(&behavior.simplification_options());

self.gc(behavior);
Expand Down Expand Up @@ -234,7 +239,7 @@
match &mut tile {
Tile::Pane(pane) => {
if behavior.pane_ui(&mut ui, tile_id, pane) == UiResponse::DragStarted {
ui.memory_mut(|mem| mem.set_dragged_id(tile_id.egui_id()));
ui.memory_mut(|mem| mem.set_dragged_id(tile_id.egui_id(&self.id)));
}
}
Tile::Container(container) => {
Expand Down Expand Up @@ -411,7 +416,7 @@
continue; // not allowed to drag root
}

let id = tile_id.egui_id();
let id = tile_id.egui_id(&self.id);
let is_tile_being_dragged = ctx.memory(|mem| mem.is_being_dragged(id));
if is_tile_being_dragged {
// Abort drags on escape:
Expand Down
Loading