Skip to content

Add PercentageAndTime + icon variant for battery - #869

Open
InnocentZero wants to merge 6 commits into
MalpenZibo:mainfrom
InnocentZero:battery_options
Open

Add PercentageAndTime + icon variant for battery#869
InnocentZero wants to merge 6 commits into
MalpenZibo:mainfrom
InnocentZero:battery_options

Conversation

@InnocentZero

Copy link
Copy Markdown

Closes #868

Adds the two formats: PercentageAndTime and IconAndPercentageAndTime.

I'm not super familiar with the codebase, so let me know if things can be done in a better way.

@MalpenZibo MalpenZibo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A visual bug, and we also need to update the documentation

Comment thread src/modules/settings/power.rs Outdated
Signed-off-by: innocentzero <1nn0c3n7z3r0@proton.me>
Signed-off-by: innocentzero <1nn0c3n7z3r0@proton.me>
@InnocentZero

Copy link
Copy Markdown
Author

Updated the docs as well, and fixed the bug

Signed-off-by: innocentzero <1nn0c3n7z3r0@proton.me>
@InnocentZero
InnocentZero requested a review from MalpenZibo August 1, 2026 17:41

@MalpenZibo MalpenZibo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should update the full_config.md example with the new options

Comment thread src/modules/settings/power.rs
Comment thread src/modules/settings/power.rs
Signed-off-by: innocentzero <1nn0c3n7z3r0@proton.me>
Signed-off-by: innocentzero <1nn0c3n7z3r0@proton.me>
@romanstingler

Copy link
Copy Markdown
Collaborator

@MalpenZibo maybe your opinion before we continue

Minor inconsistency: battery_indicator joins percentage+time into one text with a plain space (power.rs:366), while peripheral_indicators uses two text widgets with space.xxs spacing. Not blocking.

battery_time == "100%" || battery_time == battery_capacity is missing in 1 place and
BatteryStatus::Unknown returns "", producing a trailing space in "80% " and an empty text("") widget in the icon variant.

we could add a helper like format_percentage_and_time and reuse the code

  • unrelated to this feature I have tested all the SettingsFormat and quite a bunch were displaying incorrectly also added some fixes for those
diff --git a/src/modules/settings/power.rs b/src/modules/settings/power.rs
index 6262f576..fab5e923 100644
--- a/src/modules/settings/power.rs
+++ b/src/modules/settings/power.rs
@@ -24,8 +24,10 @@ use iced::{
 fn format_time_for_battery(battery: &BatteryData) -> String {
     match battery.status {
         BatteryStatus::Charging(duration) => {
-            if battery.capacity >= 100 || duration.is_zero() {
+            if battery.capacity >= 100 {
                 "100%".to_string()
+            } else if duration.is_zero() {
+                format!("{}%", battery.capacity)
             } else {
                 format_duration(&duration)
             }
@@ -39,12 +41,29 @@ fn format_time_for_battery(battery: &BatteryData) -> String {
                 format_duration(&duration)
             }
         }
-        BatteryStatus::NotCharging => format!("{}%", battery.capacity),
-        BatteryStatus::Unknown => String::new(),
+        BatteryStatus::NotCharging | BatteryStatus::Unknown => {
+            format!("{}%", battery.capacity)
+        }
         BatteryStatus::Full => "100%".to_string(),
     }
 }
 
+fn format_percentage_and_time(battery: &BatteryData) -> String {
+    let capacity = format!("{}%", battery.capacity);
+    let time = match battery.status {
+        BatteryStatus::Charging(duration) | BatteryStatus::Discharging(duration)
+            if battery.capacity < 100 && !duration.is_zero() =>
+        {
+            format_duration(&duration)
+        }
+        BatteryStatus::Discharging(_) if battery.capacity < 100 => {
+            t!("settings-power-calculating")
+        }
+        _ => return capacity,
+    };
+    format!("{capacity} {time}")
+}
+
 #[derive(Debug, Clone)]
 pub enum Message {
     Event(ServiceEvent<UPowerService>),
@@ -267,13 +286,7 @@ impl PowerSettings {
                     SettingsFormat::Icon => {
                         convert::Into::<Element<'a, Message>>::into(icon(p.get_icon_state()))
                     }
-                    SettingsFormat::Percentage => row!(
-                        icon(p.kind.get_icon()),
-                        text(format!("{}%", p.data.capacity))
-                    )
-                    .spacing(space.xxs)
-                    .align_y(Alignment::Center)
-                    .into(),
+                    SettingsFormat::Percentage => text(format!("{}%", p.data.capacity)).into(),
                     SettingsFormat::IconAndPercentage => row!(
                         icon(p.get_icon_state()),
                         text(format!("{}%", p.data.capacity))
@@ -289,29 +302,20 @@ impl PowerSettings {
                     .spacing(space.xxs)
                     .align_y(Alignment::Center)
                     .into(),
-                    SettingsFormat::Name | SettingsFormat::IconAndName => {
-                        convert::Into::<Element<'a, Message>>::into(icon(p.get_icon_state()))
+                    SettingsFormat::Name => text(p.name.to_string()).into(),
+                    SettingsFormat::IconAndName => {
+                        row!(icon(p.get_icon_state()), text(p.name.to_string()))
+                            .spacing(space.xxs)
+                            .align_y(Alignment::Center)
+                            .into()
                     }
-                    SettingsFormat::PercentageAndTime => row!(
-                        text(format!("{}%", p.data.capacity)),
-                        text(format_time_for_battery(&p.data))
-                    )
-                    .spacing(space.xxs)
-                    .align_y(Alignment::Center)
-                    .into(),
-                    SettingsFormat::IconAndPercentageAndTime => {
-                        let battery_capacity = format!("{}%", p.data.capacity);
-                        let battery_time = format_time_for_battery(&p.data);
-                        if battery_time == "100%" || battery_time == battery_capacity {
-                            row!(icon(p.get_icon_state()), text(battery_capacity))
-                        } else {
-                            row!(
-                                icon(p.get_icon_state()),
-                                text(battery_capacity),
-                                text(battery_time)
-                            )
-                        }
+                    SettingsFormat::PercentageAndTime => {
+                        text(format_percentage_and_time(&p.data)).into()
                     }
+                    SettingsFormat::IconAndPercentageAndTime => row!(
+                        icon(p.get_icon_state()),
+                        text(format_percentage_and_time(&p.data))
+                    )
                     .spacing(space.xxs)
                     .align_y(Alignment::Center)
                     .into(),
@@ -358,13 +362,7 @@ impl PowerSettings {
                     }
                     SettingsFormat::PercentageAndTime
                     | SettingsFormat::IconAndPercentageAndTime => {
-                        let battery_time = format_time_for_battery(&battery);
-                        let battery_capacity = format!("{}%", battery.capacity);
-                        if battery_time == "100%" || battery_time == battery_capacity {
-                            battery_capacity
-                        } else {
-                            battery_capacity + " " + &battery_time
-                        }
+                        format_percentage_and_time(&battery)
                     }
                     _ => format!("{}%", battery.capacity),
                 };

@MalpenZibo

Copy link
Copy Markdown
Owner

@MalpenZibo maybe your opinion before we continue

This feature starts to become quite messy 😅
We need to avoid joining things with spaces or adding trailing spaces for sure. A helper that's able to reuse the code and centralize the formatting will be great.

Also, SettingsFormat is an enum with 9 options used for these kinds of things in various places, but each place only uses a subset of the 9 possible values. It's something that we should not resolve here but maybe take into consideration for a future refactor.

@InnocentZero

Copy link
Copy Markdown
Author

I was trying to use existing functions to reduce the code I was writing, but it seems hacky now. I guess a dedicated format_percentage_and_time_for_battery (we can bikeshed on the naming), would be better since there are a bunch of cases on these.

Apart from that, do you want me to add @romanstingler 's changes in this PR itself? I don't mind, but I'll leave it for you to decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add PercentageAndTime/IconAndPercentageAndTime options for battery_format?

3 participants