New issue
Advanced search Search tips

Issue 865476 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Stop drawing UISwitch in "off" state when disabled

Project Member Reported by mahmadi@chromium.org, Jul 19

Issue description

CollectionViewSwitchItem and SettingsSwitchItem are drawn in "off" state when disabled. is inconsistent with how the SyncSwitchItem behaves in the Sync Settings page as well as the other platforms. It can also cause confusion in the redesigned Autofill Settings page where turning the
master toggle on/off enables/disables the addresses and credit cards sub toggles.

 
disabled.png
171 KB View Download
enabled.png
178 KB View Download
Could we make this an option on the item?  That way we can enable the new behavior for autofill switches without changing it for existing cells.  Something like turnsSwitchOffWhenDisabled (defaults to YES).
Cc: kkhorimoto@chromium.org mahmadi@chromium.org
Owner: sczs@chromium.org
Status: Assigned (was: Untriaged)
I'm worried that would create even more inconsistency.
If we fail to dig out the original reason why this decision was made, maybe we can land  crrev.com/c/1143591 after M69 branch point to let it bake in Canary for a while to catch unforeseen consequences.
https://chromereviews.googleplex.com/471037015

Somewhat arbitrary decision originally, but that's what the existing behavior was at the time.

Voicesearch is a case that would be affected by this change.  Right now if you change your voicesearch language to something that doesn't support TTS (Afrikaans), we visibly animate the switch to off and disable it.  That's a clear indication that your newly chosen language doesn't support TTS.  Simply disabling the switch while leaving it visibly drawing as on would imply that TTS were still active.

Voicesearch might be a special case, in which case we can change the shared item and handle this in the voicesearch code.  But I think we should audit the current uses and make sure there aren't any others.
Did an audit of where CollectionViewSwitchCell and SettingsSwitchCell are used.

Credit card unmask prompt: OK
Payment request card editor: OK
Google app launcher: OK
Settings page: OK
Translate settings page: OK
Privacy settings page: OK
Handoff settings: OK
Autofill settings: OK (switch may be disabled. It doesn't need to be turned off when disabled)
Password settings: OK (switch may be disabled. It doesn't need to be turned off when disabled)
Voice search: OK (switch may be disabled. It is turned off in the code when disabled. Proposed change will not affect this behavior)
Block popups settings: OK
Do not track settings: OK
Compose email handler: OK
Cc: -mahmadi@chromium.org sczs@chromium.org
Owner: mahmadi@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10

commit a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Mon Jul 30 21:08:38 2018

[IOS] Stop drawing UISwitch in "off" state when disabled

Exisitng behavior is inconsistent with the Sync Settings switches and can
caused confusion in the redesigned Autofill Settings where turning the
master on/off enables/disables the addresses and credit cards toggles.

Bug:865476

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I6d2afcb0a24de95b4eb1141ec42699f18733bfe3
Reviewed-on: https://chromium-review.googlesource.com/1143591
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579169}
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/collection_view/cells/collection_view_switch_item.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/collection_view/cells/collection_view_switch_item_unittest.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/settings/autofill_settings_egtest.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/settings/cells/settings_switch_item.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/settings/passwords_settings_egtest.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/settings/voicesearch_collection_view_controller.mm
[modify] https://crrev.com/a927f2f05e9fd20d14fda9e88d1b8daa0c2f3e10/ios/chrome/browser/ui/settings/voicesearch_collection_view_controller_unittest.mm

Status: Fixed (was: Started)

Sign in to add a comment