1114453 - [Wifi] Add wifi setting UI for Allow Off function.

Status: RESOLVED FIXED

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

Description

[Amy Chung [:Amy]]

Refer to the diagram:
https://wiki.mozilla.org/images/c/c6/WifiPowerModeState.svg
AllowOff is the flag that allows wifi off mode when screen off time out and 
unplugged.
We need to set Allow Off option on UX and write gaia setting to connect gaia/system/js/wifi.js.

Comment 1

[Amy Chung [:Amy]]

Added Jenny to focus this issue.

Comment 2

[Jenny Lee]

Hello Amy,

Given the major changes in plans for 2.2 (i.e. that most of UX changes will not be included in scope) and subsequent near-tearm releases, the UX team will evaluate all feature and UI requests after v3 planning, which will have a strong impact on future versions of FXOS. So I won't be able to work on this until things are more settled, but I will definitely think about it and we can have a discussion first. Thanks for understanding!

+Bruce to take a look at this feature request.

Comment 3

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Seinlin,
Please help me to confirm this modification.

Thanks!
Amy

Comment 4

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 8552298 [details]
Added allow off UI on Wifi Setting

Looks good. But need confirm/correct the following:

1. The place of this setting need to be approved by UX team.

2. The string used for 'message-wifi-allowoff-details' and 'wifi-allowoff' need to be confirmed by UX team.

3. A lot of strings in apps/settings/locales/settings.en-US.properties were deleted. I think you need to rebase your patch.

Comment 5

[Amy Chung [:Amy]]

Attachment: wifi_setting.png

Dear Jenny,
Please help me to confirm the wifi setting UI.

Thanks!

Comment 6

[Kai-Zhen Li [:seinlin][:kli]]

Bug 1084156 need this UI.

Comment 7

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Seinlin,
I modified apps/settings/locales/settings.en-US.properties,
please help me to confirm it.

Thanks!

Comment 8

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Comment 9

[Jenny Lee]

Comment on attachment 8552355 [details]
wifi_setting.png

Hi Amy,

Please make change to string to make it clearer:

Wi-Fi Off during Sleep  [toggle]
When enabled, Wi-Fi will be automatically turned off during sleep to save battery power. 

Toggle default is set to off.  
Thanks!

Comment 10

[Amy Chung [:Amy]]

Attachment: wifi_setting.png

Dear Jenny,
Please help me to confirm the UI for selecting wifi sleep mode.

Thanks!

Comment 11

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Seinlin,
I have modified this issue, please help me to review it.

Thanks!

Comment 12

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Jenny Lee from comment #9)
> 
> Toggle default is set to off.  
> Thanks!

Toggle default is set to off ==> wifi will always on during sleep. This will consume more power by default.
i
How do you think set it to on by default (consume less power)?

User can turn off it if necessary.

Comment 13

[Kai-Zhen Li [:seinlin][:kli]]

(In reply to Amy Chung from comment #11)
> Created attachment 8552971 [details] [review]

Amy, I left some comments on PR, could you have a look and update your PR accordingly?

Comment 14

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Seinlin,
I have modified some mistakes in your answer, please help me to confirm it.

Thanks!

Comment 15

[Jenny Lee]

Comment on attachment 8552967 [details]
wifi_setting.png

Looks good, thanks Amy! =)

Comment 16

[Jenny Lee]

Hi Kai-Zhen,

I think under normal circumstances, user would expect Wi-Fi to stay on all the time as an environment with stable Wi-Fi usually comes with power source. So in general saving battery power would have lower priority than getting instant incoming message(or any other type of info) notification. That is to say, I suggest we set the default to off. Thanks!   

(In reply to Kai-Zhen Li [:seinlin] from comment #12)
> (In reply to Jenny Lee from comment #9)
> > 
> > Toggle default is set to off.  
> > Thanks!
> 
> Toggle default is set to off ==> wifi will always on during sleep. This will
> consume more power by default.
> i
> How do you think set it to on by default (consume less power)?
> 
> User can turn off it if necessary.

Comment 17

[Amy Chung [:Amy]]

Hi Seinlin,
I have modified in your answer, please help me to review it.

Thanks!

Comment 18

[howie [:howie]]

If comment 6 Bug 1084156 is for 2.0M, this one should be nominated as 2.0m?

Comment 19

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 8553017 [details] [review]
Added allow off UI on Wifi Setting

Arthur, Could you review this patch? Thanks!

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

Bug 1084156 is not only for v2.0m. It is landed in m-c before v2.2 branch-out, so I think we should also have this in v2.2.

Comment 21

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8553017 [details] [review]
Added allow off UI on Wifi Setting

Good work! Please check my comments in github, thanks.

Comment 22

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Arthur,
I have modified by your suggestions, please help me to review the modification.

Thanks!

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8555075 [details] [review]
Added allow off UI on Wifi Setting

There are still lint errors, please help check them.

Flagging Alive on the system app part.

Comment 24

[howie [:howie]]

Triage: This is not a blocker. Hi Amy, please go through approval process for 2.2 uplift, thanks.

Comment 25

[Amy Chung [:Amy]]

Dear Arthur,
I have modified it, please review this modification.

Thanks!

Comment 26

[Arthur Chen [:arthurcc][inactive after 6/30]]

The indention of <p> is incorrect (line 55). And I noticed that gaia-try fails [1] on wifi tests, so we should modify the test accordingly.

[1]: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=f2a9434fc859

Comment 27

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8555075 [details] [review]
Added allow off UI on Wifi Setting

r+ iff nits addressed

Comment 28

[Amy Chung [:Amy]]

Dear Arthur,
I have modified the mistake of indention on wifi.html and wifi test case.
Please help me to review it.

Thanks!

Comment 29

[Arthur Chen [:arthurcc][inactive after 6/30]]

wifi.html is looking good now, thanks!

Note that there are still lint errors in wifi_test.js and I believe the reason is the same [1]. We are ready to land the patch after all lint errors get fixed.

[1]: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=465003&repo=gaia-try

Comment 30

[Amy Chung [:Amy]]

Comment on attachment 8555075 [details] [review]
Added allow off UI on Wifi Setting

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):Improve wifi power control.
[User impact] if declined: User can use this function to allow wifi into sleep mode.
[Testing completed]: local unit test and try server are all pass.
[Risk to taking this patch] (and alternatives if risky): low, added a item to control wifi into sleep mode by user. 
[String changes made]: apps/settings/locales/settings.en-US.properties
   1. wifi-off-during-sleep=Wi-Fi Off during Sleep.
   2. wifi-off-during-sleep-description=When enabled, Wi-Fi will be automatically turned off during sleep to save battery power.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/eb824cc6e228a86e7e2e8ce3d3817496f91be8ef

Comment 32

[Francesco Lodolo [:flod]]

We should really try to avoid this kind of structure (long label+checkbox), given our inability to span the label on multiple lines. This is going to be extremely painful to localize. 

The case used is also confusing: why "during" lowercase?

Comment 33

[Théo Chevalier]

I agree with flod, we don't have enough space.
To give you an idea, the shortest acceptable translation I can think of in French is "Wi-Fi éteint en mode veille", and that doesn't fit.

Comment 34

[Amy Chung [:Amy]]

Dear Arthur,
What do you think with the problem of long label? Do I need to discuss with UX again and modify this problem?

Thanks!

Comment 35

[Arthur Chen [:arthurcc][inactive after 6/30]]

Amy, we need a shorter name for this feature, or we have to create a new type of UI for this kind of cases.

Jenny, given we have a description of the feature below, is it possible to have a shorter name?

Comment 36

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Kai-Zhen, we already triaged and decided this is a feature and shouldn't block. The patch is also ready with approval flag set? Any reason why you ask for block again?

Comment 37

[Jenny Lee]

Hi Matej,

We need your help with some wording issue. Can you take a look at the toggle description below and see if it's possible to make the title line shorter? Thanks!

Wi-Fi Off during Sleep  [toggle]
When enabled, Wi-Fi will be automatically turned off during sleep to save battery power. 


Hi Arthur,

There's a two line title toggle design, so if there's no way to make title shorter, we can apply that layout =)

Comment 38

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(Reset flag)

Comment 39

[Matej Novak [:matej]]

(In reply to Jenny Lee from comment #37)
> Hi Matej,
> 
> We need your help with some wording issue. Can you take a look at the toggle
> description below and see if it's possible to make the title line shorter?
> Thanks!
> 
> Wi-Fi Off during Sleep  [toggle]
> When enabled, Wi-Fi will be automatically turned off during sleep to save
> battery power. 

I think we could just call it "Wi-Fi sleep" given the longer description below.

Does that work?

Here's also a slightly shorter version of the line:

"Enable to automatically turn off Wi-Fi during sleep to save battery power."

Comment 40

[Kai-Zhen Li [:seinlin][:kli]]

Tim, According to landing rule, https://wiki.mozilla.org/Release_Management/B2G_Landing#v2.2,
landing into v2.2, we need "approval‑mozilla‑b2g37+ / approval-gaia-v2.2+ to land (including bugs marked as blocking-b2g:2.2+ or feature-b2g:2.2+)". So I think we should set this bug as 2.2+

Comment 41

[Arthur Chen [:arthurcc][inactive after 6/30]]

Revert the commit per comment 32.

master: 670317dc9f1ecfab238fd7930eb32b7bac7c0621

Comment 42

[GitHub Autolander Bot]

Attachment: [PullReq] tefn3849:wifi_1114453 to mozilla-b2g:master

Comment 43

[Amy Chung [:Amy]]

Attachment: Added allow off UI on Wifi Setting

Dear Arthur,
Please help me to review this patch for modifying the text wifi off label and checkbox.

Dear Flod,
I have modified the text on wifi off label and checkbox that following comment 39, please help me to confirm it.

Thanks!

Comment 44

[Francesco Lodolo [:flod]]

Comment on attachment 8558978 [details] [review]
Added allow off UI on Wifi Setting

Looks good to me, ++ for having changed the string IDs.

Comment 45

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8558978 [details] [review]
Added allow off UI on Wifi Setting

r=me. Please use upper case for the "s" in "Wi-Fi Sleep", thanks!

Comment 46

[Amy Chung [:Amy]]

Comment on attachment 8558978 [details] [review]
Added allow off UI on Wifi Setting

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):Improve wifi power control.
[User impact] if declined: User can use this function to allow wifi into sleep mode.
[Testing completed]: local unit test and try server are all pass.
[Risk to taking this patch] (and alternatives if risky): low, added a item to control wifi into sleep mode by user. 
[String changes made]: apps/settings/locales/settings.en-US.properties
   1. wifi-off-during-sleep=Wi-Fi Off during Sleep.
   2. wifi-off-during-sleep-description=When enabled, Wi-Fi will be automatically turned off during sleep to save battery power.

Comment 47

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/4d1d218aa1363bdaa0a90871d80b0053881a7a9c

Comment 48

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/cec25f85693e05bd08bd66c25941bb3c0fc59676