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
             }
-        };
+        }
+
+        // Ensure we don't select a non-existent index.
+        self.window_index.index = min(self.num_items.saturating_sub(1), self.window_index.index);
 
-        self.tui_state
-            .select(Some(self.current_index.saturating_sub(list_start)));
+        self.tui_state.select(Some(
+            self.current_index.saturating_sub(self.window_index.index),
+        ));
 
-        list_start
+        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,43 +178,39 @@ 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)];
         let split_area = Layout::default()
             .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>)> {