n00kii / egui-modal

a simple modal library for egui
MIT License
57 stars 9 forks source link

new() should only take id as parameter. #5

Closed apoorv569 closed 1 year ago

apoorv569 commented 1 year ago

I am making a desktop using egui and really like your modal library, but I am having a problem. The new() for Modal takes ctx and id as parameters. This is causing me some problems, as I am defining different parts of UI in different files.

Say I have a module (a file) app.rs in which I define eframe::App which has the update() and all. And I have different files for different sections of the app like a settings dialog for which I want to use Modal.

Now in app I have,

pub struct App {
    dark_mode: bool,
    is_side_panel_open: bool,
    is_window_fullscreen: bool,
    waveform_viewer: WaveformViewer,
    preferences_dialog: PreferencesDialog,
}

where WaveformViewer is a egui::Window and I have a new() method for all sections like this and are initialized this like,

impl App {
    pub fn new() -> Self {
        Self {
            dark_mode: true,
            is_side_panel_open: false,
            is_window_fullscreen: false,
            waveform_viewer: WaveformViewwer::new(),
            preferences_dialog: PreferencesDialog::new(),
        }
    }

    // other methods..
}

now in order to call new() from the PreferencesDialog I need to have something like this,

pub struct PreferencesDialog {
    dialog: Modal,
    is_open: bool,
}

which I can initialize like this,

impl PreferencesDialog {
    pub fn new(ctx: &egui::Context) -> Self {
        Self {
            dialog: Modal::new(ctx, "preferences_dialog"),
            is_open: false,
        }
    }

now here comes the problem.. if I don't take ctx as an argument in new() I can't have dialog as a data member of PreferencesDialog and if I do I can't call it in my App::new().

But say I don't do any of this, then I have to initialize PreferencesDialog::new(ctx, id) directly in update and I won't be able to query other data and/or methods from PreferencesDialog outside update().

I think perhaps you should have a set_ctx() instead of directly taking ctx as an argument in the new().

n00kii commented 1 year ago

i may look into other ways of setting the ctx but you shouldnt need to save the modal to a struct. is there a reason why you're doing this and including an is_open field in that struct? my intention with the api is that you make the modal every frame, the same way you would make a Window or Grid or CentralPanel every frame

n00kii commented 1 year ago

having a set_ctx introduces a couple other issues, like needing to change some function signatures from T to Option<T> for when the ctx is not set yet which is sorta annoying to use. reading your issue again, it seems you aren't able to pass a &Context to your App::new()? if you are using eframe, you can get a Context from the CreationContext like so: image

apoorv569 commented 1 year ago

i may look into other ways of setting the ctx but you shouldnt need to save the modal to a struct.

I was saving the modal to the struct so I can call modal->open() in the update() in the app.rs file.

is there a reason why you're doing this and including an is_open field in that struct?

The is_open was left from before when I creating my own dialog like window.

my intention with the api is that you make the modal every frame, the same way you would make a Window or Grid or CentralPanel every frame

Yes, that is what I am sort of doing as well, but I only want to show the modal when I click a button.

After reading you reply though, I think I was confused a little with the API, but things are making sense now. I still have issues though. Here is a dialog I am trying to create, the about dialog for the app (the settings dialog was a little more complex to show here as an example)

use eframe::{egui, emath};
use egui_modal::{Modal, ModalStyle};

const PROJECT_NAME: &str = "SampleHive-rs";
const PROJECT_VERSION: &str = "v0.1";
const PROJECT_COPYRIGHT_YEARS: &str = "2023";

pub struct AboutDialog {
    is_open: bool,
}

impl AboutDialog {
    pub fn new() -> Self {
        Self { is_open: false }
    }

    pub fn render(&mut self, ctx: &egui::Context, ui: &mut egui::Ui) {
        let modal = Modal::new(ctx, "about_dialog")
            .with_style(&ModalStyle {
                body_alignment: egui::Align::Center,
                ..Default::default()
            })
            .with_close_on_outside_click(true);

        modal.title(ui, "About");

        modal.show(|ui| {
            // ui.with_layout(
            //     egui::Layout::top_down_justified(emath::Align::Center),
            //     |ui| {
            modal.body(
                ui,
                format!(
                    "{} {}\n
Copyright (C) {}  Apoorv\n
\n
{} is free software: you can redistribute it and/or modify\n
it under the terms of the GNU General Public License as published by\n
the Free Software Foundation, either version 3 of the License, or\n
(at your option) any later version.\n
\n
{} is distributed in the hope that it will be useful,\n
but WITHOUT ANY WARRANTY; without even the implied warranty of\n
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n
GNU General Public License for more details.\n
\n
You should have received a copy of the GNU General Public License\n
along with this program.  If not, see",
                    PROJECT_NAME,
                    PROJECT_VERSION,
                    PROJECT_COPYRIGHT_YEARS,
                    PROJECT_NAME,
                    PROJECT_NAME,
                ),
            );

            ui.spacing_mut().item_spacing.x = 0.0;
            ui.horizontal(|ui| {
                ui.label("<");
                ui.hyperlink("https://www.gnu.org/licenses/");
                ui.label(">.\n");
            });
            //     },
            // );

            // ui.with_layout(egui::Layout::bottom_up(egui::Align::Center), |ui| {
            //     // ui.with_layout(
            //     //     egui::Layout::centered_and_justified(egui::Direction::BottomUp),
            //     //     |ui| {
            //     ui.centered_and_justified(|ui| {
            //         ui.horizontal(|ui| {
            //             ui.radio(true, "");
            //             ui.radio(false, "");
            //         });
            //     })
            // })
            // });
        });

        if self.is_open {
            modal.open();
        }

        if modal.was_outside_clicked() {
            log::debug!("Outside clicked");

            modal.close();
            self.is_open = false;
        }
    }

    pub fn show(&mut self) {
        self.is_open = !self.is_open;
    }

    pub fn is_open(&self) -> bool {
        self.is_open
    }
}

and in the app.rs I am calling this in the update()

        egui::CentralPanel::default().show(ctx, |ui| {
            // Render other widgets..

            // Render control bar
            self.render_control_bar(ui, ctx);

            // Render about dialog
            if self.about_dialog.is_open() {
                self.about_dialog.render(ctx, ui);
            }
        });

the render_control_bar has this button defined,

                if ui.button("❓").clicked() {
                    self.about_dialog.show();
                };

The was_outside_clicked() is not doing anything, I tried moving it up and down but no luck.

How do I control the opening and closing of the dialog? I thought I could use is_open to control the opening and closing of the dialog but it doesn't help.

Also is it possible adjust the size as in width and height of the dialog?

n00kii commented 1 year ago

okay, so i patched up your approach; some things to note:

here's a fixed version, all in one file:

use eframe::{egui, NativeOptions};
use egui::{Context, Ui};
use egui_modal::{Modal, ModalStyle};

const PROJECT_NAME: &str = "SampleHive-rs";
const PROJECT_VERSION: &str = "v0.1";
const PROJECT_COPYRIGHT_YEARS: &str = "2023";

fn main() {
    eframe::run_native(
        "app_name",
        NativeOptions::default(),
        Box::new(|cc| Box::new(App::default())),
    )
}

struct App {
    about_dialog: AboutDialog,
}

impl Default for App {
    fn default() -> Self {
        Self {
            about_dialog: AboutDialog::new(),
        }
    }
}

impl App {
    fn render_control_bar(&mut self, ui: &mut Ui, ctx: &Context) {
        if ui.button("❓").clicked() {
            self.about_dialog.open()
        };
    }
}

impl eframe::App for App {
    fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            // Render about dialog
            self.about_dialog.render(ctx);

            // Render other widgets..

            // Render control bar
            self.render_control_bar(ui, ctx);
        });
    }
}

pub struct AboutDialog {
    modal: Option<Modal>,
}

impl AboutDialog {
    pub fn new() -> Self {
        Self { modal: None }
    }

    pub fn open(&self) {
        if let Some(modal) = self.modal.as_ref() {
            modal.open()
        }
    }

    pub fn render(&mut self, ctx: &egui::Context) {
        let modal = Modal::new(ctx, "about_dialog")
            .with_style(&ModalStyle {
                body_alignment: egui::Align::Center,
                ..Default::default()
            })
            .with_close_on_outside_click(true);

        modal.show(|ui| {
            modal.title(ui, "About");
            modal.body(
                ui,
                format!(
                    "{} {}\n
Copyright (C) {}  Apoorv\n
\n
{} is free software: you can redistribute it and/or modify\n
it under the terms of the GNU General Public License as published by\n
the Free Software Foundation, either version 3 of the License, or\n
(at your option) any later version.\n
\n
{} is distributed in the hope that it will be useful,\n
but WITHOUT ANY WARRANTY; without even the implied warranty of\n
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n
GNU General Public License for more details.\n
\n
You should have received a copy of the GNU General Public License\n
along with this program.  If not, see",
                    PROJECT_NAME,
                    PROJECT_VERSION,
                    PROJECT_COPYRIGHT_YEARS,
                    PROJECT_NAME,
                    PROJECT_NAME,
                ),
            );

            ui.spacing_mut().item_spacing.x = 0.0;
            ui.horizontal(|ui| {
                ui.label("<");
                ui.hyperlink("https://www.gnu.org/licenses/");
                ui.label(">.\n");
            });
        });

        if modal.was_outside_clicked() {
            println!("outside clicked");
        }

        self.modal = Some(modal)
    }
}

however, i would still personally discourage saving the modal to a struct because it add some unnecessary complication. rather, i think it would be better if you just made and returned the modal in a single function so you can do with it as you please, as in this approach:

use eframe::{egui, NativeOptions};
use egui::{Context, Ui};
use egui_modal::{Modal, ModalStyle};

const PROJECT_NAME: &str = "SampleHive-rs";
const PROJECT_VERSION: &str = "v0.1";
const PROJECT_COPYRIGHT_YEARS: &str = "2023";

fn main() {
    eframe::run_native(
        "app_name",
        NativeOptions::default(),
        Box::new(|cc| Box::new(App)),
    )
}

struct App;

impl App {
    fn render_control_bar(&mut self, ui: &mut Ui, ctx: &Context) {
        let about_dialog = self.render_about_dialog(ctx);
        if ui.button("❓").clicked() {
            about_dialog.open()
        };
    }
    pub fn render_about_dialog(&mut self, ctx: &egui::Context) -> Modal {
        let modal = Modal::new(ctx, "about_dialog")
            .with_style(&ModalStyle {
                body_alignment: egui::Align::Center,
                ..Default::default()
            })
            .with_close_on_outside_click(true);

        modal.show(|ui| {
            modal.title(ui, "About");
            modal.body(
                ui,
                format!(
                    "{} {}\n
Copyright (C) {}  Apoorv\n
\n
{} is free software: you can redistribute it and/or modify\n
it under the terms of the GNU General Public License as published by\n
the Free Software Foundation, either version 3 of the License, or\n
(at your option) any later version.\n
\n
{} is distributed in the hope that it will be useful,\n
but WITHOUT ANY WARRANTY; without even the implied warranty of\n
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n
GNU General Public License for more details.\n
\n
You should have received a copy of the GNU General Public License\n
along with this program.  If not, see",
                    PROJECT_NAME,
                    PROJECT_VERSION,
                    PROJECT_COPYRIGHT_YEARS,
                    PROJECT_NAME,
                    PROJECT_NAME,
                ),
            );

            ui.spacing_mut().item_spacing.x = 0.0;
            ui.horizontal(|ui| {
                ui.label("<");
                ui.hyperlink("https://www.gnu.org/licenses/");
                ui.label(">.\n");
            });
        });

        if modal.was_outside_clicked() {
            println!("outside clicked");
        }

        modal
    }
}

impl eframe::App for App {
    fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            // Render other widgets..

            // Render control bar
            self.render_control_bar(ui, ctx);
        });
    }
}
apoorv569 commented 1 year ago

okay, so i patched up your approach; some things to note:

* you dont need to keep track of a bool for whether or not the modal is open (it has its own internal one)

* you're meant to _always_ call the show function (rather than only calling it on the condition that the modal is open)

* your `modal.title` was outside of the `show` so it wasn't showing up

* as for changing size and width, nothing exists for that right now (save for adjusting the margin in `ModalStyle`) but i can work on that

* `was_outside_clicked` is indeed bugged, i'll push a fix

  * but you dont need to close the modal with `was_outside_clicked` if you already set `.with_close_on_outside_click`

...

OK so its working as intended but I still have couple of problems,

use eframe::{egui, emath};
use egui_modal::{Modal, ModalStyle};

const PROJECT_NAME: &str = "SampleHive-rs";
const PROJECT_VERSION: &str = "v0.1";
const PROJECT_COPYRIGHT_YEARS: &str = "2023";

enum AboutDialogPage {
    AboutProject,
    LicenseInfo,
}

pub struct AboutDialog {
    is_open: bool,
    is_page_changed: bool,
    page: AboutDialogPage,
}

impl AboutDialog {
    pub fn new() -> Self {
        Self {
            is_open: false,
            is_page_changed: false,
            page: AboutDialogPage::AboutProject,
        }
    }

    pub fn render(&mut self, ctx: &egui::Context, ui: &mut egui::Ui) {
        let modal = Modal::new(ctx, "about_dialog")
            .with_style(&ModalStyle {
                body_alignment: egui::Align::Center,
                ..Default::default()
            })
            .with_close_on_outside_click(true);

        modal.show(|ui| {
            modal.title(ui, "About");

            match self.page {
                AboutDialogPage::AboutProject => {
                    ui.label("SampleHive-rs");
                }
                AboutDialogPage::LicenseInfo => {
                    modal.body(
                        ui,
                        format!(
                            "{} {}\n
        Copyright (C) {}  Apoorv\n
        \n
        {} is free software: you can redistribute it and/or modify\n
        it under the terms of the GNU General Public License as published by\n
        the Free Software Foundation, either version 3 of the License, or\n
        (at your option) any later version.\n
        \n
        {} is distributed in the hope that it will be useful,\n
        but WITHOUT ANY WARRANTY; without even the implied warranty of\n
        MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\n
        GNU General Public License for more details.\n
        \n
        You should have received a copy of the GNU General Public License\n
        along with this program.  If not, see",
                            PROJECT_NAME,
                            PROJECT_VERSION,
                            PROJECT_COPYRIGHT_YEARS,
                            PROJECT_NAME,
                            PROJECT_NAME,
                        ),
                    );

                    ui.spacing_mut().item_spacing.x = 0.0;
                    ui.horizontal(|ui| {
                        ui.label("<");
                        ui.hyperlink("https://www.gnu.org/licenses/");
                        ui.label(">.\n");
                    });
                }
            }

            ui.with_layout(egui::Layout::bottom_up(egui::Align::Center), |ui| {
                ui.horizontal(|ui| {
                    // Add some space before to position the radio buttons in middle
                    ui.add_space(
                        (ui.available_width() - egui::Style::default().spacing.item_spacing.x)
                            / 2.1,
                    );

                    if ui.radio(!self.is_page_changed, "").clicked() {
                        self.is_page_changed = false;
                        self.page = AboutDialogPage::AboutProject;
                    };
                    if ui.radio(self.is_page_changed, "").clicked() {
                        self.is_page_changed = true;
                        self.page = AboutDialogPage::LicenseInfo;
                    };
                });

                ui.separator();
            });
        });

        modal.open();

        // if modal.was_outside_clicked() {
        //     log::debug!("Outside clicked");

        //     modal.close();
        //     self.is_open = false;
        // }
    }

    pub fn show(&mut self) {
        self.is_open = !self.is_open;
    }

    pub fn is_open(&self) -> bool {
        self.is_open
    }
}

And I think I no longer need to either store the Modal in struct or return from the method, I was doing so before only so that I can call modal.open() elsewhere.

But I do need the is_open field as if I just have the render() for the AboutDialog called in the update() it just shows the dialog as soon as the app launches.

so this,

                if ui.button("❓").clicked() {
                    self.about_dialog.show();
                };

toggles the is_open bool and the update() checks if this is bool is true only then show the dialog,

            // Render about dialog
            if self.about_dialog.is_open() {
                self.about_dialog.render(ctx, ui);
            }

Here is how the Modal dialog looks, 2023-01-19_12-30 2023-01-19_12-30_1

And here is how my dialog that I created my self looks like using egui::Window 2023-01-19_12-21 2023-01-19_12-21_1

        egui::Window::new("About")
            .open(&mut self.is_open())
            .default_width(550.0)
            .default_height(500.0)
            .anchor(emath::Align2::CENTER_CENTER, egui::vec2(0.0, 0.0))
            .collapsible(false)
            .resizable(false)
            .title_bar(false)
            .show(ctx, |ui| {
                ui.horizontal(|ui| {
                    ui.with_layout(egui::Layout::left_to_right(egui::Align::Center), |ui| {
                        if ui.button("❌").clicked() {
                            self.show()
                        };
                    });

                    ui.with_layout(
                        egui::Layout::centered_and_justified(egui::Direction::LeftToRight),
                        |ui| {
                            ui.heading("About the project");
                        },
                    );
                });

                ui.separator();

                match self.page {
                    AboutDialogPage::AboutProject => {
                        ui.label("SampleHive-rs");
                    }
                    AboutDialogPage::LicenseInfo => {

rest of the code is same as above.

n00kii commented 1 year ago

yes, i will add a function so that you can set the default_width and default_height of the modal so it doesn't look squished.

as for it not working as expected, please look at my examples above again (i dont think you changed your code to match either of the examples); i believe you may still be confused with the api. you need to be calling the show function every frame, not just when the modal is_open. modal.open should not be called every frame withmodal.show.

if you follow the examples that i gave above, the outside click should work fine: egui-test_gnSjfliCkZ

apoorv569 commented 1 year ago

yes, i will add a function so that you can set the default_width and default_height of the modal so it doesn't look squished.

as for it not working as expected, please look at my examples above again (i dont think you changed your code to match either of the examples); i believe you may still be confused with the api. you need to be calling the show function every frame, not just when the modal is_open. modal.open should not be called every frame withmodal.show.

if you follow the examples that i gave above, the outside click should work fine: egui-test_gnSjfliCkZ egui-test_gnSjfliCkZ

Ah! My bad. Ok I changed the code according to your example. It works now.

Waiting for to adjustable width and height patch now.

n00kii commented 1 year ago

alright, i added a default_width to ModalStyle so you should be able to set the width of the modal now (try the changes on the latest of the main branch on git). unfortunately it seems setting the default_height or min_height of a Window doesn't change the height of the window for some reason so i won't include that in this patch

apoorv569 commented 1 year ago

alright, i added a default_width to ModalStyle so you should be able to set the width of the modal now (try the changes on the latest of the main branch on git). unfortunately it seems setting the default_height or min_height of a Window doesn't change the height of the window for some reason so i won't include that in this patch

default_width is working as expected.

Any idea why the default_height is not working?

n00kii commented 1 year ago

no clue, if you make you own egui::Window and set .default_height, nothing changes, so it's part of egui's internal behavior that i'm not aware of. but since it seems everything else is working on your end ill publish the changes and close the issue

apoorv569 commented 1 year ago

no clue, if you make you own egui::Window and set .default_height, nothing changes, so it's part of egui's internal behavior that i'm not aware of. but since it seems everything else is working on your end ill publish the changes and close the issue

I played with the height a little bit. It seems like if there is not enough content in either x or y axis the window won't stretch to the desired size, so its not just height the width doesn't work either if there is not enough content.

We can perhaps add something like a separator and use ui.add_visible_ui() setting it to false to hide the contents inside.

n00kii commented 1 year ago

perhaps, maybe this is something to discuss in a new issue; ill poke around and try to see what works though

apoorv569 commented 1 year ago

perhaps, maybe this is something to discuss in a new issue; ill poke around and try to see what works though

I asked about this on the egui repository and I was able to fix this. Here is the relevant issue, https://github.com/emilk/egui/issues/2711

n00kii commented 1 year ago

continuing this discussion in #6, there still seems to be some things to consider