bug: fix selected text bg colour being wrong if only the fg colour was set (#1021)

* rename file to be more generic

* fix selected text BG colour being wrong by default

* update changelog

* add test for bug

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Bug Fixes
 
+- [#1021](https://github.com/ClementTsang/bottom/pull/1021): Fix selected text background colour being wrong if only the foreground colour was set.
+
 ## Features
 
 - [#1016](https://github.com/ClementTsang/bottom/pull/1016): Add support for displaying process usernames on Windows.

src/bin/main.rs

@@ -19,7 +19,7 @@ use std::{
 
 use anyhow::{Context, Result};
 use bottom::{
-    canvas::{self, canvas_colours::CanvasColours},
+    canvas::{self, canvas_styling::CanvasColours},
     constants::*,
     data_conversion::*,
     options::*,

src/canvas.rs

@@ -1,6 +1,6 @@
 use std::str::FromStr;
 
-use canvas_colours::*;
+use canvas_styling::*;
 use itertools::izip;
 use tui::{
     backend::Backend,
@@ -21,7 +21,7 @@ use crate::{
     utils::error::BottomError,
 };
 
-pub mod canvas_colours;
+pub mod canvas_styling;
 mod dialogs;
 mod drawing_utils;
 mod widgets;

src/canvas/canvas_styling.rs

@@ -43,11 +43,15 @@ pub struct CanvasColours {
 impl Default for CanvasColours {
     fn default() -> Self {
         let text_colour = Color::Gray;
+        let currently_selected_text_colour = Color::Black;
+        let currently_selected_bg_colour = HIGHLIGHT_COLOUR;
 
         CanvasColours {
-            currently_selected_text_colour: Color::Black,
-            currently_selected_bg_colour: Color::Cyan,
-            currently_selected_text_style: Style::default().fg(Color::Black).bg(HIGHLIGHT_COLOUR),
+            currently_selected_text_colour,
+            currently_selected_bg_colour,
+            currently_selected_text_style: Style::default()
+                .fg(currently_selected_text_colour)
+                .bg(currently_selected_bg_colour),
             table_header_style: Style::default().fg(HIGHLIGHT_COLOUR),
             ram_style: Style::default().fg(FIRST_COLOUR),
             swap_style: Style::default().fg(SECOND_COLOUR),
@@ -240,95 +244,95 @@ impl CanvasColours {
     }
 
     pub fn set_disabled_text_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.disabled_text_style = get_style_from_config(colour)?;
+        self.disabled_text_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_text_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.text_style = get_style_from_config(colour)?;
+        self.text_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_border_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.border_style = get_style_from_config(colour)?;
+        self.border_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_highlighted_border_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.highlighted_border_style = get_style_from_config(colour)?;
+        self.highlighted_border_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_table_header_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.table_header_style = get_style_from_config(colour)?;
+        self.table_header_style = str_to_fg(colour)?;
         // Disabled as it seems to be bugged when I go into full command mode...?  It becomes huge lol
         // self.table_header_style = get_style_from_config(colour)?.modifier(Modifier::BOLD);
         Ok(())
     }
 
     pub fn set_ram_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.ram_style = get_style_from_config(colour)?;
+        self.ram_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_swap_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.swap_style = get_style_from_config(colour)?;
+        self.swap_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_arc_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.arc_style = get_style_from_config(colour)?;
+        self.arc_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_gpu_colours(&mut self, colours: &[Cow<'static, str>]) -> error::Result<()> {
         self.gpu_colour_styles = colours
             .iter()
-            .map(|colour| get_style_from_config(colour))
+            .map(|colour| str_to_fg(colour))
             .collect::<error::Result<Vec<Style>>>()?;
         Ok(())
     }
 
     pub fn set_rx_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.rx_style = get_style_from_config(colour)?;
+        self.rx_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_tx_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.tx_style = get_style_from_config(colour)?;
+        self.tx_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_rx_total_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.total_rx_style = get_style_from_config(colour)?;
+        self.total_rx_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_tx_total_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.total_tx_style = get_style_from_config(colour)?;
+        self.total_tx_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_avg_cpu_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.avg_colour_style = get_style_from_config(colour)?;
+        self.avg_colour_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_all_cpu_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.all_colour_style = get_style_from_config(colour)?;
+        self.all_colour_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_cpu_colours(&mut self, colours: &[Cow<'static, str>]) -> error::Result<()> {
         self.cpu_colour_styles = colours
             .iter()
-            .map(|colour| get_style_from_config(colour))
+            .map(|colour| str_to_fg(colour))
             .collect::<error::Result<Vec<Style>>>()?;
         Ok(())
     }
 
     pub fn set_scroll_entry_text_color(&mut self, colour: &str) -> error::Result<()> {
-        self.currently_selected_text_colour = get_colour_from_config(colour)?;
+        self.currently_selected_text_colour = str_to_colour(colour)?;
         self.currently_selected_text_style = Style::default()
             .fg(self.currently_selected_text_colour)
             .bg(self.currently_selected_bg_colour);
@@ -336,7 +340,7 @@ impl CanvasColours {
     }
 
     pub fn set_scroll_entry_bg_color(&mut self, colour: &str) -> error::Result<()> {
-        self.currently_selected_bg_colour = get_colour_from_config(colour)?;
+        self.currently_selected_bg_colour = str_to_colour(colour)?;
         self.currently_selected_text_style = Style::default()
             .fg(self.currently_selected_text_colour)
             .bg(self.currently_selected_bg_colour);
@@ -344,27 +348,63 @@ impl CanvasColours {
     }
 
     pub fn set_widget_title_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.widget_title_style = get_style_from_config(colour)?;
+        self.widget_title_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_graph_colour(&mut self, colour: &str) -> error::Result<()> {
-        self.graph_style = get_style_from_config(colour)?;
+        self.graph_style = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_high_battery_color(&mut self, colour: &str) -> error::Result<()> {
-        self.high_battery_colour = get_style_from_config(colour)?;
+        self.high_battery_colour = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_medium_battery_color(&mut self, colour: &str) -> error::Result<()> {
-        self.medium_battery_colour = get_style_from_config(colour)?;
+        self.medium_battery_colour = str_to_fg(colour)?;
         Ok(())
     }
 
     pub fn set_low_battery_color(&mut self, colour: &str) -> error::Result<()> {
-        self.low_battery_colour = get_style_from_config(colour)?;
+        self.low_battery_colour = str_to_fg(colour)?;
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+
+    use tui::style::{Color, Style};
+
+    use super::CanvasColours;
+
+    #[test]
+    fn default_selected_colour_works() {
+        let mut colours = CanvasColours::default();
+
+        assert_eq!(
+            colours.currently_selected_text_style,
+            Style::default()
+                .fg(colours.currently_selected_text_colour)
+                .bg(colours.currently_selected_bg_colour),
+        );
+
+        colours.set_scroll_entry_text_color("red").unwrap();
+
+        assert_eq!(
+            colours.currently_selected_text_style,
+            Style::default()
+                .fg(Color::Red)
+                .bg(colours.currently_selected_bg_colour),
+        );
+
+        colours.set_scroll_entry_bg_color("magenta").unwrap();
+
+        assert_eq!(
+            colours.currently_selected_text_style,
+            Style::default().fg(Color::Red).bg(Color::Magenta),
+        );
+    }
+}

src/canvas/canvas_styling/colour_utils.rs

@@ -10,7 +10,7 @@ pub const HIGHLIGHT_COLOUR: Color = Color::LightBlue;
 pub const AVG_COLOUR: Color = Color::Red;
 pub const ALL_COLOUR: Color = Color::Green;
 
-pub fn convert_hex_to_color(hex: &str) -> error::Result<Color> {
+fn convert_hex_to_color(hex: &str) -> error::Result<Color> {
     fn hex_err(hex: &str) -> error::Result<u8> {
         Err(
             error::BottomError::ConfigError(format!(
@@ -47,26 +47,13 @@ pub fn convert_hex_to_color(hex: &str) -> error::Result<Color> {
     Ok(Color::Rgb(rgb.0, rgb.1, rgb.2))
 }
 
-pub fn get_style_from_config(input_val: &str) -> error::Result<Style> {
-    if input_val.len() > 1 {
-        if &input_val[0..1] == "#" {
-            get_style_from_hex(input_val)
-        } else if input_val.contains(',') {
-            get_style_from_rgb(input_val)
-        } else {
-            get_style_from_color_name(input_val)
-        }
-    } else {
-        Err(error::BottomError::ConfigError(format!(
-            "value \"{}\" is not valid.",
-            input_val
-        )))
-    }
+pub fn str_to_fg(input_val: &str) -> error::Result<Style> {
+    Ok(Style::default().fg(str_to_colour(input_val)?))
 }
 
-pub fn get_colour_from_config(input_val: &str) -> error::Result<Color> {
+pub fn str_to_colour(input_val: &str) -> error::Result<Color> {
     if input_val.len() > 1 {
-        if &input_val[0..1] == "#" {
+        if input_val.starts_with('#') {
             convert_hex_to_color(input_val)
         } else if input_val.contains(',') {
             convert_rgb_to_color(input_val)
@@ -81,10 +68,6 @@ pub fn get_colour_from_config(input_val: &str) -> error::Result<Color> {
     }
 }
 
-pub fn get_style_from_hex(hex: &str) -> error::Result<Style> {
-    Ok(Style::default().fg(convert_hex_to_color(hex)?))
-}
-
 fn convert_rgb_to_color(rgb_str: &str) -> error::Result<Color> {
     let rgb_list = rgb_str.split(',').collect::<Vec<&str>>();
     if rgb_list.len() != 3 {
@@ -114,10 +97,6 @@ fn convert_rgb_to_color(rgb_str: &str) -> error::Result<Color> {
     }
 }
 
-pub fn get_style_from_rgb(rgb_str: &str) -> error::Result<Style> {
-    Ok(Style::default().fg(convert_rgb_to_color(rgb_str)?))
-}
-
 fn convert_name_to_color(color_name: &str) -> error::Result<Color> {
     match color_name.to_lowercase().trim() {
         "reset" => Ok(Color::Reset),
@@ -160,10 +139,6 @@ The following are supported strings:
     }
 }
 
-pub fn get_style_from_color_name(color_name: &str) -> error::Result<Style> {
-    Ok(Style::default().fg(convert_name_to_color(color_name)?))
-}
-
 #[cfg(test)]
 mod test {
     use super::*;

src/components/data_table/styling.rs

@@ -1,6 +1,6 @@
 use tui::style::Style;
 
-use crate::canvas::canvas_colours::CanvasColours;
+use crate::canvas::canvas_styling::CanvasColours;
 
 #[derive(Default)]
 pub struct DataTableStyling {

src/options.rs

@@ -18,7 +18,7 @@ use typed_builder::*;
 
 use crate::{
     app::{layout_manager::*, *},
-    canvas::{canvas_colours::CanvasColours, ColourScheme},
+    canvas::{canvas_styling::CanvasColours, ColourScheme},
     constants::*,
     units::data_units::DataUnit,
     utils::error::{self, BottomError},

src/widgets/cpu_graph.rs

@@ -5,7 +5,7 @@ use tui::{style::Style, text::Text, widgets::Row};
 
 use crate::{
     app::{data_harvester::cpu::CpuDataType, AppConfigFields},
-    canvas::{canvas_colours::CanvasColours, Painter},
+    canvas::{canvas_styling::CanvasColours, Painter},
     components::data_table::{
         Column, ColumnHeader, DataTable, DataTableColumn, DataTableProps, DataTableStyling,
         DataToCell,

src/widgets/disk_table.rs

@@ -5,7 +5,7 @@ use tui::text::Text;
 
 use crate::{
     app::AppConfigFields,
-    canvas::canvas_colours::CanvasColours,
+    canvas::canvas_styling::CanvasColours,
     components::data_table::{
         ColumnHeader, DataTableColumn, DataTableProps, DataTableStyling, DataToCell, SortColumn,
         SortDataTable, SortDataTableProps, SortOrder, SortsRow,

src/widgets/process_table.rs

@@ -11,7 +11,7 @@ use crate::{
         query::*,
         AppConfigFields, AppSearchState,
     },
-    canvas::canvas_colours::CanvasColours,
+    canvas::canvas_styling::CanvasColours,
     components::data_table::{
         Column, ColumnHeader, ColumnWidthBounds, DataTable, DataTableColumn, DataTableProps,
         DataTableStyling, SortColumn, SortDataTable, SortDataTableProps, SortOrder, SortsRow,

src/widgets/temperature_table.rs

@@ -6,7 +6,7 @@ use tui::text::Text;
 
 use crate::{
     app::{data_harvester::temperature::TemperatureType, AppConfigFields},
-    canvas::canvas_colours::CanvasColours,
+    canvas::canvas_styling::CanvasColours,
     components::data_table::{
         ColumnHeader, DataTableColumn, DataTableProps, DataTableStyling, DataToCell, SortColumn,
         SortDataTable, SortDataTableProps, SortOrder, SortsRow,