Closed Bug 798254 Opened 12 years ago Closed 10 years ago

[Settings] Prompt the user when switching on two incompatible settings

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-basecamp:-)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-basecamp -

People

(Reporter: jj.evelyn, Assigned: mancas)

References

Details

(Whiteboard: [qa+])

Attachments

(1 file)

According to current platform implementation, we can't enable USB tethering if USB mass storage or wifi hotspot is enabled. so if we remove this feature for v1, it's easier to identify what user wants when USB is connected. 
Otherwise, we need to present an option dialog, and pass the user's selection to platform. That needs extra effort for both Gaia and Gecko sides.
Chris,

Would you give the green light on this? It's a product's call.
Whiteboard: [blocked-on-input Chris Lee]
Flags: needinfo?(clee)
Needs UX work, assigning to LCO.

Not blocking on this for V1. Likely not a common case.
Assignee: nobody → lco
blocking-basecamp: ? → -
(In reply to Dietrich Ayala (:dietrich) from comment #2)
> Needs UX work, assigning to LCO.
> 
> Not blocking on this for V1. Likely not a common case.

I disagree. User will very likely try to enable everything without knowing the conflicts. The next thing they would do will be start calling customer support complaining something is broken.
blocking-basecamp: - → ?
We shouldn't remove features just because they are in conflict with each other. Is it possible to disable the conflicting setting when the user is connected to USB in a different way?  I don't think we need any messaging.
The title of the bug is misleading. It idea is that one should be disabled in the UI when the other is active.
(In reply to Larissa Co from comment #4)
> We shouldn't remove features just because they are in conflict with each
> other. Is it possible to disable the conflicting setting when the user is
> connected to USB in a different way?  I don't think we need any messaging.

Could you elaborate "in a different way"? There is only one plug and one slot.

(In reply to Dietrich Ayala (:dietrich) from comment #5)
> The title of the bug is misleading.

The title states exactly what we have concluded (before filing the bug), though that might be the ideal solution so if we concluded otherwise here I am more than happy to change it.

> It idea is that one should be disabled in the UI when the other is active.

That's one way to do it if UX thinks it's better this way. As the user turn on one setting, we quietly turn off the other one on another page. However I don't see that results less confusion ... one might still *thinks* he had enabled everything and only found out something not working when he plugged in the USB cable.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #6)
> (In reply to Larissa Co from comment #4)
> > We shouldn't remove features just because they are in conflict with each
> > other. Is it possible to disable the conflicting setting when the user is
> > connected to USB in a different way?  I don't think we need any messaging.
> 
> Could you elaborate "in a different way"? There is only one plug and one
> slot.

I was being intentionally vague so that we're not stuck on one solution. One approach would be:

1. If the user is tethered via USB, the "USB mass storage enabled" setting in Media Storage is greyed out.
2. If the user is already connected as a mass storage device, the "USB Tethering" switch in "Internet Sharing" is greyed out.

The downside of this option is that we don't explain to the user why the setting is greyed out.

Another solution:
1. whenever the user tries to switch a competing setting on, we display a prompt that asks the user whether he wants to switch mode or cancel the operation.
What's the current status if we do nothing today?  It's not clear to me who has priority -- USB mass storage, USB tethering, WiFi hotspot?  Or is it a first come first serve and whoever comes 2nd the function will silently fail until the user turns off the first action.  Is this the case?
Flags: needinfo?(clee)
Whiteboard: [blocked-on-input Chris Lee]
This sounds like a case where it would make sense for the USB selection overlay to be prioritized.

As a suggestion:
When the user plugs their phone into a computer USB port, they should be prompted to select either USB tethering (if enabled through settings), USB Mass storage or Charge only.

- Selecting USB tethering will disable USB mass storage.
- Selecting USB mass storage will disable USB tethering.
- Selecting Charge only will disable both USB mass storage and USB tethering.

Because this is a radio selection, the user should not expect these features to be available concurrently.


This leaves only Wifi hot-spot:

Wifi hot-spot can only be enabled through the Settings app -> Internet sharing -> Wifi-hotspot On/Off.

Maybe it is in here that we need to inform the user that enabling the hot-spot feature will:

- Disable USB tethering.
- Disable USB mass storage
- Disconnect you from any Wifi network you are connected to.
I don't think it even needs to be as complicated as that. What about showing the appropriate prompt when the user is about to do an action that conflicts with another?

For example:
1. The user is tethered via USB
2. The user turns on the "Mass Storage Enabled" setting
3. The user gets a prompt telling him that he must switch from tethering to mass storage.

This design means that only one of these settings can be enabled at a time. There's no way for the user to have the setting for tethering, and mass storage on at the same time.
We need to reach a resolution on this
blocking-basecamp: ? → +
Flags: needinfo?(clee)
Priority: -- → P1
Is everyone happy with my proposal?

:evelyn , does this work for you?
Flags: needinfo?(clee)
Flags: needinfo?(ehung)
Yes. Thanks!
Flags: needinfo?(ehung)
Assignee: lco → ehung
Summary: [Settings] remove USB tethering from Setting App menu → [Settings] Prompt the user when switching on two incompatible settings
+! on lco's proposal
Priority: P1 → --
Not a blocker for shipping, but would consider a low risk fix if found. In that case, please nominate the patch with approval-gaia-master?.
blocking-basecamp: + → -
Component: Gaia → Gaia::Settings
:michal, this is the bug about Settings app that there should be no two conflict USB features enabled, so no dialog is needed.
Triage: please see discussion in bug 809764. We need this blocking+ so we will not need to implement bug 809764.
blocking-basecamp: - → ?
Chris, need your input, same as 809764
Flags: needinfo?(clee)
I think we should do what Larissa suggested in Comment 10. As we removed bug 809764 I believe we should at least warn the user about this.
blocking-basecamp: ? → -
As pointed out during the triage meeting, if we do not implement this we need to agree on the desired behaviour. 

What happens if the user has "USB Mass Storage" active and tries to activate the "USB tethering" or vice versa? This should be agreed at least.
Flags: needinfo?(jcarpenter)
As pointed out during the triage meeting, if we do not implement this we need to agree on the desired behaviour. 

What happens if the user has "USB Mass Storage" active and tries to activate the "USB tethering" or vice versa? This should be agreed at least.
Just thought I'd throw a technical wrinkle into this.

If USB Mass Storage is enabled and the phone is actively being shared with the PC, then you have to wait for UMS to become disabled before you can enable tethering. You can't forcibly disable UMS.

So one possible solution is that while UMS is in the Shared state, that the Enable tethering checkbox could be grayed out and the subtitle text set to something like (Disable UMS to enable tethering).

You could also do something like the reverse if tethering is active.
ok, the point is that we need *some* kind of work to have a decent UX so the sooner we agree on the desired behaviour the less risky will be
> So one possible solution is that while UMS is in the Shared state, that the
> Enable tethering checkbox could be grayed out and the subtitle text set to
> something like (Disable UMS to enable tethering).

That works for me. Larissa, I believe we do something similar for the manual system update button in Device Information when there is no connectivity?
Flags: needinfo?(jcarpenter)
Flags: needinfo?(lco)
(In reply to Josh Carpenter [:jcarpenter] from comment #25)
> > So one possible solution is that while UMS is in the Shared state, that the
> > Enable tethering checkbox could be grayed out and the subtitle text set to
> > something like (Disable UMS to enable tethering).
> 
> That works for me. Larissa, I believe we do something similar for the manual
> system update button in Device Information when there is no connectivity?

Fine with me too.
Fine, hopefully this is a corner case anyway. (Honestly with how interrelated but far apart these two settings are in the settings app, I am tempted to roll them into one setting called "USB", but that's a different conversation)

Can the strings be:

Case 1: mass storage is ON
"Disconnect device from the computer to enable"

Case 2: tethering is ON
"Disable tethering to connect to computer"
Flags: needinfo?(lco)
Flags: needinfo?(clee)
Not sure if our UX still think so because UI has been changed a lot. ni Setting's UX Juwei again, and de-assign myself. I can mentor this bug.
Assignee: ehung → nobody
Flags: needinfo?(jhuang)
Hi Evelyn, Setting's owner is Omega now :) Need info Omega to comment.
Flags: needinfo?(jhuang) → needinfo?(ofeng)
Let's do the following:

We have these 3 USB switches now:
1) Internet Sharing > Wi-Fi Hotspot
2) Internet Sharing > USB Tethering
3) USB Storage

When enabling one of them, if there is aleary one enabled, show a dialog:

%NewFunctionName
-----------
Enabling %NewFunctionName will disable %OldFunctionName. Do you want to enable %NewFunctionName?
[Cancel] [Enable]

for example:

USB Tethering
-----------
Enabling USB Tethering will disable Wi-Fi Hotspot. Do you want to enable USB Tethering?
[Cancel] [Enable]
Flags: needinfo?(ofeng)
Assignee: nobody → b.mcb
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

Flagging Jenny for ui-review and Arthur for code review.
Attachment #8456734 - Flags: ui-review?(jelee)
Attachment #8456734 - Flags: review?(ofeng)
Attachment #8456734 - Flags: review?(arthur.chen)
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

Hello Manuel, please follow the building block dialog style. Thanks!
Example dialog: go to settings>media storage>format SD card> (confirm dialog)
Attachment #8456734 - Flags: ui-review?(jelee) → ui-review-
(In reply to Jenny Lee from comment #33)
> Comment on attachment 8456734 [details] [review]
> Prompt the user if choose two incompatible settings
> 
> Hello Manuel, please follow the building block dialog style. Thanks!
> Example dialog: go to settings>media storage>format SD card> (confirm dialog)

Hi Jenny, please take a look at the new commit. I've changed the UI.

Thanks
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

Manuel, thanks for the patch! The following are my comments:

- Note that USB storage and Wifi hotspot could be enabled at the same time. They are not exclusive.
- I noticed that there are similar functions in the patch. I would suggest to place integrate them into one shared function.

Let me know if you encounter any problem.
Attachment #8456734 - Flags: review?(arthur.chen)
(In reply to Arthur Chen [:arthurcc] from comment #35)
> Comment on attachment 8456734 [details] [review]
> Prompt the user if choose two incompatible settings
> 
> Manuel, thanks for the patch! The following are my comments:
> 
> - Note that USB storage and Wifi hotspot could be enabled at the same time.
> They are not exclusive.
> - I noticed that there are similar functions in the patch. I would suggest
> to place integrate them into one shared function.
> 
> Let me know if you encounter any problem.

Arthur I've made the changes you said. Please take a look at the commit. Thanks
Attachment #8456734 - Flags: ui-review?(jelee)
Attachment #8456734 - Flags: ui-review-
Attachment #8456734 - Flags: review?(arthur.chen)
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

Looks great! Thank you!
Attachment #8456734 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

There are a few minor issues to be addressed before merging. Please check them in github, thanks!
Attachment #8456734 - Flags: review?(arthur.chen)
(In reply to Arthur Chen [:arthurcc] from comment #38)
> Comment on attachment 8456734 [details] [review]
> Prompt the user if choose two incompatible settings
> 
> There are a few minor issues to be addressed before merging. Please check
> them in github, thanks!

Please take a look at the new commit. Thanks!
Attachment #8456734 - Flags: review?(arthur.chen)
Comment on attachment 8456734 [details] [review]
Prompt the user if choose two incompatible settings

r=me, thank you! Please resolve the conflict before merging.
Attachment #8456734 - Flags: review?(arthur.chen) → review+
Keywords: checkin-needed
master: https://github.com/mozilla-b2g/gaia/commit/89b3cd51330ae0ae848057a8d453c252e147e61d
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
I'm honestly torn about the strings landed in this bug. Is this approach going to be used for more conflicting settings? Because it seems very frail (referencing other string IDs as oldSettingL10n and newSettingL10n), and bad for localization with a small gain in terms of strings count.

We have 3 conflicting settings: "USB Storage", "USB Tethering", "Wi-Fi hotspot".

> is-warning-message=Enabling {{newSetting}} will disable {{oldSetting}}. Do you want to enable {{newSetting}}?

We're taking existing localizations out of context and role, and reusing them. I don't have control over grammar case or the fact that these strings will start with an uppercase character, which is wrong is my language (and probably others).

> is-warning-head={{newSetting}}
This doesn't seem useful, since we're not localizing it and it's dynamically set by the code for each function we try to enable.
I confirm what :flod said. In the current state, it's impossible to properly localize this.

To properly localize this in French, for instance, we would need a different translation for "USB Storage", "USB Tethering", "Wi-Fi hotspot" than the current one. And I'm not even sure one translation for all the cases would work for all locales.

For instance for "Wi-Fi hotspot":

The Setting is translated as "Point d’accès Wi-Fi"
In the sentence, it should be "le point d’accès Wi-Fi" (no uppercase + adding a word, that's a new  translation because of context)

Currently we'd have "Activer Point d’accès Wi-Fi désactivera..." witch is wrong and ugly.
Arthur, what's the best way to proceed here?
Flags: needinfo?(arthur.chen)
Thank you for pointing this out. I've filed bug 1046552 tracking this issue.
Flags: needinfo?(arthur.chen)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: