refactor: share implementation for pecentage-based time graphs (#1736)

* refactor: move components to a 'drawing' folder

* Revert "refactor: move components to a 'drawing' folder"

This reverts commit a1316bdf3aa4437bed2ca786896c2b387ccf5f0e.

* move stuff out of constants because it sucks

* move more things!

* cleanup

* some restructuring

* refactor percent time graph to common impl

* wow thanks copilot

src/app.rs

@@ -26,6 +26,8 @@ use crate::{
     widgets::{ProcWidgetColumn, ProcWidgetMode},
 };
 
+const STALE_MIN_MILLISECONDS: u64 = 30 * 1000; // Lowest is 30 seconds
+
 #[derive(Debug, Clone, Eq, PartialEq, Default, Copy)]
 pub enum AxisScaling {
     #[default]
@@ -1091,13 +1093,15 @@ impl App {
             return;
         }
 
+        const MAX_KEY_TIMEOUT_IN_MILLISECONDS: u64 = 1000;
+
         // Forbid any char key presses when showing a dialog box...
         if !self.ignore_normal_keybinds() {
             let current_key_press_inst = Instant::now();
             if current_key_press_inst
                 .duration_since(self.last_key_press)
                 .as_millis()
-                > constants::MAX_KEY_TIMEOUT_IN_MILLISECONDS.into()
+                > MAX_KEY_TIMEOUT_IN_MILLISECONDS.into()
             {
                 self.reset_multi_tap_keys();
             }
@@ -2315,15 +2319,13 @@ impl App {
                         .current_display_time
                         .saturating_sub(self.app_config_fields.time_interval);
 
-                    if new_time >= constants::STALE_MIN_MILLISECONDS {
+                    if new_time >= STALE_MIN_MILLISECONDS {
                         cpu_widget_state.current_display_time = new_time;
                         if self.app_config_fields.autohide_time {
                             cpu_widget_state.autohide_timer = Some(Instant::now());
                         }
-                    } else if cpu_widget_state.current_display_time
+                    } else if cpu_widget_state.current_display_time != STALE_MIN_MILLISECONDS {
-                        != constants::STALE_MIN_MILLISECONDS
+                        cpu_widget_state.current_display_time = STALE_MIN_MILLISECONDS;
-                    {
-                        cpu_widget_state.current_display_time = constants::STALE_MIN_MILLISECONDS;
                         if self.app_config_fields.autohide_time {
                             cpu_widget_state.autohide_timer = Some(Instant::now());
                         }
@@ -2341,15 +2343,13 @@ impl App {
                         .current_display_time
                         .saturating_sub(self.app_config_fields.time_interval);
 
-                    if new_time >= constants::STALE_MIN_MILLISECONDS {
+                    if new_time >= STALE_MIN_MILLISECONDS {
                         mem_widget_state.current_display_time = new_time;
                         if self.app_config_fields.autohide_time {
                             mem_widget_state.autohide_timer = Some(Instant::now());
                         }
-                    } else if mem_widget_state.current_display_time
+                    } else if mem_widget_state.current_display_time != STALE_MIN_MILLISECONDS {
-                        != constants::STALE_MIN_MILLISECONDS
+                        mem_widget_state.current_display_time = STALE_MIN_MILLISECONDS;
-                    {
-                        mem_widget_state.current_display_time = constants::STALE_MIN_MILLISECONDS;
                         if self.app_config_fields.autohide_time {
                             mem_widget_state.autohide_timer = Some(Instant::now());
                         }
@@ -2367,15 +2367,13 @@ impl App {
                         .current_display_time
                         .saturating_sub(self.app_config_fields.time_interval);
 
-                    if new_time >= constants::STALE_MIN_MILLISECONDS {
+                    if new_time >= STALE_MIN_MILLISECONDS {
                         net_widget_state.current_display_time = new_time;
                         if self.app_config_fields.autohide_time {
                             net_widget_state.autohide_timer = Some(Instant::now());
                         }
-                    } else if net_widget_state.current_display_time
+                    } else if net_widget_state.current_display_time != STALE_MIN_MILLISECONDS {
-                        != constants::STALE_MIN_MILLISECONDS
+                        net_widget_state.current_display_time = STALE_MIN_MILLISECONDS;
-                    {
-                        net_widget_state.current_display_time = constants::STALE_MIN_MILLISECONDS;
                         if self.app_config_fields.autohide_time {
                             net_widget_state.autohide_timer = Some(Instant::now());
                         }

src/canvas.rs

@@ -1,3 +1,8 @@
+//! Code related to drawing.
+//!
+//! Note that eventually this should not contain any widget-specific draw code, but rather just generic code
+//! or components.
+
 pub mod components;
 mod dialogs;
 mod drawing_utils;

src/canvas/components/mod.rs

@@ -1,4 +1,4 @@
-//! Lower-level components used throughout bottom.
+//! Lower-level or shared drawing components used throughout bottom.
 
 pub mod data_table;
 pub mod pipe_gauge;

src/canvas/components/time_graph/base.rs

@@ -141,9 +141,7 @@ impl TimeGraph<'_> {
     ///   graph.
     /// - Expects `graph_data`, which represents *what* data to draw, and
     ///   various details like style and optional legends.
-    pub fn draw_time_graph(
+    pub fn draw(&self, f: &mut Frame<'_>, draw_loc: Rect, graph_data: Vec<GraphData<'_>>) {
-        &self, f: &mut Frame<'_>, draw_loc: Rect, graph_data: Vec<GraphData<'_>>,
-    ) {
         // TODO: (points_rework_v1) can we reduce allocations in the underlying graph by saving some sort of state?
 
         let x_axis = self.generate_x_axis();

src/canvas/components/time_graph/mod.rs

@@ -0,0 +1,4 @@
+mod base;
+pub mod variants;
+
+pub(crate) use base::*;

src/canvas/components/time_graph/variants/auto_y_axis.rs

@@ -0,0 +1,2 @@
+//! A variant of a [`crate::canvas::components::time_graph::TimeGraph`] that
+//! automatically adjusts the y-axis based on the data provided.

src/canvas/components/time_graph/variants/mod.rs

@@ -0,0 +1,15 @@
+use tui::style::Style;
+
+use crate::options::config::style::Styles;
+
+pub(crate) mod auto_y_axis;
+pub(crate) mod percent;
+
+fn get_border_style(styles: &Styles, widget_id: u64, selected_widget_id: u64) -> Style {
+    let is_on_widget = widget_id == selected_widget_id;
+    if is_on_widget {
+        styles.highlighted_border_style
+    } else {
+        styles.border_style
+    }
+}

src/canvas/components/time_graph/variants/percent.rs

@@ -0,0 +1,96 @@
+//! A variant of a [`TimeGraph`] that expects data to be in a percentage format, from 0.0 to 100.0.
+
+use std::borrow::Cow;
+
+use tui::{layout::Constraint, symbols::Marker};
+
+use crate::{
+    app::AppConfigFields,
+    canvas::components::time_graph::{
+        AxisBound, ChartScaling, LegendPosition, TimeGraph, variants::get_border_style,
+    },
+    options::config::style::Styles,
+};
+
+/// Acts as a wrapper for a [`TimeGraph`] that expects data to be in a percentage format,
+pub(crate) struct PercentTimeGraph<'a> {
+    /// The total display range of the graph in milliseconds.
+    ///
+    /// TODO: Make this a [`std::time::Duration`].
+    pub(crate) display_range: u64,
+
+    /// Whether to hide the x-axis labels.
+    pub(crate) hide_x_labels: bool,
+
+    /// The app config fields.
+    ///
+    /// This is mostly used as a shared mutability workaround due to [`App`]
+    /// being a giant state struct.
+    pub(crate) app_config_fields: &'a AppConfigFields,
+
+    /// The current widget selected by the app.
+    ///
+    /// This is mostly used as a shared mutability workaround due to [`App`]
+    /// being a giant state struct.
+    pub(crate) current_widget: u64,
+
+    /// Whether the current widget is expanded.
+    ///  
+    /// This is mostly used as a shared mutability workaround due to [`App`]
+    /// being a giant state struct.
+    pub(crate) is_expanded: bool,
+
+    /// The title of the graph.
+    pub(crate) title: Cow<'a, str>,
+
+    /// A reference to the styles.
+    pub(crate) styles: &'a Styles,
+
+    /// The widget ID corresponding to this graph.
+    pub(crate) widget_id: u64,
+
+    /// The position of the legend.
+    pub(crate) legend_position: Option<LegendPosition>,
+
+    /// The constraints for the legend.
+    pub(crate) legend_constraints: Option<(Constraint, Constraint)>,
+}
+
+impl<'a> PercentTimeGraph<'a> {
+    /// Return the final [`TimeGraph`].
+    pub fn build(self) -> TimeGraph<'a> {
+        const Y_BOUNDS: AxisBound = AxisBound::Max(100.5);
+        const Y_LABELS: [Cow<'static, str>; 2] = [Cow::Borrowed("  0%"), Cow::Borrowed("100%")];
+
+        let x_min = -(self.display_range as f64);
+
+        let marker = if self.app_config_fields.use_dot {
+            Marker::Dot
+        } else {
+            Marker::Braille
+        };
+
+        let graph_style = self.styles.graph_style;
+        let border_style = get_border_style(self.styles, self.widget_id, self.current_widget);
+        let title_style = self.styles.widget_title_style;
+        let border_type = self.styles.border_type;
+
+        TimeGraph {
+            x_min,
+            hide_x_labels: self.hide_x_labels,
+            y_bounds: Y_BOUNDS,
+            y_labels: &Y_LABELS,
+            graph_style,
+            border_style,
+            border_type,
+            title: self.title,
+            is_selected: self.current_widget == self.widget_id,
+            is_expanded: self.is_expanded,
+            title_style,
+            legend_position: self.legend_position,
+            legend_constraints: self.legend_constraints,
+            marker,
+            scaling: ChartScaling::Linear,
+        }
+    }
+}

src/canvas/drawing_utils.rs

@@ -5,13 +5,14 @@ use tui::{
     widgets::{Block, BorderType, Borders},
 };
 
-use super::SIDE_BORDERS;
+pub const SIDE_BORDERS: Borders = Borders::LEFT.union(Borders::RIGHT);
+pub const AUTOHIDE_TIMEOUT_MILLISECONDS: u64 = 5000; // 5 seconds to autohide
 
 /// Determine whether a graph x-label should be hidden.
 pub fn should_hide_x_label(
     always_hide_time: bool, autohide_time: bool, timer: &mut Option<Instant>, draw_loc: Rect,
 ) -> bool {
-    use crate::constants::*;
+    const TIME_LABEL_HEIGHT_LIMIT: u16 = 7;
 
     if always_hide_time || (autohide_time && timer.is_none()) {
         true
@@ -62,8 +63,6 @@ mod test {
 
         use tui::layout::Rect;
 
-        use crate::constants::*;
-
         let rect = Rect::new(0, 0, 10, 10);
         let small_rect = Rect::new(0, 0, 10, 6);
 
@@ -91,4 +90,14 @@ mod test {
         ));
         assert!(over_timer.is_none());
     }
+
+    /// This test exists because previously, [`SIDE_BORDERS`] was set
+    /// incorrectly after I moved from tui-rs to ratatui.
+    #[test]
+    fn assert_side_border_bits_match() {
+        assert_eq!(
+            SIDE_BORDERS,
+            Borders::ALL.difference(Borders::TOP.union(Borders::BOTTOM))
+        )
+    }
 }

src/canvas/widgets/cpu_graph.rs

@@ -1,9 +1,6 @@
-use std::borrow::Cow;
-
 use tui::{
     Frame,
     layout::{Constraint, Direction, Layout, Rect},
-    symbols::Marker,
 };
 
 use crate::{
@@ -12,7 +9,7 @@ use crate::{
         Painter,
         components::{
             data_table::{DrawInfo, SelectionState},
-            time_graph::{AxisBound, GraphData, TimeGraph},
+            time_graph::{GraphData, variants::percent::PercentTimeGraph},
         },
         drawing_utils::should_hide_x_label,
     },
@@ -120,7 +117,7 @@ impl Painter {
     }
 
     fn generate_points<'a>(
-        &self, cpu_widget_state: &'a mut CpuWidgetState, data: &'a StoredData, show_avg_cpu: bool,
+        &self, cpu_widget_state: &'a CpuWidgetState, data: &'a StoredData, show_avg_cpu: bool,
     ) -> Vec<GraphData<'a>> {
         let show_avg_offset = if show_avg_cpu { AVG_POSITION } else { 0 };
         let current_scroll_position = cpu_widget_state.table.state.current_index;
@@ -172,15 +169,10 @@ impl Painter {
     fn draw_cpu_graph(
         &self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect, widget_id: u64,
     ) {
-        const Y_BOUNDS: AxisBound = AxisBound::Max(100.5);
-        const Y_LABELS: [Cow<'static, str>; 2] = [Cow::Borrowed("  0%"), Cow::Borrowed("100%")];
-
         if let Some(cpu_widget_state) = app_state.states.cpu_state.widget_states.get_mut(&widget_id)
         {
             let data = app_state.data_store.get_data();
 
-            let border_style = self.get_border_style(widget_id, app_state.current_widget.widget_id);
-            let x_min = -(cpu_widget_state.current_display_time as f64);
             let hide_x_labels = should_hide_x_label(
                 app_state.app_config_fields.hide_time,
                 app_state.app_config_fields.autohide_time,
@@ -212,30 +204,20 @@ impl Painter {
                 }
             };
 
-            let marker = if app_state.app_config_fields.use_dot {
+            PercentTimeGraph {
-                Marker::Dot
+                display_range: cpu_widget_state.current_display_time,
-            } else {
-                Marker::Braille
-            };
-
-            TimeGraph {
-                x_min,
                 hide_x_labels,
-                y_bounds: Y_BOUNDS,
+                app_config_fields: &app_state.app_config_fields,
-                y_labels: &Y_LABELS,
+                current_widget: app_state.current_widget.widget_id,
-                graph_style: self.styles.graph_style,
-                border_style,
-                border_type: self.styles.border_type,
-                title,
-                is_selected: app_state.current_widget.widget_id == widget_id,
                 is_expanded: app_state.is_expanded,
-                title_style: self.styles.widget_title_style,
+                title,
+                styles: &self.styles,
+                widget_id,
                 legend_position: None,
                 legend_constraints: None,
-                marker,
-                scaling: Default::default(),
             }
-            .draw_time_graph(f, draw_loc, graph_data);
+            .build()
+            .draw(f, draw_loc, graph_data);
         }
     }
 

src/canvas/widgets/mem_graph.rs

@@ -1,17 +1,16 @@
-use std::{borrow::Cow, time::Instant};
+use std::time::Instant;
 
 use tui::{
     Frame,
     layout::{Constraint, Rect},
     style::Style,
-    symbols::Marker,
 };
 
 use crate::{
     app::{App, data::Values},
     canvas::{
         Painter,
-        components::time_graph::{AxisBound, GraphData, TimeGraph},
+        components::time_graph::{GraphData, variants::percent::PercentTimeGraph},
         drawing_utils::should_hide_x_label,
     },
     collection::memory::MemData,
@@ -57,12 +56,7 @@ impl Painter {
     pub fn draw_memory_graph(
         &self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect, widget_id: u64,
     ) {
-        const Y_BOUNDS: AxisBound = AxisBound::Max(100.5);
-        const Y_LABELS: [Cow<'static, str>; 2] = [Cow::Borrowed("  0%"), Cow::Borrowed("100%")];
-
         if let Some(mem_state) = app_state.states.mem_state.widget_states.get_mut(&widget_id) {
-            let border_style = self.get_border_style(widget_id, app_state.current_widget.widget_id);
-            let x_min = -(mem_state.current_display_time as f64);
             let hide_x_labels = should_hide_x_label(
                 app_state.app_config_fields.hide_time,
                 app_state.app_config_fields.autohide_time,
@@ -170,30 +164,20 @@ impl Painter {
                 points
             };
 
-            let marker = if app_state.app_config_fields.use_dot {
+            PercentTimeGraph {
-                Marker::Dot
+                display_range: mem_state.current_display_time,
-            } else {
-                Marker::Braille
-            };
-
-            TimeGraph {
-                x_min,
                 hide_x_labels,
-                y_bounds: Y_BOUNDS,
+                app_config_fields: &app_state.app_config_fields,
-                y_labels: &Y_LABELS,
+                current_widget: app_state.current_widget.widget_id,
-                graph_style: self.styles.graph_style,
-                border_style,
-                border_type: self.styles.border_type,
-                title: " Memory ".into(),
-                is_selected: app_state.current_widget.widget_id == widget_id,
                 is_expanded: app_state.is_expanded,
-                title_style: self.styles.widget_title_style,
+                title: " Memory ".into(),
+                styles: &self.styles,
+                widget_id,
                 legend_position: app_state.app_config_fields.memory_legend_position,
                 legend_constraints: Some((Constraint::Ratio(3, 4), Constraint::Ratio(3, 4))),
-                marker,
-                scaling: Default::default(),
             }
-            .draw_time_graph(f, draw_loc, graph_data);
+            .build()
+            .draw(f, draw_loc, graph_data);
         }
 
         if app_state.should_get_widget_bounds() {

src/canvas/widgets/network_graph.rs

@@ -240,7 +240,7 @@ impl Painter {
                 marker,
                 scaling,
             }
-            .draw_time_graph(f, draw_loc, graph_data);
+            .draw(f, draw_loc, graph_data);
         }
     }
 

src/constants.rs

@@ -1,25 +1,13 @@
-use tui::widgets::Borders;
+//! A bunch of constants used throughout the application.
+//!
+//! FIXME: Move these to where it makes more sense.
 
 // Default widget ID
 pub const DEFAULT_WIDGET_ID: u64 = 56709;
 
-// How much data is SHOWN
-pub const DEFAULT_TIME_MILLISECONDS: u64 = 60 * 1000; // Defaults to 1 min.
-pub const STALE_MIN_MILLISECONDS: u64 = 30 * 1000; // Lowest is 30 seconds
-pub const TIME_CHANGE_MILLISECONDS: u64 = 15 * 1000; // How much to increment each time
-pub const AUTOHIDE_TIMEOUT_MILLISECONDS: u64 = 5000; // 5 seconds to autohide
-
-// How fast the screen refreshes
-pub const DEFAULT_REFRESH_RATE_IN_MILLISECONDS: u64 = 1000;
-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 = 7;
-pub const TIME_LABEL_HEIGHT_LIMIT: u16 = 7;
-
-// Side borders
-pub const SIDE_BORDERS: Borders = Borders::LEFT.union(Borders::RIGHT);
 
 // Help text
 const HELP_CONTENTS_TEXT: [&str; 10] = [
@@ -581,16 +569,6 @@ mod test {
         }
     }
 
-    /// This test exists because previously, [`SIDE_BORDERS`] was set
-    /// incorrectly after I moved from tui-rs to ratatui.
-    #[test]
-    fn assert_side_border_bits_match() {
-        assert_eq!(
-            SIDE_BORDERS,
-            Borders::ALL.difference(Borders::TOP.union(Borders::BOTTOM))
-        )
-    }
-
     /// Checks that the default config is valid.
     #[test]
     #[cfg(feature = "default")]

src/options.rs

@@ -670,8 +670,11 @@ macro_rules! parse_ms_option {
     }};
 }
 
+/// How fast the screen refreshes
 #[inline]
 fn get_update_rate(args: &BottomArgs, config: &Config) -> OptionResult<u64> {
+    const DEFAULT_REFRESH_RATE_IN_MILLISECONDS: u64 = 1000;
+
     parse_ms_option!(
         &args.general.rate,
         config.flags.as_ref().and_then(|flags| flags.rate.as_ref()),
@@ -735,6 +738,8 @@ fn get_dedicated_avg_row(config: &Config) -> bool {
 fn get_default_time_value(
     args: &BottomArgs, config: &Config, retention_ms: u64,
 ) -> OptionResult<u64> {
+    const DEFAULT_TIME_MILLISECONDS: u64 = 60 * 1000; // Defaults to 1 min.
+
     parse_ms_option!(
         &args.general.default_time_value,
         config
@@ -750,6 +755,8 @@ fn get_default_time_value(
 
 #[inline]
 fn get_time_interval(args: &BottomArgs, config: &Config, retention_ms: u64) -> OptionResult<u64> {
+    const TIME_CHANGE_MILLISECONDS: u64 = 15 * 1000; // How much to increment each time
+
     parse_ms_option!(
         &args.general.time_delta,
         config