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

Max window size & other size helpers #3537

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Conversation

arduano
Copy link
Contributor

@arduano arduano commented Nov 8, 2023

Was a bit confused why the max_size API isn't exposed on Window, when it's perfectly functional in Resize. Anyway here's the main thing that it affects:

let screen = ctx.available_rect();
let size = screen.size();

egui::Window::new(self.name())
    .resizable(true)
    .resize(|resize| resize.max_size(size)) // Before
    .max_size(size)                         // After
    .show(ctx, |ui| todo!());

I also added some other relevant helpers for consistency.

This PR doesn't change any logic, only forwards along some helper functions that are already public for consistency.

Copy link

@Lord-Hellgrim Lord-Hellgrim left a comment

Choose a reason for hiding this comment

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

This looks very reasonable and I don't see any breaking bugs.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Looks good, but please add #[inline] to the functions

@arduano
Copy link
Contributor Author

arduano commented Nov 16, 2023

@emilk I'm sorry, which functions? Just the ones I added, or all size related ones, or all of them in the window struct? None of the other functions there have #[inline] so I didn't add it either

@emilk
Copy link
Owner

emilk commented Nov 16, 2023

Oh, they don't? My bad. All trivial builder functions should have #[inline] on them, but I guess we haven't been great at enforcing that. I can take a pass on fixing that after this PR is merged.

EDIT: Fixed in #3557

@emilk emilk added the egui label Nov 16, 2023
@emilk emilk merged commit 4886c8c into emilk:master Nov 16, 2023
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants