1179425 - Time remaining to full charge is not displayed in battery settings

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Sheeri Cabral [:sheeri]]

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

[:Atoll]

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.

Comment 2

[Michael Henretty [:mikehenrty][:mhenretty]]

(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.

Comment 3

[:Atoll]

I'm told there is an addon for this, in case that affects how this bug is triaged.

Comment 4

[Reza Akhavan [:jedireza]]

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.

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] jedireza:battery-info-in-settings > mozilla-b2g:master

Comment 6

[Reza Akhavan [:jedireza]]

Attachment: 4 visual states of battery charging discharging time

Comment 7

[:Atoll]

"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?

Comment 8

[Reza Akhavan [:jedireza]]

(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

[:Atoll]

Cool. Thanks for helping with this!

Comment 10

[Reza Akhavan [:jedireza]]

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

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=b10e904aec801dc95eabe87352ebddf0e43f10a8

Comment 11

[Reza Akhavan [:jedireza]]

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?

Comment 12

[Evelyn Hung]

(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. :)

Comment 13

[Harly Hsu[:harly]]

NI Tina as she is the UX for Settings

Comment 14

[Tina Hsieh[:Tina_Hsieh] UX]

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"

Comment 15

[[e-mail]]

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.

Comment 16

[Reza Akhavan [:jedireza]]

Attachment: 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"

Comment 17

[Tina Hsieh[:Tina_Hsieh] UX]

Attachment: [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!:)

Comment 18

[Reza Akhavan [:jedireza]]

Attachment: 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.

Comment 19

[:Atoll]

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?

Comment 20

[Reza Akhavan [:jedireza]]

(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

Comment 21

[Reza Akhavan [:jedireza]]

Attachment: 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 ;)

Comment 22

[Tina Hsieh[:Tina_Hsieh] UX]

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.

Comment 23

[Reza Akhavan [:jedireza]]

(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?

Comment 24

[Tina Hsieh[:Tina_Hsieh] UX]

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.

Comment 25

[Reza Akhavan [:jedireza]]

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. :)

Comment 26

[[e-mail]]

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?

Comment 27

[[e-mail]]

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.

Comment 28

[Reza Akhavan [:jedireza]]

(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

Comment 29

[Francesco Lodolo [:flod]]

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...

Comment 30

[GitHub Autolander Bot]

Attachment: [gaia] jedireza:settings-strings-fix-1179425 > mozilla-b2g:master

Comment 31

[Reza Akhavan [:jedireza]]

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

Francesco, thanks for pointing this out.

Comment 32

[Francesco Lodolo [:flod]]

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

Comment 33

[Reza Akhavan [:jedireza]]

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

Comment 34

[Francesco Lodolo [:flod]]

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

Even better, thanks!

Comment 35

[Reza Akhavan [:jedireza]]

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