-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
egui: Add collapsed
and default_collapsed
to Window for programatically collapsing it
#5661
base: master
Are you sure you want to change the base?
Conversation
Preview available at https://egui-pr-preview.github.io/pr/5661-ErikWDevwindow-collapse |
collapse
and default_collapse
to Window for programatically collapsing itcollapse
and default_collapse
to Window for programatically collapsing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments about why mark default_open
deprecated and more
@@ -66,6 +70,14 @@ impl CollapsingState { | |||
self.state.open = open; | |||
} | |||
|
|||
pub fn is_collapsed(&self) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Easier to read/write with when the Window
contained usages of both the word 'open' and 'collapse', so I added this which is just the opposite of is_open
@@ -278,9 +292,17 @@ impl<'open> Window<'open> { | |||
} | |||
|
|||
/// Set initial collapsed state of the window | |||
#[deprecated = "Use `default_collapsed` to set default collapsed state instead. This function might change meaning in future, but for now does the same thing as it used to"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old function was named default_open
which didn't really align with the usage of open
to mean whether the window was shown or not. So, I changed to explicitly use collapsed
for indicating the collapsed state of the window.
I kept the old function and its behavior though
@@ -1213,6 +1240,7 @@ impl TitleBar { | |||
open: Option<&mut bool>, | |||
collapsing: &mut CollapsingState, | |||
collapsible: bool, | |||
mut collapsed: Option<&mut bool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has to be mut collapsed
due to if let Some() =
causes the mutable reference to move
and I needed to use it twice..
@@ -33,20 +33,21 @@ use super::{area, resize, Area, Frame, Resize, ScrollArea}; | |||
/// Note that this is NOT a native OS window. | |||
/// To create a new native OS window, use [`crate::Context::show_viewport_deferred`]. | |||
#[must_use = "You should call .show()"] | |||
pub struct Window<'open> { | |||
pub struct Window<'open, 'collapsed> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could maybe be same lifetimes for simplicitly, though since they can technically be different they should perhaps be modelled like this.
I think any usecase I would have they would have the same lifetimes though
collapse
and default_collapse
to Window for programatically collapsing itcollapsed
and default_collapsed
to Window for programatically collapsing it
@@ -1233,7 +1261,19 @@ impl TitleBar { | |||
let button_rect = button_rect.round_to_pixels(ui.pixels_per_point()); | |||
|
|||
ui.allocate_new_ui(UiBuilder::new().max_rect(button_rect), |ui| { | |||
collapsing.show_default_button_with_size(ui, button_size); | |||
if let Some(collapsed) = collapsed.as_ref() { | |||
if **collapsed != collapsing.is_collapsed() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic in human language: "in case the user-provided state exists and has a different value than the internal 'is_collapsed', change the internal"
In summary, this PR:
collapsed(&self, collapsed: &mut bool)
to window which can be used to explicitly collapse a window. It will, similar to theopen
function, set thecollapsed
boolean according to the internal state when it is changed by the default collapse button / double click on title and explicitly set it to the provided value if changeddefault_collapsed
which does what olddefault_open
did - sets default collapsed statedefault_open
as deprecated since it was miss-labeled imo, should have been calleddefault_collapse
. Theopen
word in theWindow
is related to the open/close button on the window and got mixed up with the collapsed state. I would also be fine if theopen
function was renamed, though this seemed like the smallest change.This PR does not redesign the window API to handle
events
like #4835, but rather implements it using the same API as the currentopen
API./scripts/check.sh
(edit: though I apparently misinterpreted the output. It should show errors in red xD didn't read) andcargo test
incrates/egui
as well as used and verified the new API works how we want for our application