-
Notifications
You must be signed in to change notification settings - Fork 1
V0.1.0 #2
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
base: main
Are you sure you want to change the base?
V0.1.0 #2
Changes from all commits
5922b48
38f955d
809a327
8541a89
41d381e
37fc363
ceb96f2
9e88ea1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ pub struct Preset { | |||||||||||||||
| pub img: Option<PathBuf>, | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| #[derive(Debug, Clone)] | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Added Debug and Clone traits Adding Debug and Clone traits is useful for logging and copying. Ensure that these traits are necessary for the Node enum to avoid unnecessary trait implementations. |
||||||||||||||||
| pub enum Node { | ||||||||||||||||
| PresetId(usize), | ||||||||||||||||
| InnerNode(BTreeMap<String, Node>), | ||||||||||||||||
|
|
@@ -50,7 +51,7 @@ impl Presets { | |||||||||||||||
| path: path.clone(), | ||||||||||||||||
| img: if img.exists() { Some(img) } else { None }, | ||||||||||||||||
| }; | ||||||||||||||||
| node.insert(name, Node::PresetId(preset_id)); | ||||||||||||||||
| node.insert(name.to_lowercase(), Node::PresetId(preset_id)); | ||||||||||||||||
| self.lists.push(preset); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -59,7 +60,26 @@ impl Presets { | |||||||||||||||
| println!("Error: Check `presets path` in config ! ({})", e); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| node | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| pub fn filter_presets_tree(query: &str, node: &BTreeMap<String, Node>) -> BTreeMap<String, Node> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (performance): New filter_presets_tree function The new filter_presets_tree function is a good addition for filtering. Ensure that this function is optimized for large trees to avoid performance bottlenecks.
Suggested change
|
||||||||||||||||
| let mut filtered_node = BTreeMap::new(); | ||||||||||||||||
| for (name, node) in node { | ||||||||||||||||
| match node { | ||||||||||||||||
| Node::PresetId(_) => { | ||||||||||||||||
| if name.contains(query.to_lowercase().as_str()) { | ||||||||||||||||
| filtered_node.insert(name.clone(), node.clone()); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Node::InnerNode(inner_node) => { | ||||||||||||||||
| let filtered_inner = filter_presets_tree(query, inner_node); | ||||||||||||||||
| if !filtered_inner.is_empty() { | ||||||||||||||||
| filtered_node.insert(name.clone(), Node::InnerNode(filtered_inner)); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| filtered_node | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,11 +1,18 @@ | ||||||||||||||||
| use crate::ReDropApp; | ||||||||||||||||
| use common::ipc_message::Message; | ||||||||||||||||
| use common::preset::filter_presets_tree; | ||||||||||||||||
|
|
||||||||||||||||
| impl ReDropApp { | ||||||||||||||||
| pub fn show_main_ui(&mut self, ctx: &egui::Context) { | ||||||||||||||||
| egui::TopBottomPanel::top("top_panel").show(ctx, |ui| { | ||||||||||||||||
| ui.horizontal(|ui| { | ||||||||||||||||
| ui.text_edit_singleline(&mut self.preset_search_query); | ||||||||||||||||
| if ui | ||||||||||||||||
| .add(egui::TextEdit::singleline(&mut self.preset_search_query)) | ||||||||||||||||
| .changed() | ||||||||||||||||
| { | ||||||||||||||||
| self.filtered_presets_tree = | ||||||||||||||||
| filter_presets_tree(&self.preset_search_query, &self.presets.tree); | ||||||||||||||||
|
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (performance): Filtering presets on search query change Filtering presets on search query change is a good approach. Ensure that the UI updates efficiently to reflect the filtered results without noticeable lag.
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
| if ui.button("Config").clicked() { | ||||||||||||||||
| self.show_config = true; | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -68,11 +75,9 @@ impl ReDropApp { | |||||||||||||||
| // self.show_presets_into_flat_tree(ui, &self.presets.tree); | ||||||||||||||||
| let option_grid = true; // TODO: Option in config [ReDrop] | ||||||||||||||||
| if option_grid { | ||||||||||||||||
| self.show_presets_into_tree_grid(ui, &self.presets.tree); | ||||||||||||||||
| // TODO: Move presets_tree in the fn | ||||||||||||||||
| self.show_presets_into_tree_grid(ui, &self.filtered_presets_tree); | ||||||||||||||||
| } else { | ||||||||||||||||
| self.show_presets_into_flat_tree(ui, &self.presets.tree); | ||||||||||||||||
| // TODO: Move presets_tree in the fn | ||||||||||||||||
| self.show_presets_into_flat_tree(ui, &self.filtered_presets_tree); | ||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,25 +8,20 @@ impl ReDropApp { | |||||||
| ui: &mut egui::Ui, | ||||||||
| node: &BTreeMap<String, preset::Node>, | ||||||||
| ) { | ||||||||
| const MAX_PRESETS_PER_ROW: usize = 8; // TODO: Autosize with ui.available_width() / preset_width | ||||||||
|
|
||||||||
| egui::Grid::new("preset_grid") | ||||||||
| .num_columns(MAX_PRESETS_PER_ROW) | ||||||||
| .num_columns(1) | ||||||||
ceebeel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| .spacing([4., 4.]) | ||||||||
| .show(ui, |ui| { | ||||||||
| let mut preset_count = 0; | ||||||||
| let max_preset_per_row = (ui.available_width() / 68.) as usize; // Preset width = 64. + 4. (spacing) | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Magic number for preset width Consider defining a constant or a configuration parameter for the preset width (68) to improve readability and maintainability.
Suggested change
|
||||||||
| for (name, node) in node { | ||||||||
| match node { | ||||||||
| preset::Node::PresetId(preset_id) => { | ||||||||
| if self.presets.lists[*preset_id] | ||||||||
| .name | ||||||||
| .contains(&self.preset_search_query) | ||||||||
|
Comment on lines
-20
to
-22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (performance): Removed search query filtering The removal of the search query filtering might lead to performance issues if the preset list is large. Ensure that the filtering is handled elsewhere effectively. |
||||||||
| { | ||||||||
| self.show_preset(ui, preset_id); | ||||||||
| preset_count += 1; | ||||||||
| if preset_count >= MAX_PRESETS_PER_ROW { | ||||||||
| ui.end_row(); | ||||||||
| preset_count = 0; | ||||||||
| } | ||||||||
| self.show_preset(ui, preset_id); | ||||||||
| preset_count += 1; | ||||||||
| if preset_count >= max_preset_per_row { | ||||||||
| ui.end_row(); | ||||||||
| preset_count = 0; | ||||||||
| } | ||||||||
| } | ||||||||
| preset::Node::InnerNode(inner_node) => { | ||||||||
|
|
@@ -41,7 +36,6 @@ impl ReDropApp { | |||||||
| } | ||||||||
|
|
||||||||
| fn show_preset(&self, ui: &mut egui::Ui, preset_id: &usize) { | ||||||||
| // TODO: Add image button into a Grid (Responsive ?) | ||||||||
| let preset = &self.presets.lists[*preset_id]; | ||||||||
| if let Some(img_path) = &preset.img { | ||||||||
| let file_path = "file://".to_owned() + img_path.to_str().unwrap(); | ||||||||
|
|
@@ -82,12 +76,7 @@ impl ReDropApp { | |||||||
| for (name, node) in node { | ||||||||
| match node { | ||||||||
| preset::Node::PresetId(preset_id) => { | ||||||||
| if self.presets.lists[*preset_id] | ||||||||
| .name | ||||||||
| .contains(&self.preset_search_query) | ||||||||
| { | ||||||||
| self.show_preset_flat(ui, preset_id); | ||||||||
| } | ||||||||
| self.show_preset_flat(ui, preset_id); | ||||||||
| } | ||||||||
| preset::Node::InnerNode(inner_node) => { | ||||||||
| egui::CollapsingHeader::new(name).show(ui, |ui| { | ||||||||
|
|
@@ -101,10 +90,7 @@ impl ReDropApp { | |||||||
| fn show_preset_flat(&self, ui: &mut egui::Ui, preset_id: &usize) { | ||||||||
| let preset = &self.presets.lists[*preset_id]; | ||||||||
| if ui | ||||||||
| .add_sized( | ||||||||
| [ui.available_width(), 0.], | ||||||||
| egui::Button::new(&preset.name).wrap(true), // TODO: Text to left | ||||||||
| ) // TODO Fix: Button size "overflow" if name is too long / This can be a problem with grid.. | ||||||||
| .add(egui::Button::new(&preset.name).wrap(true).frame(false)) | ||||||||
| .clicked() | ||||||||
| { | ||||||||
| self.send_load_preset_file(preset.id, self.smooth) | ||||||||
|
|
||||||||
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.
question (documentation): Clarify 'Set Window default size (persistence !?)'
The exclamation and question marks might be confusing. Consider rephrasing to 'Set Window default size (persistent?)' or clarify the intended meaning.