1032651 - [Gaia] CMAS on/off toggle in Settings

Status: RESOLVED FIXED

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

Description

[howie [:howie]]

To implement a CMAS on/off toggle in Settings.

Comment 1

[howie [:howie]]

Hi Arthur, please reassign if needed, thanks.

Comment 2

[[e-mail]]

Hi jenny, could you help provide related UI spec?

Comment 3

[Jenny Lee]

You can find it here: bug 1026685 attachment 8452848 [details] 
Note that all future spec update will be posted in the bug too, thanks!

Comment 4

[[e-mail]]

Attachment: CMAS settings

Thanks jenny!

According to the spec, the requirement is add `Emergency Alert` toggle under `Message Settings` panel > `Cell Broadcast Messages` section.

Comment 5

[[e-mail]]

@Bruce I'd like confirm the dependency between `Cell Broadcast` toggle and `Emergency Alert` toggle.

Does the following rule correct?

1. When Cell Broadcast is disabled, Emergency Alert is disabled as well
2. When Cell Broadcast is enabled, Emergency Alert get's its pref state and can be toggled

Comment 6

[[e-mail]]

got the confirm in bug 1026685 comment 11

1. Two toggles, CB and CMAS
2. When CB on, user can turn on/off the CMAS
3. When CB off, CMAS is also off for sure.

I'd use 'ril.cellbroadcast.cmas.disabled' as mozSettings key in UI if no objection.

Comment 7

[[e-mail]]

Attachment: pull request redirect to github

Implement above requirement, add following rule:

* When CB on, CMAS state will be retained to the state before CB off.
(if CMAS is on before CB off, after CB on, CMAS will be on as well)

with marionette test

Comment 8

[[e-mail]]

Note this patch does not include `SIM settings` panel and the multiple SIMs sub panels described in UI spec

Comment 9

[[e-mail]]

BTW, The wording is corrected by following Title wording rule described in bug 936398

Comment 10

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

Comment on attachment 8464400 [details] [review]
pull request redirect to github

Thanks for the patch. Please check my comments in github.

Comment 11

[[e-mail]]

as bug 1026685 comment 21, The CMAS settings key would be 'cmas.enabled'

Comment 12

[[e-mail]]

Comment on attachment 8464400 [details] [review]
pull request redirect to github

The update includes

- Add CMAS disabled when CB off rule.
- Use asyncStorage to remember last CMAS state when CB off
- add marionette test for CB/CMAS dependency

Comment 13

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8464400 [details] [review]
pull request redirect to github

Hi Fred,

please check my comments on Github ! Thanks :)

Comment 14

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

Comment on attachment 8464400 [details] [review]
pull request redirect to github

Please check my comments in github, thanks.

Comment 15

[[e-mail]]

Comment on attachment 8464400 [details] [review]
pull request redirect to github

The logic is revised to make CMAS totally count on CBS, which lead to simpler code \o/

please kindly review it again

Comment 16

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

Comment on attachment 8464400 [details] [review]
pull request redirect to github

There is one last issue to be addressed before merging. AsyncStorage supports storing arrays, please check my github comments.

Comment 17

[[e-mail]]

Comment on attachment 8464400 [details] [review]
pull request redirect to github

changed to write array directly into asyncStorage, please kindly review it again

Comment 18

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

Comment on attachment 8464400 [details] [review]
pull request redirect to github

r=me with the nits addressed. Please squash the commits before merging, thanks!

Comment 19

[[e-mail]]

TBPL test passed,
merged to gaia master https://github.com/mozilla-b2g/gaia/commit/aab6b29f304d4bea7fd6e03dc661f86f9a8d4e9b

thanks!