refactor: add back widget titles

Also has a few clippy fixes and bug fixes:
- Fix redundant rerendering on scroll on time graphs.
- Fix being off by one cell during rendering for no-battery situation
  on the battery widget.
- Fix having empty columns for
  rtl column width calculations (as otherwise it goes to the wrong column).
- Fix rendering issue on small windows with text tables.

We also now ensure that the CPU legend has enough room to draw!

src/app/layout_manager.rs

@@ -1315,11 +1315,9 @@ pub fn create_layout_tree(
                                     child_ids
                                         .into_iter()
                                         .filter_map(|child_id| {
-                                            if let Some(w) = widget_lookup_map.get(&child_id) {
-                                                Some((child_id, w.get_pretty_name().into()))
-                                            } else {
-                                                None
-                                            }
+                                            widget_lookup_map
+                                                .get(&child_id)
+                                                .map(|w| (child_id, w.get_pretty_name().into()))
                                         })
                                         .collect(),
                                 )
@@ -1773,7 +1771,7 @@ pub fn generate_layout(
 
                     // Handle flexible children now.
                     let denominator: u32 = flexible_indices.iter().map(|(_, _, ratio)| ratio).sum();
-                    let original_constraints = constraints.clone();
+                    let original_constraints = constraints;
                     let mut split_constraints = flexible_indices
                         .iter()
                         .map(|(_, _, numerator)| {
@@ -1876,7 +1874,7 @@ pub fn generate_layout(
                     }
 
                     let denominator: u32 = flexible_indices.iter().map(|(_, _, ratio)| ratio).sum();
-                    let original_constraints = constraints.clone();
+                    let original_constraints = constraints;
                     let mut split_constraints = flexible_indices
                         .iter()
                         .map(|(_, _, numerator)| {

src/app/widgets.rs

@@ -3,7 +3,13 @@ use std::time::Instant;
 use crossterm::event::{KeyEvent, MouseEvent};
 use enum_dispatch::enum_dispatch;
 use indextree::NodeId;
-use tui::{backend::Backend, layout::Rect, widgets::TableState, Frame};
+use tui::{
+    backend::Backend,
+    layout::Rect,
+    text::Span,
+    widgets::{Block, Borders, TableState},
+    Frame,
+};
 
 use crate::{
     app::{
@@ -115,6 +121,30 @@ pub trait Widget {
     /// Returns a [`Widget`]'s "pretty" display name.
     fn get_pretty_name(&self) -> &'static str;
 
+    /// Returns a new [`Block`]. The default implementation returns
+    /// a basic [`Block`] with different border colours based on selected state.
+    fn block<'a>(&self, painter: &'a Painter, is_selected: bool, borders: Borders) -> Block<'a> {
+        let has_top_bottom_border =
+            borders.contains(Borders::TOP) || borders.contains(Borders::BOTTOM);
+
+        let block = Block::default()
+            .border_style(if is_selected {
+                painter.colours.highlighted_border_style
+            } else {
+                painter.colours.border_style
+            })
+            .borders(borders);
+
+        if has_top_bottom_border {
+            block.title(Span::styled(
+                format!(" {} ", self.get_pretty_name()),
+                painter.colours.widget_title_style,
+            ))
+        } else {
+            block
+        }
+    }
+
     /// Draws a [`Widget`]. The default implementation draws nothing.
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,

src/app/widgets/base/scrollable.rs

@@ -1,4 +1,4 @@
-use std::cmp::Ordering;
+use std::cmp::{min, Ordering};
 
 use crossterm::event::{KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind};
 use tui::{layout::Rect, widgets::TableState};
@@ -76,20 +76,18 @@ impl Scrollable {
             self.window_index.cached_area = self.bounds;
         }
 
-        let list_start = match self.scroll_direction {
+        match self.scroll_direction {
             ScrollDirection::Down => {
                 if self.current_index < self.window_index.index + num_visible_rows {
                     // If, using the current window index, we can see the element
                     // (so within that and + num_visible_rows) just reuse the current previously scrolled position
-                    self.window_index.index
                 } else if self.current_index >= num_visible_rows {
-                    // Else if the current position past the last element visible in the list, omit
-                    // until we can see that element.  The +1 is of how indexes start at 0.
+                    // Else if the current position is past the last element visible in the list, omit
+                    // until we can see that element. The +1 is because of how indexes start at 0.
                     self.window_index.index = self.current_index - num_visible_rows + 1;
-                    self.window_index.index
                 } else {
                     // Else, if it is not past the last element visible, do not omit anything
-                    0
+                    self.window_index.index = 0;
                 }
             }
             ScrollDirection::Up => {
@@ -101,15 +99,17 @@ impl Scrollable {
                     // just put it so that the selected index is the last entry,
                     self.window_index.index = self.current_index - num_visible_rows + 1;
                 }
-                // Else, don't change what our start position is from whatever it is set to!
-                self.window_index.index
             }
-        };
+        }
 
-        self.tui_state
-            .select(Some(self.current_index.saturating_sub(list_start)));
+        // Ensure we don't select a non-existent index.
+        self.window_index.index = min(self.num_items.saturating_sub(1), self.window_index.index);
 
-        list_start
+        self.tui_state.select(Some(
+            self.current_index.saturating_sub(self.window_index.index),
+        ));
+
+        self.window_index.index
     }
 
     /// Update the index with this!  This will automatically update the scroll direction as well!

src/app/widgets/base/text_table.rs

@@ -255,6 +255,7 @@ where
             left_to_right: bool, mut desired_widths: Vec<DesiredColumnWidth>, total_width: u16,
         ) -> Vec<u16> {
             let mut total_width_left = total_width;
+            let num_widths = desired_widths.len();
             if !left_to_right {
                 desired_widths.reverse();
             }
@@ -302,6 +303,7 @@ where
                 }
 
                 if !left_to_right {
+                    column_widths.extend(vec![0; num_widths.saturating_sub(column_widths.len())]);
                     column_widths.reverse();
                 }
             }
@@ -384,7 +386,8 @@ where
         use tui::widgets::Row;
 
         let inner_area = block.inner(block_area);
-        if inner_area.height < 2 {
+        if inner_area.height < 1 {
+            f.render_widget(block, block_area);
             return;
         }
         let table_gap = if !self.show_gap || inner_area.height < TABLE_GAP_HEIGHT_LIMIT {
@@ -415,11 +418,13 @@ where
         // Then calculate rows. We truncate the amount of data read based on height,
         // as well as truncating some entries based on available width.
         let data_slice = {
+            // Note: `get_list_start` already ensures `start` is within the bounds of the number of items, so no need to check!
             let start = self.scrollable.get_list_start(scrollable_height as usize);
-            let end = std::cmp::min(
+            let end = min(
                 self.scrollable.num_items(),
                 start + scrollable_height as usize,
             );
+
             &data[start..end]
         };
         let rows = data_slice.iter().map(|row| {

src/app/widgets/base/time_graph.rs

@@ -23,7 +23,10 @@ use crate::{
         AppConfigFields, Component,
     },
     canvas::Painter,
-    constants::{AUTOHIDE_TIMEOUT_MILLISECONDS, STALE_MAX_MILLISECONDS, STALE_MIN_MILLISECONDS},
+    constants::{
+        AUTOHIDE_TIMEOUT_MILLISECONDS, STALE_MAX_MILLISECONDS, STALE_MIN_MILLISECONDS,
+        TIME_LABEL_HEIGHT_LIMIT,
+    },
 };
 
 #[derive(Clone)]
@@ -169,7 +172,9 @@ impl TimeGraph {
     fn zoom_in(&mut self) -> WidgetEventResult {
         let new_time = self.current_display_time.saturating_sub(self.time_interval);
 
-        if new_time >= self.min_duration {
+        if self.current_display_time == new_time {
+            WidgetEventResult::NoRedraw
+        } else if new_time >= self.min_duration {
             self.current_display_time = new_time;
             self.autohide_timer.start_display_timer();
 
@@ -187,7 +192,9 @@ impl TimeGraph {
     fn zoom_out(&mut self) -> WidgetEventResult {
         let new_time = self.current_display_time + self.time_interval;
 
-        if new_time <= self.max_duration {
+        if self.current_display_time == new_time {
+            WidgetEventResult::NoRedraw
+        } else if new_time <= self.max_duration {
             self.current_display_time = new_time;
             self.autohide_timer.start_display_timer();
 
@@ -246,7 +253,7 @@ impl TimeGraph {
             let x_axis = Axis::default()
                 .bounds([time_start, 0.0])
                 .style(painter.colours.graph_style);
-            if self.autohide_timer.is_showing() {
+            if inner_area.height >= TIME_LABEL_HEIGHT_LIMIT && self.autohide_timer.is_showing() {
                 x_axis.labels(self.get_x_axis_labels(painter))
             } else {
                 x_axis
@@ -261,6 +268,7 @@ impl TimeGraph {
                     .map(|label| Span::styled(label.clone(), painter.colours.graph_style))
                     .collect(),
             );
+        // TODO: [Small size bug] There's a rendering issue if you use a very short window with how some legend entries are hidden. It sometimes hides the 0; instead, it should hide middle entries!
 
         let mut datasets: Vec<Dataset<'_>> = data
             .iter()

src/app/widgets/bottom_widgets/basic_cpu.rs

@@ -175,13 +175,12 @@ impl Widget for BasicCpu {
                         "{:3}",
                         data.cpu_count
                             .map(|c| c.to_string())
-                            .unwrap_or(data.cpu_prefix.clone())
+                            .unwrap_or_else(|| data.cpu_prefix.clone())
                     ),
                     format!("{:3.0}%", data.cpu_usage.round()),
                 )
             })
             .collect::<Vec<_>>();
-
     }
 
     fn width(&self) -> LayoutRule {

src/app/widgets/bottom_widgets/battery.rs

@@ -8,7 +8,7 @@ use tui::{
     backend::Backend,
     layout::{Constraint, Direction, Layout, Rect},
     text::{Span, Spans},
-    widgets::{Block, Borders, Paragraph, Tabs},
+    widgets::{Borders, Paragraph, Tabs},
     Frame,
 };
 
@@ -144,11 +144,11 @@ impl Component for BatteryTable {
 
     fn handle_mouse_event(&mut self, event: MouseEvent) -> WidgetEventResult {
         for (itx, bound) in self.tab_bounds.iter().enumerate() {
-            if does_bound_intersect_coordinate(event.column, event.row, *bound) {
-                if itx < self.battery_data.len() {
-                    self.selected_index = itx;
-                    return WidgetEventResult::Redraw;
-                }
+            if does_bound_intersect_coordinate(event.column, event.row, *bound)
+                && itx < self.battery_data.len()
+            {
+                self.selected_index = itx;
+                return WidgetEventResult::Redraw;
             }
         }
         WidgetEventResult::NoRedraw
@@ -178,13 +178,7 @@ impl Widget for BatteryTable {
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,
     ) {
-        let block = Block::default()
-            .border_style(if selected {
-                painter.colours.highlighted_border_style
-            } else {
-                painter.colours.border_style
-            })
-            .borders(self.block_border.clone());
+        let block = self.block(painter, selected, self.block_border);
 
         let inner_area = block.inner(area);
         const CONSTRAINTS: [Constraint; 2] = [Constraint::Length(1), Constraint::Min(0)];
@@ -192,29 +186,31 @@ impl Widget for BatteryTable {
             .direction(Direction::Vertical)
             .constraints(CONSTRAINTS)
             .split(inner_area);
-        let tab_area = Rect::new(
-            split_area[0].x.saturating_sub(1),
-            split_area[0].y,
-            split_area[0].width,
-            split_area[0].height,
-        );
-        let data_area = if inner_area.height >= TABLE_GAP_HEIGHT_LIMIT && split_area[1].height > 0 {
-            Rect::new(
-                split_area[1].x,
-                split_area[1].y + 1,
-                split_area[1].width,
-                split_area[1].height - 1,
-            )
-        } else {
-            split_area[1]
-        };
 
         if self.battery_data.is_empty() {
             f.render_widget(
                 Paragraph::new("No batteries found").style(painter.colours.text_style),
-                tab_area,
+                split_area[0],
             );
         } else {
+            let tab_area = Rect::new(
+                split_area[0].x.saturating_sub(1),
+                split_area[0].y,
+                split_area[0].width,
+                split_area[0].height,
+            );
+            let data_area =
+                if inner_area.height >= TABLE_GAP_HEIGHT_LIMIT && split_area[1].height > 0 {
+                    Rect::new(
+                        split_area[1].x,
+                        split_area[1].y + 1,
+                        split_area[1].width,
+                        split_area[1].height - 1,
+                    )
+                } else {
+                    split_area[1]
+                };
+
             let battery_tab_names = self
                 .battery_data
                 .iter()

src/app/widgets/bottom_widgets/carousel.rs

@@ -64,7 +64,7 @@ impl Carousel {
 
     /// Returns the currently selected [`NodeId`] if possible.
     pub fn get_currently_selected(&self) -> Option<NodeId> {
-        self.children.get(self.index).map(|i| i.0.clone())
+        self.children.get(self.index).map(|i| i.0)
     }
 
     fn get_next(&self) -> Option<&(NodeId, Cow<'static, str>)> {

src/app/widgets/bottom_widgets/cpu.rs

@@ -80,7 +80,7 @@ impl CpuGraph {
         let graph = TimeGraph::from_config(app_config_fields);
         let legend = TextTable::new(vec![
             SimpleColumn::new_flex("CPU".into(), 0.5),
-            SimpleColumn::new_hard("Use%".into(), None),
+            SimpleColumn::new_hard("Use".into(), None),
         ])
         .default_ltr(false);
         let legend_position = if app_config_fields.left_legend {
@@ -165,12 +165,17 @@ impl Widget for CpuGraph {
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,
     ) {
-        let constraints = match self.legend_position {
-            CpuGraphLegendPosition::Left => {
-                [Constraint::Percentage(15), Constraint::Percentage(85)]
-            }
-            CpuGraphLegendPosition::Right => {
-                [Constraint::Percentage(85), Constraint::Percentage(15)]
+        let constraints = {
+            const CPU_LEGEND_MIN_WIDTH: u16 = 10;
+            let (legend_constraint, cpu_constraint) =
+                if area.width * 15 / 100 < CPU_LEGEND_MIN_WIDTH {
+                    (Constraint::Length(CPU_LEGEND_MIN_WIDTH), Constraint::Min(0))
+                } else {
+                    (Constraint::Percentage(15), Constraint::Percentage(85))
+                };
+            match self.legend_position {
+                CpuGraphLegendPosition::Left => [legend_constraint, cpu_constraint],
+                CpuGraphLegendPosition::Right => [cpu_constraint, legend_constraint],
             }
         };
 
@@ -180,6 +185,11 @@ impl Widget for CpuGraph {
             .constraints(constraints)
             .split(area);
 
+        let (graph_block_area, legend_block_area) = match self.legend_position {
+            CpuGraphLegendPosition::Left => (split_area[1], split_area[0]),
+            CpuGraphLegendPosition::Right => (split_area[0], split_area[1]),
+        };
+
         const Y_BOUNDS: [f64; 2] = [0.0, 100.5];
         let y_bound_labels: [Cow<'static, str>; 2] = ["0%".into(), "100%".into()];
 
@@ -217,6 +227,35 @@ impl Widget for CpuGraph {
             })
             .collect::<Vec<_>>();
 
+        let graph_block = self.block(
+            painter,
+            selected && matches!(&self.selected, CpuGraphSelection::Graph),
+            Borders::ALL,
+        );
+
+        self.graph.draw_tui_chart(
+            painter,
+            f,
+            &cpu_data,
+            &y_bound_labels,
+            Y_BOUNDS,
+            true,
+            graph_block,
+            graph_block_area,
+        );
+
+        let legend_block = Block::default()
+            .border_style(if selected {
+                if let CpuGraphSelection::Legend = &self.selected {
+                    painter.colours.highlighted_border_style
+                } else {
+                    painter.colours.border_style
+                }
+            } else {
+                painter.colours.border_style
+            })
+            .borders(Borders::ALL);
+
         let legend_data = self
             .display_data
             .iter()
@@ -249,46 +288,6 @@ impl Widget for CpuGraph {
             })
             .collect::<Vec<_>>();
 
-        let graph_block = Block::default()
-            .border_style(if selected {
-                if let CpuGraphSelection::Graph = &self.selected {
-                    painter.colours.highlighted_border_style
-                } else {
-                    painter.colours.border_style
-                }
-            } else {
-                painter.colours.border_style
-            })
-            .borders(Borders::ALL);
-
-        let legend_block = Block::default()
-            .border_style(if selected {
-                if let CpuGraphSelection::Legend = &self.selected {
-                    painter.colours.highlighted_border_style
-                } else {
-                    painter.colours.border_style
-                }
-            } else {
-                painter.colours.border_style
-            })
-            .borders(Borders::ALL);
-
-        let (graph_block_area, legend_block_area) = match self.legend_position {
-            CpuGraphLegendPosition::Left => (split_area[1], split_area[0]),
-            CpuGraphLegendPosition::Right => (split_area[0], split_area[1]),
-        };
-
-        self.graph.draw_tui_chart(
-            painter,
-            f,
-            &cpu_data,
-            &y_bound_labels,
-            Y_BOUNDS,
-            true,
-            graph_block,
-            graph_block_area,
-        );
-
         self.legend.draw_tui_table(
             painter,
             f,

src/app/widgets/bottom_widgets/disk.rs

@@ -1,12 +1,7 @@
 use std::collections::HashMap;
 
 use crossterm::event::{KeyEvent, MouseEvent};
-use tui::{
-    backend::Backend,
-    layout::Rect,
-    widgets::{Block, Borders},
-    Frame,
-};
+use tui::{backend::Backend, layout::Rect, widgets::Borders, Frame};
 
 use crate::{
     app::{
@@ -127,13 +122,7 @@ impl Widget for DiskTable {
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,
     ) {
-        let block = Block::default()
-            .border_style(if selected {
-                painter.colours.highlighted_border_style
-            } else {
-                painter.colours.border_style
-            })
-            .borders(self.block_border.clone());
+        let block = self.block(painter, selected, self.block_border);
 
         self.table
             .draw_tui_table(painter, f, &self.display_data, block, area, selected);

src/app/widgets/bottom_widgets/mem.rs

@@ -1,11 +1,7 @@
 use std::{borrow::Cow, collections::HashMap, time::Instant};
 
 use crossterm::event::{KeyEvent, MouseEvent};
-use tui::{
-    backend::Backend,
-    layout::Rect,
-    widgets::{Block, Borders},
-};
+use tui::{backend::Backend, layout::Rect, widgets::Borders};
 
 use crate::{
     app::{event::WidgetEventResult, time_graph::TimeGraphData, DataCollection},
@@ -93,13 +89,7 @@ impl Widget for MemGraph {
         &mut self, painter: &crate::canvas::Painter, f: &mut tui::Frame<'_, B>, area: Rect,
         selected: bool,
     ) {
-        let block = Block::default()
-            .border_style(if selected {
-                painter.colours.highlighted_border_style
-            } else {
-                painter.colours.border_style
-            })
-            .borders(Borders::ALL);
+        let block = self.block(painter, selected, Borders::ALL);
 
         let mut chart_data = Vec::with_capacity(2);
         if let Some((label_percent, label_frac)) = &self.mem_labels {

src/app/widgets/bottom_widgets/net.rs

@@ -518,13 +518,7 @@ impl Widget for NetGraph {
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,
     ) {
-        let block = Block::default()
-            .border_style(if selected {
-                painter.colours.highlighted_border_style
-            } else {
-                painter.colours.border_style
-            })
-            .borders(Borders::ALL);
+        let block = self.block(painter, selected, Borders::ALL);
 
         self.set_draw_cache();
 
@@ -686,6 +680,7 @@ impl Widget for OldNetGraph {
                 painter.colours.border_style
             })
             .borders(Borders::ALL);
+
         self.table.draw_tui_table(
             painter,
             f,

src/app/widgets/bottom_widgets/process.rs

@@ -1233,17 +1233,11 @@ impl Widget for ProcessManager {
             area
         };
 
-        let process_block = Block::default()
-            .border_style(if selected {
-                if let ProcessManagerSelection::Processes = self.selected {
-                    painter.colours.highlighted_border_style
-                } else {
-                    painter.colours.border_style
-                }
-            } else {
-                painter.colours.border_style
-            })
-            .borders(self.block_border.clone());
+        let process_block = self.block(
+            painter,
+            selected && matches!(self.selected, ProcessManagerSelection::Processes),
+            self.block_border,
+        );
 
         self.process_table.draw_tui_table(
             painter,

src/app/widgets/bottom_widgets/temp.rs

@@ -1,12 +1,7 @@
 use std::collections::HashMap;
 
 use crossterm::event::{KeyEvent, MouseEvent};
-use tui::{
-    backend::Backend,
-    layout::Rect,
-    widgets::{Block, Borders},
-    Frame,
-};
+use tui::{backend::Backend, layout::Rect, widgets::Borders, Frame};
 
 use crate::{
     app::{
@@ -138,13 +133,7 @@ impl Widget for TempTable {
     fn draw<B: Backend>(
         &mut self, painter: &Painter, f: &mut Frame<'_, B>, area: Rect, selected: bool,
     ) {
-        let block = Block::default()
-            .border_style(if selected {
-                painter.colours.highlighted_border_style
-            } else {
-                painter.colours.border_style
-            })
-            .borders(self.block_border.clone());
+        let block = self.block(painter, selected, self.block_border);
 
         self.table
             .draw_tui_table(painter, f, &self.display_data, block, area, selected);

src/canvas.rs

@@ -384,10 +384,7 @@ impl Painter {
                                     // );
 
                                     if let TmpBottomWidget::Carousel(carousel) = widget {
-                                        let carousel: &mut crate::app::widgets::Carousel =
-                                            carousel.into();
-
-                                        let remaining_area =
+                                        let remaining_area: Rect =
                                             carousel.draw_carousel(painter, f, area);
                                         if let Some(to_draw_node) =
                                             carousel.get_currently_selected()
@@ -395,10 +392,6 @@ impl Painter {
                                             if let Some(child_widget) =
                                                 lookup_map.get_mut(&to_draw_node)
                                             {
-                                                debug!(
-                                                    "Selected ID: {:?}, to_draw_node: {:?}",
-                                                    selected_id, to_draw_node
-                                                );
                                                 child_widget.set_bounds(remaining_area);
                                                 child_widget.draw(
                                                     painter,
@@ -431,7 +424,7 @@ impl Painter {
                 let lookup_map = &mut app_state.widget_lookup_map;
                 traverse_and_draw_tree(
                     *root,
-                    &arena,
+                    arena,
                     f,
                     lookup_map,
                     self,

src/constants.rs

@@ -20,7 +20,7 @@ pub const MAX_KEY_TIMEOUT_IN_MILLISECONDS: u64 = 1000;
 
 // Limits for when we should stop showing table gaps/labels (anything less means not shown)
 pub const TABLE_GAP_HEIGHT_LIMIT: u16 = 5;
-pub const TIME_LABEL_HEIGHT_LIMIT: u16 = 7;
+pub const TIME_LABEL_HEIGHT_LIMIT: u16 = 5;
 
 // For kill signals
 #[cfg(target_os = "windows")]