Time remaining to full charge is not displayed in battery settings

RESOLVED FIXED in 2.6 S2 - 12/4

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: scabral, Assigned: rakhavan)

Tracking

({foxfood})

unspecified
2.6 S2 - 12/4
ARM
Gonk (Firefox OS)
foxfood

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
reply from mailing list:
This is displayed on the lockscreen though.
This seems odd as we have a report : bug 1177221 time estimate to full charge seams to be off

Comment 1

3 years ago
Yes, it's displayed on the lockscreen. This bug is not about the lockscreen percentage indicator. It's about the inability to see the percentage either in the status bar *or* in the Battery settings pane.
(In reply to Richard Soderberg [:atoll] from comment #1)
> Yes, it's displayed on the lockscreen. This bug is not about the lockscreen
> percentage indicator. It's about the inability to see the percentage either
> in the status bar *or* in the Battery settings pane.

Those are two different components. Let's treat this but as the statusbar one.
Component: General → Gaia::System::Status bar, Utility tray, Notification
Whiteboard: [systemsfe]

Comment 3

3 years ago
I'm told there is an addon for this, in case that affects how this bug is triaged.
(Assignee)

Comment 4

3 years ago
We don't have enough room in the status bar to display the time until fully charged. I do think this should be in the battery settings area though.
Assignee: nobody → rakhavan
Component: Gaia::System::Status bar, Utility tray, Notification → Gaia::Settings
Summary: Time remaining to full charge is not displayed in battery settings nor status bar → Time remaining to full charge is not displayed in battery settings
Created attachment 8685219 [details] [review]
[gaia] jedireza:battery-info-in-settings > mozilla-b2g:master
(Assignee)

Comment 6

3 years ago
Created attachment 8685230 [details]
4 visual states of battery charging discharging time

Comment 7

3 years ago
"3h 17 mins" seems inconsistent, when "3 hours 17 minutes", "3 hr 17 min", or "3h 17m" would each be more consistent.

'charging (showing charge time)' is very clear what the timer indicates: the time until fully charged.

'not charging (showing time remaining)' is not clear what the timer indicates: the time until .. what?
(Assignee)

Comment 8

3 years ago
(In reply to Richard Soderberg [:atoll] from comment #7)
> "3h 17 mins" seems inconsistent, when "3 hours 17 minutes", "3 hr 17 min",
> or "3h 17m" would each be more consistent.

I took the same strings that were on the lock screen. We can easily change these since they're new strings now.

> 'charging (showing charge time)' is very clear what the timer indicates: the
> time until fully charged.
> 'not charging (showing time remaining)' is not clear what the timer
> indicates: the time until .. what?

This is another thing I was going to bring up during review. The BatteryManager API can let us know; 

 - when charging: how long until we're fully charged
 - when discharging (not charging) how long the battery has left until fully discharged (depleted)

You'll see in the screen shots the it says "3h 17 mins remaining".

We should get a UX review for this and the strings in question.

Comment 9

3 years ago
Cool. Thanks for helping with this!
(Assignee)

Comment 10

3 years ago
Comment on attachment 8685219 [details] [review]
[gaia] jedireza:battery-info-in-settings > mozilla-b2g:master

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=b10e904aec801dc95eabe87352ebddf0e43f10a8
Attachment #8685219 - Flags: review?(ehung)
(Assignee)

Comment 11

3 years ago
Comment on attachment 8685230 [details]
4 visual states of battery charging discharging time

Evelyn, do you know who can help us with design and ux reviews?
Flags: needinfo?(ehung)
(Assignee)

Updated

3 years ago
Target Milestone: --- → FxOS-S12 (27Nov)
(Assignee)

Updated

3 years ago
Target Milestone: FxOS-S12 (27Nov) → 2.6 S1 - 11/20

Comment 12

3 years ago
(In reply to Reza Akhavan [:jedireza] from comment #11)
> Comment on attachment 8685230 [details]
> 4 visual states of battery charging discharging time
> 
> Evelyn, do you know who can help us with design and ux reviews?

I'm not sure, but Harly knows. :)
Flags: needinfo?(ehung) → needinfo?(hhsu)

Updated

3 years ago
Attachment #8685219 - Flags: review?(ehung) → review?(gasolin)

Comment 13

3 years ago
NI Tina as she is the UX for Settings
Flags: needinfo?(hhsu) → needinfo?(thsieh)
Hey guys, I agree with Reza that details of battery should stay in settings instead of status bar.

1. For the inconsistency, lets display the time format as "3 hours 17 minutes".

2. For the strings in battery settings: The "remaining" didn't confuse me.
    - when charging: "3 hours 17 minutes until fully charged"
    - when discharging: "3 hours 17 minutes remaining"
Flags: needinfo?(thsieh)

Comment 15

3 years ago
Comment on attachment 8685219 [details] [review]
[gaia] jedireza:battery-info-in-settings > mozilla-b2g:master

Overall looks good, please address UX comment from Tina and set review again. Thanks.
Attachment #8685219 - Flags: review?(gasolin) → feedback+
(Assignee)

Comment 16

3 years ago
Created attachment 8688650 [details]
visual states of battery time

(In reply to Tina Hsieh[:Tina_Hsieh] UX from comment #14)
> 2. For the strings in battery settings: The "remaining" didn't confuse me.
>     - when charging: "3 hours 17 minutes until fully charged"
>     - when discharging: "3 hours 17 minutes remaining"

These strings have been updated. In this new screenshot I'm calling out the visuals of when the text wraps.

Also it takes time (minute(s)) to calculate the time until fully-charged/remaining. During these calculation states we're not showing anything to the user. I feel like we should communicate what's happening during these states.

We could use a generic string for both states like: "calculating battery time"

Or we could be explicit in either case: "calculating time until fully charged" and "calculating time remaining"
Attachment #8685230 - Attachment is obsolete: true
Flags: needinfo?(thsieh)
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Created attachment 8689901 [details]
[Settings] Battery v1.0.pdf

Thanks for pointing out the text format, Reza.
Please see the attached spec. We display time as description format so that strings won't wrap in any of situation.   

Also, I agree with the explicit strings for time calculating. Nice suggestion!:)
Flags: needinfo?(thsieh)
(Assignee)

Updated

3 years ago
Target Milestone: 2.6 S1 - 11/20 → 2.6 S2 - 12/4
(Assignee)

Comment 18

3 years ago
Created attachment 8692251 [details]
visual states of battery time in settings

Thanks for uploading the spec. I've updated the PR and included new screen shots showing the change to the main panel item and the different states of battery time.

There were two states we missed; "Fully charged" and "Fully depleted". Although "Fully depleted" will probably never be seen it's there for posterity's sake.
Attachment #8688650 - Attachment is obsolete: true
Flags: needinfo?(thsieh)

Comment 19

3 years ago
Would "Fully depleted" be shown, albeit briefly, if the phone was charging from a 500mA adapter while making a phone call?

What would be shown if the phone was plugged into an adapter and the battery was removed?
(Assignee)

Comment 20

3 years ago
(In reply to Richard Soderberg [:atoll] from comment #19)
> Would "Fully depleted" be shown, albeit briefly, if the phone was charging
> from a 500mA adapter while making a phone call?
> 
> What would be shown if the phone was plugged into an adapter and the battery
> was removed?

Thanks for mentioning this. Similar thoughts crossed my mind.

We're using the battery wrapper module in the settings app, which supports three states [1] `unplugged`, `charged` and `charging`. We lean on the `unplugged` (and not `unplugged`) state.

With the PR as is, "Fully depleted" would only be shown in the case where the battery module `state` property returns `unplugged` and the `level` property returns `0` (zero).

FWIW, I tried to plugin my Flame and remove the battery, but it never booted into FxOS, it seems to be in a boot loop, showing me the ThunderSoft screen over and over.

[1] https://github.com/mozilla-b2g/gaia/blob/83ab258bea992aa391c86e1ad2c94458b4b30a92/apps/settings/js/modules/battery.js#L66-L72
(Assignee)

Comment 21

3 years ago
Created attachment 8693777 [details]
battery fully depleted

Well it turns out (observed on a Flame) you'll see the "Fully depleted" message.

I booted up a Flame with a drained battery and saw a notification about charging the battery. I quickly went into the battery settings and there I saw the "Fully depleted" message until the phone turned off after about a minute or so.

Just in time to grab a screen shot ;)
Hi Reza,

Thanks for the awesome screen shots! "Fully charged" and "Fully depleted" look good to me.

However, there are some small format issues in those screen shots:
1. Value selectors follow sentence case, so it shall be "Never".
2. The paragraph, which is under the section of Power Save Mode, is suggested to follow the format of description (as those screens in spec). Therefore, the font size shall be smaller and turn to gray color.
Flags: needinfo?(thsieh)
(Assignee)

Comment 23

3 years ago
(In reply to Tina Hsieh[:Tina_Hsieh] UX from comment #22)
> Hi Reza,
> 
> Thanks for the awesome screen shots! "Fully charged" and "Fully depleted"
> look good to me.

:) My pleasure.

> However, there are some small format issues in those screen shots:
> 1. Value selectors follow sentence case, so it shall be "Never".

I updated the PR and changed the case of "Never", since we're only changing the case, I don't think we need to change the identifier.

> 2. The paragraph, which is under the section of Power Save Mode, is
> suggested to follow the format of description (as those screens in spec).
> Therefore, the font size shall be smaller and turn to gray color.

I confirmed that we're using the same pattern as most other settings screens. Specifically, we using an <li class="description"> tag with a <p> tag as a child that contains the content. I could update the CSS for this selector, but it would affect many settings screens. And adding another style just for the battery description doesn't seem like the best move.

Do we want all the settings screen to get this update? Would it be better to create a new bug for this since it's not directly related to this bug?
Flags: needinfo?(thsieh)
Yes, we want all settings screens to follow this update. So it's fine to file a bug for identifying text format updates. I'll deliver another spec of text format guideline for that bug as well.
Flags: needinfo?(thsieh)
(Assignee)

Comment 25

3 years ago
Comment on attachment 8685219 [details] [review]
[gaia] jedireza:battery-info-in-settings > mozilla-b2g:master

(In reply to Tina Hsieh[:Tina_Hsieh] UX from comment #24)
> Yes, we want all settings screens to follow this update. So it's fine to
> file a bug for identifying text format updates. I'll deliver another spec of
> text format guideline for that bug as well.

Great. Bug 1229630 has been created to address the text formatting changes.

And with that, we can move forward with a review for this PR. :)
Attachment #8685219 - Flags: feedback+ → review?(gasolin)

Comment 26

3 years ago
Thanks Reza, the code looks fine.

Though I found the 'Calculating time remaining.../Calculating time required...' message shows for a pretty long time (> 1 min) on flame. Once I kill settings, wait a while and go back to battery panel, it still shows `Calculating...` for 10+ seconds, then it finally shows the remaining time.

If those `Calculating...` message need to be done for a period of time, is it better just remove them and just show the result(X minute remaining) once the calculation is done?
Flags: needinfo?(thsieh)

Comment 27

3 years ago
Comment on attachment 8685219 [details] [review]
[gaia] jedireza:battery-info-in-settings > mozilla-b2g:master

per offline discussion, Tina think we could keep the 'Calculating' messages. 
No other problems.
Attachment #8685219 - Flags: review?(gasolin) → review+

Updated

3 years ago
Flags: needinfo?(thsieh)
(Assignee)

Comment 28

3 years ago
(In reply to Fred Lin [:gasolin] from comment #26)
> Thanks Reza, the code looks fine.
> 
> Though I found the 'Calculating time remaining.../Calculating time
> required...' message shows for a pretty long time (> 1 min) on flame. Once I
> kill settings, wait a while and go back to battery panel, it still shows
> `Calculating...` for 10+ seconds, then it finally shows the remaining time.

Oh I know! It can take a long time to calculate (mentioned in comment #16). I feel confident that showing the user the calculating messages are better than showing nothing.

On master: https://github.com/mozilla-b2g/gaia/commit/1c60dc1a38b04e017d4c3944a3fc6bb28b35dd8e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Please remember to use … instead of ... for consistency with all the other strings in Gaia.

+battery-charging-calculating=Calculating time required...
+battery-discharging-calculating=Calculating time remaining...
Created attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master
(Assignee)

Comment 31

3 years ago
Comment on attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

Francesco, thanks for pointing this out.
Attachment #8694813 - Flags: review?(francesco.lodolo)
Comment on attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

Looks good, thanks.

I noticed another small issue, mostly for future reference: there is no [zero] variant for the plural forms. In general it would be good to always add all available forms, even if they're unused, to avoid potential issues or confusion.

But there are other pieces of older code sharing this issue, so it's not crucial to fix, mostly a nice to have, e.g. https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.en-US.properties#L167-L168
Attachment #8694813 - Flags: review?(francesco.lodolo) → review+
(Assignee)

Comment 33

3 years ago
Comment on attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

Oh I see. While we're here let's fix that too ;) I updated the PR
Attachment #8694813 - Flags: review+ → review?(francesco.lodolo)
Comment on attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

Even better, thanks!
Attachment #8694813 - Flags: review?(francesco.lodolo) → review+
(Assignee)

Comment 35

3 years ago
Comment on attachment 8694813 [details] [review]
[gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

On master: https://github.com/mozilla-b2g/gaia/commit/225cf4d721dafad7ba65feeca27c6676467b0c29
Duplicate of this bug: 1229291

Updated

3 years ago
Duplicate of this bug: 896847
You need to log in before you can comment on or make changes to this bug.