Collapse panels #238
Collapse panels #238
Conversation
| @@ -689,27 +708,19 @@ fn file_menu(ui: &mut egui::Ui, app: &mut App, _frame: &mut eframe::Frame) { | |||
| ui.menu_button("Advanced", |ui| { | |||
There was a problem hiding this comment.
not a great place for such a fundamental view operation. should go to a View menu. But that can ofc wait
| ui.close_menu(); | ||
| } | ||
|
|
||
| #[cfg(all(feature = "puffin", not(target_arch = "wasm32")))] | ||
| if ui | ||
| .button("Profile viewer") | ||
| .on_hover_text("Starts a profiler, showing what makes the viewer run slow") | ||
| .on_hover_text("Starts a profiler, showing what makes the viewer run slow (⌘⇧P or ⌃⇧P)") |
There was a problem hiding this comment.
do we have a way generate these strings from the set shortcut? I mean at least soemthing like format!("Starts a profiler, showing what makes the viewer run slow ({} or {})", get_puffin_shortcut().as_string(), get_puffin_alt_shortcut().as_string())
Also, more importantly neiother ⌘ nor ⌃ makes much sense to a windows user!
There was a problem hiding this comment.
(applies to all shortcut strings ofc)
There was a problem hiding this comment.
One problem is that different platforms use different standards for showing shortcuts
⇧⌘P-style is standard on MacOS- I believe
Shift+Ctrl+Pis standard on Windows? - On Linux I have no idea.
The further problem is that when running on Web it's difficult to know what platform we are running on (but I could add some detection code to egui_web that does its best).
Another problem is that on Mac it is more convenient/natural to use ⌘ Command while on Windows Ctrl is more natural.
Maybe @martenbjork has a suggestion here.
There was a problem hiding this comment.
Actually, I have a pretty good plan now. I'll use the user-agent to detect Mac, If mac, I'll show the shortcut as "⇧⌘P", and else as "Ctrl+Shift+P", but I'll save that improvement for a separate PR.
| pub blueprint_panel_expanded: bool, | ||
| pub selection_panel_expanded: bool, | ||
| pub time_panel_expanded: bool, |
There was a problem hiding this comment.
is this conceptually really part of the blueprint? It's not like we want to configure it there 🤔
There was a problem hiding this comment.
if it is, then maybe the menu button for reset should be somewhere in the blueprint panel
There was a problem hiding this comment.
is this conceptually really part of the blueprint? It's not like we want to configure it there
As the docstring says, the plan is for the Blueprint to define the layout of the whole Viewer. But we're not there yet. The plan is that we can select any element in the blueprint panel, then change its options in the selection panel.
There was a problem hiding this comment.
…but we can't/won't add everything in one PR. There is some UI design work needed first.
See also emilk/egui#2190
Checklist
CHANGELOG.md(if this is a big enough change to warrant it)