Make Memory::keep_popup_open less of a footgun#7297
Make Memory::keep_popup_open less of a footgun#7297lucasmerlin wants to merge 2 commits intomainfrom
Memory::keep_popup_open less of a footgun#7297Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/7297-lucaspopup-footgun |
| } | ||
|
|
||
| /// Check if the popup is open and keep it open. | ||
| pub fn show_popup(&mut self, popup_id: Id) -> bool { |
There was a problem hiding this comment.
The docstring for open_popup should link to this function, and vice-versa, to explain their difference..
Also: wouldn't it make sense if this called self.open_popup?
There was a problem hiding this comment.
No, if this called open_popup, you couldn't use it to check if we should show the popup.
The docs should also mention it needs to be called every frame.
There was a problem hiding this comment.
I think this is just another shape of footgun.
Based only on the names of the functions, I would expect this to always hold:
mem.show_popup(popup_id);
assert!(mem.is_showing_popup(popup_id));Which would then lead to the question "what is open_popup for?"
| } | ||
| None => { | ||
| mem.keep_popup_open(id); | ||
| mem.show_popup(id); |
There was a problem hiding this comment.
Shouldn't we always call this unless we return None below? 🤔
|
I'm doing another take on this |
2 participants
Memory::keep_popup_openless of a footgun #7037The breaking change is intentional, to force people to think about this, since otherwise the keep_popup_open thing would silently break popups.