Refactor UI code & resolve minor issues#359
Conversation
…correctly for the selected locale
… recipes from saved roations
KonaeAkira
left a comment
KonaeAkira
left a comment
There was a problem hiding this comment.
Thanks for doing this. The UI code has always been in need of some refactoring imo. I only have a few nitpicks, the rest LGTM.
| } | ||
| } | ||
|
|
||
| assert!(solver_events.is_empty()); |
| @@ -76,7 +63,7 @@ impl Simulator<'_> { | |||
| ui.label(egui::RichText::new(t!(locale, "Simulation")).strong()); | |||
| ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { | |||
| ui.add_visible( | |||
| !self.actions.is_empty() && self.config_changed, | |||
There was a problem hiding this comment.
Any particular reason why you removed !self.actions.is_empty()? I think it's odd to display the config changed warning when there is no macro being displayed. I'm not necessarily against this change. I just want to know your motivation.
There was a problem hiding this comment.
I mostly removed it so that the warning reappears after cancelling a solve before any intermediate solution is displayed, to basically make all cases consistent in whether they show the warning or not.
Before, starting a solve immediately updated the last solve params so it was never shown (and the behaviour was not easily changeable), after the rework the replacement of the last solve params are only updated for final solutions so I had to decide if/when the message is shown after cancelling (+ while solving).
I though the warning still fits after cancelling ("last solve" being interpreted as "last completed solve") so I chose to display it, but I can see that it can be distracting / unfitting if interpreted differently.
| .data_mut(egui::util::IdTypeMap::remove_by_type::<egui::scroll_area::State>); | ||
|
|
||
| load_fonts(&cc.egui_ctx); | ||
| // load_fonts(&cc.egui_ctx); |
Deploying raphael-rs with
|
| Latest commit: |
b25d51c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c2e0c21c.raphael-rs.pages.dev |
| Branch Preview URL: | https://ui-code-refactor.raphael-rs.pages.dev |
2 participants
The commit messages should hopefully give a reasonable overview of the changes. I tried making similar changes a few times before, but those attempts included more drastic changes and did not work out. This attempt is more limited in scope, so additional work can(/should, probably) be done following this.
Most of the refactor is just moving stuff around, including moving things that were previously stored (temporarily or persisted) using
eguibut used across different parts of the UI to structs owned by theMacroSolverApp.Functionally everything should work the same as before. But I also resolved some minor issues that required a rework of how things are currently handled: