[Settings] Mobile Data settings should never be on in Airplane Mode.

VERIFIED FIXED in B2G C3 (12dec-1jan)

Status

Firefox OS
Gaia::Settings
P3
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: GH to BZ, Assigned: baku)

Tracking

unspecified
B2G C3 (12dec-1jan)

Firefox Tracking Flags

(blocking-basecamp:+)

Details

(Whiteboard: [LOE:S])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
[GitHub issue by evelynhung on 2012-07-30T23:35:32Z, https://github.com/mozilla-b2g/gaia/issues/2985]
(refer issue #2212)
Although Wifi and bluetooth could be turned on in airplane mode, mobile data is always off in backend implementation. However, in Settings app and quick settings, it still could be clicked on, but it actually won't work as user expected. Need UX review on this.
(Reporter)

Comment 1

5 years ago
[GitHub comment by evelynhung on 2012-07-30T23:39:03Z]
I think the button should be greyout in airplane mode
(Reporter)

Comment 2

5 years ago
[GitHub comment by jcarpenter on 2012-07-31T02:43:38Z]
Agreed. Airplane mode should prevent data transmission. We need to communicate to the user that Settings > Network & Connectivity > Cellular & Data > Data Connection toggle is off and cannot be activated when Airplane mode is On.

@lco
(Reporter)

Comment 3

5 years ago
[GitHub comment by lco on 2012-08-01T20:31:55Z]
Fine, we should grey out the data switch if the user is in airplane mode. cc @fabi1cazenave
Do we have a *gray out* state in our template?

Comment 5

5 years ago
Adding Patryk to check on the grey out state. We've done this for a few things in Settings, but I don't know if this was an official visual design. 

Patryk, we'd need a greyed out switch and maybe a color spec for text that's greyed out.

Updated

5 years ago
Flags: needinfo?(padamczyk)
Priority: -- → P1
No we don't have a gray out state currently, but I'll get Peter to add one.
Flags: needinfo?(padamczyk)
(In reply to Patryk Adamczyk [:patryk] UX from comment #6)
> No we don't have a gray out state currently, but I'll get Peter to add one.

Or add one myself on Monday.
Assignee: nobody → ehung

Updated

5 years ago
Priority: P1 → --

Updated

5 years ago
Priority: -- → P3

Updated

5 years ago
Component: Gaia → Gaia::Settings
There's been no activity here for 7 weeks. Evelyn, if you're not planning on getting to it soon, can you re-assign to someone else, or un-assign yourself so someone else can take the bug?
okay, un-assign myself first!
Assignee: ehung → nobody
(Assignee)

Updated

5 years ago
Assignee: nobody → amarchesini
(Assignee)

Comment 10

5 years ago
Created attachment 686634 [details] [diff] [review]
patch

With this patch I remove the data connectivity if the airplane mode is enabled.
Attachment #686634 - Flags: review?(ehung)

Updated

5 years ago
Target Milestone: --- → B2G C3 (12dec-1jan)
Comment on attachment 686634 [details] [diff] [review]
patch

>diff --git a/apps/settings/index.html b/apps/settings/index.html
>index bf3a496..177e33e 100644
>--- a/apps/settings/index.html
>+++ b/apps/settings/index.html
>@@ -487,7 +487,7 @@
>         <h2 data-l10n-id="dataConnectivity"> Data Connectivity </h2>
>       </header>
>       <ul>
>-        <li>
>+        <li id="data-connectivity">
>           <label>
>             <input type="checkbox" data-type="switch" name="ril.data.enabled"/>
>             <span></span>
>diff --git a/apps/settings/js/carrier.js b/apps/settings/js/carrier.js
>index d3b0e6e..99a04e6 100644
>--- a/apps/settings/js/carrier.js
>+++ b/apps/settings/js/carrier.js
>@@ -176,6 +176,26 @@ var Carrier = (function newCarrier(window, document, undefined) {
>     document.getElementById('dataNetwork-desc').textContent = name;
>   }
> 
>+  function updateDataConnectivity() {
>+    var key = 'ril.radio.disabled';
>+
>+    var panel = document.querySelector('#data-connectivity');
nit: 'panel' is a little misleading, it's an element/item. please rename it.
>+    if (!panel)
>+      return;
>+
>+    var settings = Settings.mozSettings;
>+    if (!settings)
>+      return;
>+
>+    var req = settings.createLock().get(key);
>+    req.onsuccess = function() {
>+      panel.hidden = req.result[key];
per comment 3, we intend to *grey-out* this setting but not hide it.
I'm not sure what our visual Patryk provide for grey-out an element, you may need to ask him.
>+    };
>+    settings.addObserver(key, function(evt) {
>+      panel.hidden = evt.settingValue;
same above
>+    });
>+  }
>+
>   // 2G|3G network selection
>   document.getElementById('preferredNetworkType').onchange =
>     restartDataConnection;
>@@ -333,6 +353,7 @@ var Carrier = (function newCarrier(window, document, undefined) {
>     // startup
>     init: function carrier_init() {
>       gMobileConnection.addEventListener('datachange', updateConnection);
>+      updateDataConnectivity();
>       updateConnection();
>       updateSelectionMode();
>       this.fillAPNList(); // XXX this should be done later -- not during init()

r-, sorry, please try to grey-out the setting, do not remove it in airplane mode, it may confuse our users.
Attachment #686634 - Flags: review?(ehung) → review-
Hi Patryk, could you comment for where to find a grey-out visual or example? Thanks!
Flags: needinfo?(padamczyk)
(Assignee)

Comment 13

5 years ago
> r-, sorry, please try to grey-out the setting, do not remove it in airplane
> mode, it may confuse our users.

Sure. I need a CSS class name or an example.
Depends on: 803449
Created attachment 688427 [details]
Spec

In the settings app.
Please just apply:
1. 60% opacity to the icons.
2. Text labels should be #797e80
3. Wifi seems to have a "Disabled" label underneath the "Wi-Fi" section label, please remove it, its inconsistent with the other 3 apps.
4. The toggle switch in the GPS row, should be at 50% opacity.

In the notification tray. Please make the "off" state of the icons 50% opacity. Not that we are updating the icons with bug 803449, should be in this week.
Flags: needinfo?(padamczyk)
Created attachment 688443 [details]
Spec (Updated)


In the settings app.
Please just apply:
1. 60% opacity to the icons.
2. Text labels should be #797e80
3. Wifi seems to have a "Disabled" label underneath the "Wi-Fi" section label, please replace it with "Off" and don't disable that row. Airplane mode should just force wifi off.
4. The toggle switch in the GPS row, should be at 50% opacity.

In the notification tray. Please make the "off" state of the icons 50% opacity. Not that we are updating the icons with bug 803449, should be in this week.
Attachment #688427 - Attachment is obsolete: true
(Assignee)

Comment 16

5 years ago
Wifi, GPS and Bluetooth should not have opacity. Just data connectivity. is it?
(Assignee)

Comment 17

5 years ago
Created attachment 689229 [details] [diff] [review]
patch b
Attachment #686634 - Attachment is obsolete: true
Attachment #689229 - Flags: review?(ehung)

Comment 18

5 years ago
(In reply to Andrea Marchesini (:baku) from comment #16)
> Wifi, GPS and Bluetooth should not have opacity. Just data connectivity. is
> it?

I didn't think  you should be able turn on GPS and Bluetooth on the plane. Please correct me if I'm wrong though.
(Assignee)

Comment 19

5 years ago
you should not... or you should not be able? :)
I think technically it's possible to use GPS/Bluetooth on a plane.
Comment on attachment 689229 [details] [diff] [review]
patch b

r=me. 
Yes, I agree we should only disable data settings here.

This patch resolved Settings app part, but lost System app part.
There is a data setting control in Quick Setting of System app. I think we need another patch to resolve the problem completely.
I'd like to r+ this patch first, and wait your second patch for System app.

Thanks for your great work!
Attachment #689229 - Flags: review?(ehung) → review+
(Assignee)

Comment 21

5 years ago
https://github.com/mozilla-b2g/gaia/pull/6885 pull request
(Assignee)

Comment 22

5 years ago
Created attachment 689700 [details] [diff] [review]
quick settings
Attachment #689700 - Flags: review?(ehung)
(Assignee)

Comment 23

5 years ago
https://github.com/mozilla-b2g/gaia/pull/6919 this is for the first patch.
Comment on attachment 689700 [details] [diff] [review]
quick settings

I'd like to pass the review request to Tim. :)
Attachment #689700 - Flags: review?(ehung) → review?(timdream+bugs)
Comment on attachment 689700 [details] [diff] [review]
quick settings

r=me with nit fixed

>diff --git a/apps/system/js/quick_settings.js b/apps/system/js/quick_settings.js
>index f3e6538..fd696e6 100644
>--- a/apps/system/js/quick_settings.js
>+++ b/apps/system/js/quick_settings.js
>@@ -19,6 +19,16 @@ var QuickSettings = {
> 
>     var self = this;
> 
>+    // nonitor airplane status

// Disable data button if airplane mode is enabled

>+    SettingsListener.observe('ril.radio.disabled', true, function(value) {
>+      self.data.dataset.airplaneMode = value;
>+      if (value) {
>+        self.data.classList.add('quick-settings-airplane-mode');
>+      } else {
>+        self.data.classList.remove('quick-settings-airplane-mode');
>+      }
>+    });
>+
>     // monitor data status
>     SettingsListener.observe('ril.data.enabled', true, function(value) {
>       self.data.dataset.enabled = value;
>@@ -68,16 +78,17 @@ var QuickSettings = {
>             break;
> 
>           case this.data:
>-            var enabled = (this.data.dataset.enabled == 'true');
>-            // the actual mozSettings request is async,
>-            // but we want to be responsive to user input
>-            // and double click so we'll change the UI state here
>-            this.data.dataset.enabled = !enabled;
>-
>-            SettingsListener.getSettingsLock().set({
>-              'ril.data.enabled': !enabled
>-            });
>-
>+            if (this.data.dataset.airplaneMode != 'true') {

if (this.data.dataset.airplaneMode !== 'true')
  return;

>+              var enabled = (this.data.dataset.enabled == 'true');
>+              // the actual mozSettings request is async,
>+              // but we want to be responsive to user input
>+              // and double click so we'll change the UI state here
>+              this.data.dataset.enabled = !enabled;
>+
>+              SettingsListener.getSettingsLock().set({
>+                'ril.data.enabled': !enabled
>+              });
>+            }
>             break;
> 
>           case this.bluetooth:
>diff --git a/apps/system/style/quick_settings/quick_settings.css b/apps/system/style/quick_settings/quick_settings.css
>index 4178595..783488d 100644
>--- a/apps/system/style/quick_settings/quick_settings.css
>+++ b/apps/system/style/quick_settings/quick_settings.css
>@@ -70,3 +70,6 @@
> #quick-settings > #quick-settings-full-app {
>   background-image: url(images/settings.svg);
> }
>+.quick-settings-airplane-mode {
>+  opacity: 0.5;
>+}
Attachment #689700 - Flags: review?(timdream+bugs) → review+
(Assignee)

Comment 26

5 years ago
https://github.com/mozilla-b2g/gaia/pull/6919
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/4f35e5e00656aa7f79eac8ad7243f128117dde58

Merged.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [label:needsUXreview][label:mentored][LOE:S][label:feature] → [LOE:S]

Comment 28

5 years ago
Verified as fixed in build 201301020070202 on Unagi.
Status: RESOLVED → VERIFIED

Comment 29

5 years ago
Build was 20130102070202 on Unagi

Had one to many 0's
You need to log in before you can comment on or make changes to this bug.