From 979b910a69b630f32020df0df01e1d539dcca087 Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Sat, 6 Jul 2024 08:25:32 -0400 Subject: [PATCH] Make menus process events before updating working details (#799) * Make columnar menu update values after process events * Remove inaccurate comment from ide_menu Previously, it said that the working details were updated before processing events, which is not the case. The comment may have been copied from columnar_menu. * Make description_menu process events before updating values --- src/menu/columnar_menu.rs | 72 +++++++++++++------------- src/menu/description_menu.rs | 98 ++++++++++++++++++------------------ src/menu/ide_menu.rs | 1 - 3 files changed, 84 insertions(+), 87 deletions(-) diff --git a/src/menu/columnar_menu.rs b/src/menu/columnar_menu.rs index 611b93e8..03ca79ab 100644 --- a/src/menu/columnar_menu.rs +++ b/src/menu/columnar_menu.rs @@ -515,8 +515,42 @@ impl Menu for ColumnarMenu { painter: &Painter, ) { if let Some(event) = self.event.take() { - // The working value for the menu are updated first before executing any of the - // menu events + match event { + MenuEvent::Activate(updated) => { + self.active = true; + self.reset_position(); + + self.input = if self.settings.only_buffer_difference { + Some(editor.get_buffer().to_string()) + } else { + None + }; + + if !updated { + self.update_values(editor, completer); + } + } + MenuEvent::Deactivate => self.active = false, + MenuEvent::Edit(updated) => { + self.reset_position(); + + if !updated { + self.update_values(editor, completer); + } + } + MenuEvent::NextElement => self.move_next(), + MenuEvent::PreviousElement => self.move_previous(), + MenuEvent::MoveUp => self.move_up(), + MenuEvent::MoveDown => self.move_down(), + MenuEvent::MoveLeft => self.move_left(), + MenuEvent::MoveRight => self.move_right(), + MenuEvent::PreviousPage | MenuEvent::NextPage => { + // The columnar menu doest have the concept of pages, yet + } + } + + // The working value for the menu are updated only after executing the menu events, + // so they have the latest suggestions // // If there is at least one suggestion that contains a description, then the layout // is changed to one column to fit the description @@ -572,40 +606,6 @@ impl Menu for ColumnarMenu { self.working_details.columns = possible_cols; } } - - match event { - MenuEvent::Activate(updated) => { - self.active = true; - self.reset_position(); - - self.input = if self.settings.only_buffer_difference { - Some(editor.get_buffer().to_string()) - } else { - None - }; - - if !updated { - self.update_values(editor, completer); - } - } - MenuEvent::Deactivate => self.active = false, - MenuEvent::Edit(updated) => { - self.reset_position(); - - if !updated { - self.update_values(editor, completer); - } - } - MenuEvent::NextElement => self.move_next(), - MenuEvent::PreviousElement => self.move_previous(), - MenuEvent::MoveUp => self.move_up(), - MenuEvent::MoveDown => self.move_down(), - MenuEvent::MoveLeft => self.move_left(), - MenuEvent::MoveRight => self.move_right(), - MenuEvent::PreviousPage | MenuEvent::NextPage => { - // The columnar menu doest have the concept of pages, yet - } - } } } diff --git a/src/menu/description_menu.rs b/src/menu/description_menu.rs index 95197b7a..d9687206 100644 --- a/src/menu/description_menu.rs +++ b/src/menu/description_menu.rs @@ -462,56 +462,6 @@ impl Menu for DescriptionMenu { painter: &Painter, ) { if let Some(event) = self.event.take() { - // Updating all working parameters from the menu before executing any of the - // possible event - let max_width = self.get_values().iter().fold(0, |acc, suggestion| { - let str_len = suggestion.value.len() + self.default_details.col_padding; - if str_len > acc { - str_len - } else { - acc - } - }); - - // If no default width is found, then the total screen width is used to estimate - // the column width based on the default number of columns - let default_width = if let Some(col_width) = self.default_details.col_width { - col_width - } else { - let col_width = painter.screen_width() / self.default_details.columns; - col_width as usize - }; - - // Adjusting the working width of the column based the max line width found - // in the menu values - if max_width > default_width { - self.working_details.col_width = max_width; - } else { - self.working_details.col_width = default_width; - }; - - // The working columns is adjusted based on possible number of columns - // that could be fitted in the screen with the calculated column width - let possible_cols = painter.screen_width() / self.working_details.col_width as u16; - if possible_cols > self.default_details.columns { - self.working_details.columns = self.default_details.columns.max(1); - } else { - self.working_details.columns = possible_cols; - } - - // Updating the working rows to display the description - if self.menu_required_lines(painter.screen_width()) <= painter.remaining_lines() { - self.working_details.description_rows = self.default_details.description_rows; - self.show_examples = true; - } else { - self.working_details.description_rows = painter - .remaining_lines() - .saturating_sub(self.default_details.selection_rows + 1) - as usize; - - self.show_examples = false; - } - match event { MenuEvent::Activate(_) => { self.reset_position(); @@ -578,6 +528,54 @@ impl Menu for DescriptionMenu { } MenuEvent::PreviousPage | MenuEvent::NextPage => {} } + + let max_width = self.get_values().iter().fold(0, |acc, suggestion| { + let str_len = suggestion.value.len() + self.default_details.col_padding; + if str_len > acc { + str_len + } else { + acc + } + }); + + // If no default width is found, then the total screen width is used to estimate + // the column width based on the default number of columns + let default_width = if let Some(col_width) = self.default_details.col_width { + col_width + } else { + let col_width = painter.screen_width() / self.default_details.columns; + col_width as usize + }; + + // Adjusting the working width of the column based the max line width found + // in the menu values + if max_width > default_width { + self.working_details.col_width = max_width; + } else { + self.working_details.col_width = default_width; + }; + + // The working columns is adjusted based on possible number of columns + // that could be fitted in the screen with the calculated column width + let possible_cols = painter.screen_width() / self.working_details.col_width as u16; + if possible_cols > self.default_details.columns { + self.working_details.columns = self.default_details.columns.max(1); + } else { + self.working_details.columns = possible_cols; + } + + // Updating the working rows to display the description + if self.menu_required_lines(painter.screen_width()) <= painter.remaining_lines() { + self.working_details.description_rows = self.default_details.description_rows; + self.show_examples = true; + } else { + self.working_details.description_rows = painter + .remaining_lines() + .saturating_sub(self.default_details.selection_rows + 1) + as usize; + + self.show_examples = false; + } } } diff --git a/src/menu/ide_menu.rs b/src/menu/ide_menu.rs index 5dc325f7..fac1feab 100644 --- a/src/menu/ide_menu.rs +++ b/src/menu/ide_menu.rs @@ -652,7 +652,6 @@ impl Menu for IdeMenu { painter: &Painter, ) { if let Some(event) = self.event.take() { - // The working value for the menu are updated first before executing any of the match event { MenuEvent::Activate(updated) => { self.active = true;