lucasmerlin / hello_egui

A collection of useful crates for egui
https://lucasmerlin.github.io/hello_egui/
MIT License
272 stars 24 forks source link

Consider making dnd::ItemIterator public #32

Closed tpstevens closed 3 weeks ago

tpstevens commented 1 month ago

When writing more complex structures with dnd::show_custom(), it'd be really nice to be able to compose ui() calls with external functions. Currently, since ItemIterator is private to the egui_dnd crate, it can only be used inside the closure passed to show_custom(). For example, to render a couple of separated lists into a single dnd area, your options are:

duplicate the code for rendering:

pub fn ui(&mut self, ui: &mut egui::Ui) {
    let mut idx = 0usize;
    let response = hello_egui::dnd::dnd(ui, "dnd_two_lists").show_custom(|ui, iter| {
        // Draw first list
        for item in &self.list_1 {
            iter.next(ui, egui::Id::new(item.id), idx, true, |ui, dnd_item| {
                dnd_item.ui(ui, |ui, handle, _item_state| {
                    item.ui(ui, handle);
                })
            });

            idx += 1;
        }

        // Draw separator
        iter.next(
            ui,
            egui::Id::new("dnd_two_lists_separator"),
            idx,
            true,
            |ui, dnd_item| {
                dnd_item.ui(ui, |ui, _handle, _item_state| {
                    ui.separator();
                })
            },
        );

        idx += 1;

        // Draw second list
        for item in &self.list_2 {
            iter.next(ui, egui::Id::new(item.id), idx, true, |ui, dnd_item| {
                dnd_item.ui(ui, |ui, handle, _item_state| {
                    item.ui(ui, handle);
                })
            });

            idx += 1;
        }
    });

    if let Some(update) = response.update {
        // custom logic for handling item move
    }
}

or write a closure that takes ownership of the item iterator, but then it has to contain the logic for drawing any separator elements:

pub fn ui(&mut self, ui: &mut egui::Ui) {
    let mut idx = 0usize;

    let response = hello_egui::dnd::dnd(ui, "dnd_two_lists").show_custom(|ui, iter| {
        let mut draw_list = |list: &[Item], separator_id: Option<&str>| {
            for item in list {
                iter.next(ui, egui::Id::new(item.id), idx, true, |ui, dnd_item| {
                    dnd_item.ui(ui, |ui, handle, _item_state| {
                        item.ui(ui, handle);
                    })
                });

                idx += 1;
            }

            if let Some(separator_id) = separator_id {
                // Draw separator
                iter.next(
                    ui,
                    egui::Id::new(separator_id),
                    idx,
                    true,
                    |ui, dnd_item| {
                        dnd_item.ui(ui, |ui, _handle, _item_state| {
                            ui.separator();
                        })
                    },
                );

                idx += 1;
            }
        };

        draw_list(&self.list_1, None);
        draw_list(&self.list_2, Some("dnd_two_lists_separator"));
    });

    if let Some(update) = response.update {
        // custom logic for handling item move
    }
}

Both of these are a bit cumbersome, and making ItemIterator public would make egui_dnd much more flexible.

As a side note, I have examples for drawing/updating arbitrary numbers of lists separated by arbitrary separators, combined lists with separate ScrollAreas, and separate nested lists (where you can only drag and drop between the same list, not from one list to another). They can be found in my dnd branch here. If it'd be useful, I'd be happy to clean them up (and properly document them!) and submit them as more examples to hello_egui.

I'm working on an example for nested lists where items can be dragged arbitrarily between them, but ItemIterator being private is making recursive iteration a little more fun than it needs to be :sweat_smile:

tpstevens commented 1 month ago

Also, I can't emphasize enough that my egui-playground code isn't production ready, and I would refactor it before submitting any of it to hello_egui. I made it temporarily public to reference it as part of this issue.

lucasmerlin commented 1 month ago

Ah, because you want to pass it to a function but because ItemIterator isn't public you can't specify it's type? That makes sense. Also I think since it's not public you can't see it's documentation on docs.rs which is also a problem.

I'm really curious about how dragging items between nested lists would work, I didn't think that would be possible with egui_dnd in it's current form 🤔

tpstevens commented 1 month ago

Exactly. Though I did find a (convoluted) way to pass it into a function:

use crate::dnd::item::Item;

/// Two unified lists that are separated by a horizontal line.
pub struct TwoLists {
    list_1: Vec<Item>,
    list_2: Vec<Item>,
}

type IterNextInnerFn<'a> =
    Box<dyn Fn(&mut egui::Ui, hello_egui::dnd::Handle, hello_egui::dnd::ItemState) + 'a>;

fn draw_list<'a, T>(list: &'a [Item], dnd_idx: &mut usize, mut iter_next: T)
where
    T: FnMut(egui::Id, usize, IterNextInnerFn<'a>),
{
    for item in list {
        iter_next(
            egui::Id::new(item.id),
            *dnd_idx,
            Box::new(
                |ui: &mut egui::Ui,
                 handle: hello_egui::dnd::Handle,
                 _item_state: hello_egui::dnd::ItemState| {
                    item.ui(ui, handle);
                },
            ),
        );

        *dnd_idx += 1;
    }
}

impl TwoLists {
    pub fn new(start_id: usize, num_items_per_area: usize) -> Self {
        let mut list_1 = Vec::<Item>::new();
        let mut list_2 = Vec::<Item>::new();

        for i in 0..num_items_per_area {
            list_1.push(Item::new(start_id + i));
            list_2.push(Item::new(start_id + num_items_per_area + i));
        }

        Self { list_1, list_2 }
    }

    pub fn ui(&mut self, ui: &mut egui::Ui) {
        let mut idx = 0usize;
        let response = hello_egui::dnd::dnd(ui, "dnd_two_lists").show_custom(|ui, iter| {
            let mut iter_next = |id: egui::Id, idx: usize, f: IterNextInnerFn| {
                iter.next(ui, id, idx, true, |ui, dnd_item| dnd_item.ui(ui, f));
            };

            // Draw first list
            draw_list(&self.list_1, &mut idx, &mut iter_next);

            // Draw separator
            iter_next(
                egui::Id::new("dnd_two_lists_separator"),
                idx,
                Box::new(
                    |ui: &mut egui::Ui,
                     _handle: hello_egui::dnd::Handle,
                     _item_state: hello_egui::dnd::ItemState| {
                        ui.separator();
                    },
                ),
            );

            // Draw second list
            draw_list(&self.list_2, &mut idx, &mut iter_next);
        });

        if let Some(update) = response.update {
            crate::dnd::util::move_elements_2(
                &mut self.list_1,
                &mut self.list_2,
                update.from,
                update.to,
                1,
            );
        }
    }
}

You "just" have to implement the drawing function for each structure as a function that takes a function (which calls iter.next()) that takes a function (that actually does the drawing). Then you can define a closure that owns the ItemIterator and have that closure call each struct-provided closure to do the drawing.

With ItemIterator public, the interface gets a lot simpler. And if you make Item public as well, then it'd be easier to customize the behavior of the closure that gets passed into ItemIterator::next(). In my example above, I assume that all calls to ItemIterator::next() only care about customizing the closure passed into dnd_item.ui().

Now that I have this method worked out, I'll try doing the nested lists with arbitrary dragging.

tpstevens commented 1 month ago

To elaborate on "the interface gets a lot simpler", I think there are two main ways that ergonomics would improve:

tpstevens commented 1 month ago

I updated my multiple_lists example with this method here: https://github.com/tpstevens/egui-playground/commit/32a95de3d474d1b3dce92eb03077aac7bde6304e

tpstevens commented 1 month ago

Finished my first draft of nested lists. It gets a little tricky because you have to keep track of the dnd index that corresponds to the start and the end of each list, and it's a good idea to hide all children while dragging so you don't try to parent a list to one of its children. Finally, it's necessary to add separators to the start and end of each list so that you can differentiate which list you're appending an element to. Otherwise, you'd have to use the horizontal position of the cursor. I suspect that'll be a challenge when implementing drag and drop between separate elements, as the other two current issues mention.

Anyway, if you can accept those limitations, this method works pretty well!

https://github.com/tpstevens/egui-playground/commit/d1a518282de3265a15c7caa0790fdfe31e4e0562

(In the previous commit, I switched to my custom branch of hello_egui that makes ItemIterator public: https://github.com/tpstevens/egui-playground/commit/5592f57f725af24673b7a09ff2f94b4113d7eaf3)

lucasmerlin commented 3 weeks ago

Hi, finally managed to give this a try, the nested list works really well! It didn't even occur to me that you could use egui_dnd recursively like that. I really like your implementation!

Also, I've just release egui_dnd 0.9.1, making ItemIterator public.