diff --git a/CHANGELOG.md b/CHANGELOG.md index 917ffe49..92e6af53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,8 +65,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#425](https://github.com/ClementTsang/bottom/pull/425): Fixed a bug allowing grouped mode in tree mode if already in grouped mode. -- [#458](https://github.com/ClementTsang/bottom/pull/458): Fix basic mode having really broken click bounding boxes. - ## [0.5.7] - 2021-01-30 ## Bug Fixes diff --git a/src/app.rs b/src/app.rs index 27385ed5..365d923a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2809,7 +2809,7 @@ impl App { // Pretty dead simple - iterate through the widget map and go to the widget where the click // is within. - // TODO: [REFACTOR] might want to refactor this, it's ugly as sin. + // TODO: [REFACTOR] might want to refactor this, it's really ugly. // TODO: [REFACTOR] Might wanna refactor ALL state things in general, currently everything // is grouped up as an app state. We should separate stuff like event state and gui state and etc. @@ -2826,7 +2826,8 @@ impl App { Some((right_brc_x, right_brc_y)), ) = (bt.left_tlc, bt.left_brc, bt.right_tlc, bt.right_brc) { - if (x >= left_tlc_x && y >= left_tlc_y) && (x <= left_brc_x && y <= left_brc_y) { + if (x >= left_tlc_x && y >= left_tlc_y) && (x < left_brc_x && y < left_brc_y) { + // Case for the left "button" in the simple arrow. if let Some(new_widget) = self.widget_map.get(&(bt.currently_displayed_widget_id)) { @@ -2846,8 +2847,9 @@ impl App { return; } } else if (x >= right_tlc_x && y >= right_tlc_y) - && (x <= right_brc_x && y <= right_brc_y) + && (x < right_brc_x && y < right_brc_y) { + // Case for the right "button" in the simple arrow. if let Some(new_widget) = self.widget_map.get(&(bt.currently_displayed_widget_id)) { @@ -2905,7 +2907,7 @@ impl App { if let (Some((tlc_x, tlc_y)), Some((brc_x, brc_y))) = (widget.top_left_corner, widget.bottom_right_corner) { - if (x >= tlc_x && y >= tlc_y) && (x <= brc_x && y <= brc_y) { + if (x >= tlc_x && y >= tlc_y) && (x < brc_x && y < brc_y) { if let Some(new_widget) = self.widget_map.get(&new_widget_id) { self.current_widget = new_widget.clone(); @@ -2939,179 +2941,187 @@ impl App { } // Now handle click propagation down to widget. - if let Some((_tlc_x, tlc_y)) = &self.current_widget.top_left_corner { - match &self.current_widget.widget_type { - BottomWidgetType::Proc - | BottomWidgetType::ProcSort - | BottomWidgetType::CpuLegend - | BottomWidgetType::Temp - | BottomWidgetType::Disk => { - // Get our index... - let clicked_entry = y - *tlc_y; - // + 1 so we start at 0. - let border_offset = if self.is_drawing_border() { 1 } else { 0 }; - let header_gap_offset = 1 + if self.is_drawing_gap(&self.current_widget) { - self.app_config_fields.table_gap - } else { - 0 - }; - let offset = border_offset + header_gap_offset; - if clicked_entry >= offset { - let offset_clicked_entry = clicked_entry - offset; - match &self.current_widget.widget_type { - BottomWidgetType::Proc => { - if let Some(proc_widget_state) = self - .proc_state - .get_widget_state(self.current_widget.widget_id) - { - if let Some(visual_index) = - proc_widget_state.scroll_state.table_state.selected() - { - // If in tree mode, also check to see if this click is on - // the same entry as the already selected one - if it is, - // then we minimize. + if let (Some((_tlc_x, tlc_y)), Some((_brc_x, brc_y))) = ( + &self.current_widget.top_left_corner, + &self.current_widget.bottom_right_corner, + ) { + let border_offset = if self.is_drawing_border() { 1 } else { 0 }; - let previous_scroll_position = - proc_widget_state.scroll_state.current_scroll_position; - let is_tree_mode = proc_widget_state.is_tree_mode; - - let new_position = self.increment_process_position( - offset_clicked_entry as i64 - visual_index as i64, - ); - - if is_tree_mode { - if let Some(new_position) = new_position { - if previous_scroll_position == new_position { - self.toggle_collapsing_process_branch(); - } - } - } - } - } - } - BottomWidgetType::ProcSort => { - if let Some(proc_widget_state) = self - .proc_state - .get_widget_state(self.current_widget.widget_id - 2) - { - if let Some(visual_index) = - proc_widget_state.columns.column_state.selected() - { - self.increment_process_sort_position( - offset_clicked_entry as i64 - visual_index as i64, - ); - } - } - } - BottomWidgetType::CpuLegend => { - if let Some(cpu_widget_state) = self - .cpu_state - .get_widget_state(self.current_widget.widget_id - 1) - { - if let Some(visual_index) = - cpu_widget_state.scroll_state.table_state.selected() - { - self.increment_cpu_legend_position( - offset_clicked_entry as i64 - visual_index as i64, - ); - } - } - } - BottomWidgetType::Temp => { - if let Some(temp_widget_state) = self - .temp_state - .get_widget_state(self.current_widget.widget_id) - { - if let Some(visual_index) = - temp_widget_state.scroll_state.table_state.selected() - { - self.increment_temp_position( - offset_clicked_entry as i64 - visual_index as i64, - ); - } - } - } - BottomWidgetType::Disk => { - if let Some(disk_widget_state) = self - .disk_state - .get_widget_state(self.current_widget.widget_id) - { - if let Some(visual_index) = - disk_widget_state.scroll_state.table_state.selected() - { - self.increment_disk_position( - offset_clicked_entry as i64 - visual_index as i64, - ); - } - } - } - _ => {} - } - } else { - // We might have clicked on a header! Check if we only exceeded the table + border offset, and - // it's implied we exceeded the gap offset. - if clicked_entry == border_offset { - #[allow(clippy::single_match)] + // This check ensures the click isn't actually just clicking on the bottom border. + if y < (brc_y - border_offset) { + match &self.current_widget.widget_type { + BottomWidgetType::Proc + | BottomWidgetType::ProcSort + | BottomWidgetType::CpuLegend + | BottomWidgetType::Temp + | BottomWidgetType::Disk => { + // Get our index... + let clicked_entry = y - *tlc_y; + // + 1 so we start at 0. + let header_gap_offset = 1 + if self.is_drawing_gap(&self.current_widget) { + self.app_config_fields.table_gap + } else { + 0 + }; + let offset = border_offset + header_gap_offset; + if clicked_entry >= offset { + let offset_clicked_entry = clicked_entry - offset; match &self.current_widget.widget_type { BottomWidgetType::Proc => { if let Some(proc_widget_state) = self .proc_state - .get_mut_widget_state(self.current_widget.widget_id) + .get_widget_state(self.current_widget.widget_id) { - // Let's now check if it's a column header. - if let (Some(y_loc), Some(x_locs)) = ( - &proc_widget_state.columns.column_header_y_loc, - &proc_widget_state.columns.column_header_x_locs, - ) { - // debug!("x, y: {}, {}", x, y); - // debug!("y_loc: {}", y_loc); - // debug!("x_locs: {:?}", x_locs); + if let Some(visual_index) = + proc_widget_state.scroll_state.table_state.selected() + { + // If in tree mode, also check to see if this click is on + // the same entry as the already selected one - if it is, + // then we minimize. - if y == *y_loc { - for (itx, (x_left, x_right)) in - x_locs.iter().enumerate() - { - if x >= *x_left && x <= *x_right { - // Found our column! - proc_widget_state - .columns - .set_to_sorted_index_from_visual_index( - itx, - ); - proc_widget_state - .update_sorting_with_columns(); - self.proc_state.force_update = - Some(self.current_widget.widget_id); - break; + let previous_scroll_position = proc_widget_state + .scroll_state + .current_scroll_position; + let is_tree_mode = proc_widget_state.is_tree_mode; + + let new_position = self.increment_process_position( + offset_clicked_entry as i64 - visual_index as i64, + ); + + if is_tree_mode { + if let Some(new_position) = new_position { + if previous_scroll_position == new_position { + self.toggle_collapsing_process_branch(); } } } } } } + BottomWidgetType::ProcSort => { + if let Some(proc_widget_state) = self + .proc_state + .get_widget_state(self.current_widget.widget_id - 2) + { + if let Some(visual_index) = + proc_widget_state.columns.column_state.selected() + { + self.increment_process_sort_position( + offset_clicked_entry as i64 - visual_index as i64, + ); + } + } + } + BottomWidgetType::CpuLegend => { + if let Some(cpu_widget_state) = self + .cpu_state + .get_widget_state(self.current_widget.widget_id - 1) + { + if let Some(visual_index) = + cpu_widget_state.scroll_state.table_state.selected() + { + self.increment_cpu_legend_position( + offset_clicked_entry as i64 - visual_index as i64, + ); + } + } + } + BottomWidgetType::Temp => { + if let Some(temp_widget_state) = self + .temp_state + .get_widget_state(self.current_widget.widget_id) + { + if let Some(visual_index) = + temp_widget_state.scroll_state.table_state.selected() + { + self.increment_temp_position( + offset_clicked_entry as i64 - visual_index as i64, + ); + } + } + } + BottomWidgetType::Disk => { + if let Some(disk_widget_state) = self + .disk_state + .get_widget_state(self.current_widget.widget_id) + { + if let Some(visual_index) = + disk_widget_state.scroll_state.table_state.selected() + { + self.increment_disk_position( + offset_clicked_entry as i64 - visual_index as i64, + ); + } + } + } _ => {} } - } - } - } - BottomWidgetType::Battery => { - if let Some(battery_widget_state) = self - .battery_state - .get_mut_widget_state(self.current_widget.widget_id) - { - if let Some(tab_spacing) = &battery_widget_state.tab_click_locs { - for (itx, ((tlc_x, tlc_y), (brc_x, brc_y))) in - tab_spacing.iter().enumerate() - { - if (x >= *tlc_x && y >= *tlc_y) && (x <= *brc_x && y <= *brc_y) { - battery_widget_state.currently_selected_battery_index = itx; - break; + } else { + // We might have clicked on a header! Check if we only exceeded the table + border offset, and + // it's implied we exceeded the gap offset. + if clicked_entry == border_offset { + #[allow(clippy::single_match)] + match &self.current_widget.widget_type { + BottomWidgetType::Proc => { + if let Some(proc_widget_state) = self + .proc_state + .get_mut_widget_state(self.current_widget.widget_id) + { + // Let's now check if it's a column header. + if let (Some(y_loc), Some(x_locs)) = ( + &proc_widget_state.columns.column_header_y_loc, + &proc_widget_state.columns.column_header_x_locs, + ) { + // debug!("x, y: {}, {}", x, y); + // debug!("y_loc: {}", y_loc); + // debug!("x_locs: {:?}", x_locs); + + if y == *y_loc { + for (itx, (x_left, x_right)) in + x_locs.iter().enumerate() + { + if x >= *x_left && x <= *x_right { + // Found our column! + proc_widget_state + .columns + .set_to_sorted_index_from_visual_index( + itx, + ); + proc_widget_state + .update_sorting_with_columns(); + self.proc_state.force_update = + Some(self.current_widget.widget_id); + break; + } + } + } + } + } + } + _ => {} } } } } + BottomWidgetType::Battery => { + if let Some(battery_widget_state) = self + .battery_state + .get_mut_widget_state(self.current_widget.widget_id) + { + if let Some(tab_spacing) = &battery_widget_state.tab_click_locs { + for (itx, ((tlc_x, tlc_y), (brc_x, brc_y))) in + tab_spacing.iter().enumerate() + { + if (x >= *tlc_x && y >= *tlc_y) && (x < *brc_x && y < *brc_y) { + battery_widget_state.currently_selected_battery_index = itx; + break; + } + } + } + } + } + _ => {} } - _ => {} } } } diff --git a/src/app/states.rs b/src/app/states.rs index c8af1737..c596f038 100644 --- a/src/app/states.rs +++ b/src/app/states.rs @@ -61,7 +61,7 @@ impl Default for KillSignal { pub struct AppDeleteDialogState { pub is_showing_dd: bool, pub selected_signal: KillSignal, - // tl x, tl y, br x, br y + /// tl x, tl y, br x, br y, index/signal pub button_positions: Vec<(u16, u16, u16, u16, usize)>, pub keyboard_signal_select: usize, pub last_number_press: Option, diff --git a/src/canvas/dialogs/dd_dialog.rs b/src/canvas/dialogs/dd_dialog.rs index 0415aa2d..ac9c6910 100644 --- a/src/canvas/dialogs/dd_dialog.rs +++ b/src/canvas/dialogs/dd_dialog.rs @@ -108,20 +108,31 @@ impl KillDialog for Painter { ); if app_state.should_get_widget_bounds() { + // This is kinda weird, but the gist is: + // - We have three sections; we put our mouse bounding box for the "yes" button at the very right edge + // of the left section and 3 characters back. We then give it a buffer size of 1 on the x-coordinate. + // - Same for the "no" button, except it is the right section and we do it from the start of the right + // section. + // + // Lastly, note that mouse detection for the dd buttons assume correct widths. As such, we correct + // them here and check with >= and <= mouse bound checks, as opposed to how we do it elsewhere with + // >= and <. See https://github.com/ClementTsang/bottom/pull/459 for details. app_state.delete_dialog_state.button_positions = vec![ + // Yes ( - button_layout[2].x, - button_layout[2].y, - button_layout[2].x + button_layout[2].width, - button_layout[2].y + button_layout[2].height, - 0, - ), - ( - button_layout[0].x, + button_layout[0].x + button_layout[0].width - 4, button_layout[0].y, button_layout[0].x + button_layout[0].width, - button_layout[0].y + button_layout[0].height, - 1, + button_layout[0].y, + if cfg!(target_os = "windows") { 1 } else { 15 }, + ), + // No + ( + button_layout[2].x - 1, + button_layout[2].y, + button_layout[2].x + 2, + button_layout[2].y, + 0, ), ]; } diff --git a/src/canvas/widgets/basic_table_arrows.rs b/src/canvas/widgets/basic_table_arrows.rs index e7f769a8..6ab51124 100644 --- a/src/canvas/widgets/basic_table_arrows.rs +++ b/src/canvas/widgets/basic_table_arrows.rs @@ -136,18 +136,28 @@ impl BasicTableArrows for Painter { ); if app_state.should_get_widget_bounds() { + // Some explanations for future readers: + // - The "height" as of writing of this entire widget is 2. If it's 1, it occasionally doesn't draw. + // - As such, the buttons are only on the lower part of this 2-high widget. + // - So, we want to only check at one location, the `draw_loc.y + 1`, and that's it. + // - But why is it "+2" then? Well, it's because I have a REALLY ugly hack + // for mouse button checking, since most button checks are of the form `(draw_loc.y + draw_loc.height)`, + // and the same for the x and width. Unfortunately, if you check using >= and <=, the outer bound is + // actually too large - so, we assume all of them are one too big and check via < (see + // https://github.com/ClementTsang/bottom/pull/459 for details). + // - So in other words, to make it simple, we keep this to a standard and overshoot by one here. if let Some(basic_table) = &mut app_state.basic_table_widget_state { basic_table.left_tlc = - Some((margined_draw_loc[0].x - 1, margined_draw_loc[0].y + 1)); + Some((margined_draw_loc[0].x, margined_draw_loc[0].y + 1)); basic_table.left_brc = Some(( - margined_draw_loc[0].x + margined_draw_loc[0].width - 1, - margined_draw_loc[0].y + 1, + margined_draw_loc[0].x + margined_draw_loc[0].width, + margined_draw_loc[0].y + 2, )); basic_table.right_tlc = - Some((margined_draw_loc[2].x - 1, margined_draw_loc[2].y + 1)); + Some((margined_draw_loc[2].x, margined_draw_loc[2].y + 1)); basic_table.right_brc = Some(( - margined_draw_loc[2].x + margined_draw_loc[2].width - 1, - margined_draw_loc[2].y + 1, + margined_draw_loc[2].x + margined_draw_loc[2].width, + margined_draw_loc[2].y + 2, )); } } diff --git a/src/canvas/widgets/cpu_basic.rs b/src/canvas/widgets/cpu_basic.rs index e88bbf03..5beb3f73 100644 --- a/src/canvas/widgets/cpu_basic.rs +++ b/src/canvas/widgets/cpu_basic.rs @@ -198,10 +198,8 @@ impl CpuBasicWidget for Painter { // Update draw loc in widget map if let Some(widget) = app_state.widget_map.get_mut(&widget_id) { widget.top_left_corner = Some((draw_loc.x, draw_loc.y)); - widget.bottom_right_corner = Some(( - draw_loc.x + draw_loc.width - 1, - draw_loc.y + draw_loc.height - 1, - )); + widget.bottom_right_corner = + Some((draw_loc.x + draw_loc.width, draw_loc.y + draw_loc.height)); } } } diff --git a/src/canvas/widgets/mem_basic.rs b/src/canvas/widgets/mem_basic.rs index b252c5d3..3bb40ad7 100644 --- a/src/canvas/widgets/mem_basic.rs +++ b/src/canvas/widgets/mem_basic.rs @@ -122,10 +122,8 @@ impl MemBasicWidget for Painter { if app_state.should_get_widget_bounds() { if let Some(widget) = app_state.widget_map.get_mut(&widget_id) { widget.top_left_corner = Some((draw_loc.x, draw_loc.y)); - widget.bottom_right_corner = Some(( - draw_loc.x + draw_loc.width - 1, - draw_loc.y + draw_loc.height - 1, - )); + widget.bottom_right_corner = + Some((draw_loc.x + draw_loc.width, draw_loc.y + draw_loc.height)); } } } diff --git a/src/canvas/widgets/network_basic.rs b/src/canvas/widgets/network_basic.rs index 08aa1b6b..7f0e393e 100644 --- a/src/canvas/widgets/network_basic.rs +++ b/src/canvas/widgets/network_basic.rs @@ -70,10 +70,8 @@ impl NetworkBasicWidget for Painter { if app_state.should_get_widget_bounds() { if let Some(widget) = app_state.widget_map.get_mut(&widget_id) { widget.top_left_corner = Some((draw_loc.x, draw_loc.y)); - widget.bottom_right_corner = Some(( - draw_loc.x + draw_loc.width - 1, - draw_loc.y + draw_loc.height - 1, - )); + widget.bottom_right_corner = + Some((draw_loc.x + draw_loc.width, draw_loc.y + draw_loc.height)); } } }