Skip to content

Commit

Permalink
[iOS Enterprise] BrowserSignin: Disable settings entry point
Browse files Browse the repository at this point in the history
Adds logic to the settings view controller to replace the sign-in item
with an info button item that grays out the sign-in option. The info
button's popover menu explains that sign-in is disable by the managed
browser's organization, and provides a link to the chrome://management
page for more information.

As part of this change, the TableViewInfoButtonItem class was modified
to support changing the color of the cell's main text and letting
callers provide a pre-instantiated UIImage. Passing an image rather
than an image name is necessary in the case of sign-in, because the
image must be created by the SigninResourcesProvider.

In addition, the EnterpriseInfoPopoverViewController was modified to
enable one of the disabled constructors, which lets callers provide
the primary text message.

Screenshots of the new UI when sign-in is disabled:
https://screenshot.googleplex.com/38X86W24GXm2Ej5
https://screenshot.googleplex.com/9yee95xBSJ7u3D6

UI mocks:
go/bling-enterprise-q42020 (slide 6)

Bug: 1155745
Change-Id: I099c67b93023fb3f78181bdfeae13dcb73a0e59e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2591388
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Reviewed-by: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838195}
  • Loading branch information
guillaumejenkins authored and Chromium LUCI CQ committed Dec 17, 2020
1 parent a543d1f commit 77847d0
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 26 deletions.
1 change: 1 addition & 0 deletions ios/chrome/browser/ui/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ source_set("settings") {
"//ios/chrome/browser/ui/alert_coordinator",
"//ios/chrome/browser/ui/authentication",
"//ios/chrome/browser/ui/authentication/cells",
"//ios/chrome/browser/ui/authentication/signin:signin_headers",
"//ios/chrome/browser/ui/autofill/cells",
"//ios/chrome/browser/ui/colors",
"//ios/chrome/browser/ui/commands",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@
// Static popover presenting the information of the enterprise.
@interface EnterpriseInfoPopoverViewController : PopoverLabelViewController

- (instancetype)initWithEnterpriseName:(NSString*)enterpriseName
// Initializes the popover with default primary text, and secondary text based
// on the given |enterpriseName|.
- (instancetype)initWithEnterpriseName:(NSString*)enterpriseName;

// Initializes the popover with the given |message| as primary text, and
// secondary text based on the given |enterpriseName|.
- (instancetype)initWithMessage:(NSString*)message
enterpriseName:(NSString*)enterpriseName
NS_DESIGNATED_INITIALIZER;

- (instancetype)initWithMessage:(NSString*)message NS_UNAVAILABLE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@

NSString* const kChromeManagementURL = @"chrome://management";

NSAttributedString* PrimaryMessage() {
NSString* fullText =
l10n_util::GetNSString(IDS_IOS_ENTERPRISE_MANAGED_SETTING_MESSAGE);
NSAttributedString* PrimaryMessage(NSString* fullText) {
DCHECK(fullText);
NSDictionary* generalAttributes = @{
NSForegroundColorAttributeName : [UIColor colorNamed:kTextPrimaryColor],
NSFontAttributeName : [UIFont preferredFontForTextStyle:UIFontTextStyleBody]
Expand Down Expand Up @@ -98,8 +97,15 @@
@implementation EnterpriseInfoPopoverViewController

- (instancetype)initWithEnterpriseName:(NSString*)enterpriseName {
NSString* message =
l10n_util::GetNSString(IDS_IOS_ENTERPRISE_MANAGED_SETTING_MESSAGE);
return [self initWithMessage:message enterpriseName:enterpriseName];
}

- (instancetype)initWithMessage:(NSString*)message
enterpriseName:(NSString*)enterpriseName {
return
[super initWithPrimaryAttributedString:PrimaryMessage()
[super initWithPrimaryAttributedString:PrimaryMessage(message)
secondaryAttributedString:SecondaryMessage(enterpriseName)];
}

Expand Down
80 changes: 68 additions & 12 deletions ios/chrome/browser/ui/settings/settings_table_view_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h"
#import "ios/chrome/browser/ui/authentication/cells/table_view_account_item.h"
#import "ios/chrome/browser/ui/authentication/cells/table_view_signin_promo_item.h"
#import "ios/chrome/browser/ui/authentication/signin/signin_utils.h"
#import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_feature.h"
#import "ios/chrome/browser/ui/settings/about_chrome_table_view_controller.h"
Expand Down Expand Up @@ -396,7 +397,13 @@ - (void)loadModel {

AuthenticationService* authService =
AuthenticationServiceFactory::GetForBrowserState(_browserState);
if (!authService->IsAuthenticated()) {
// If sign-in is disabled by policy, replace the sign-in / account section
// with an info button view item.
if (!signin::IsSigninAllowed(_browserState->GetPrefs())) {
[model addSectionWithIdentifier:SettingsSectionIdentifierSignIn];
[model addItem:[self signinDisabledTextItem]
toSectionWithIdentifier:SettingsSectionIdentifierSignIn];
} else if (!authService->IsAuthenticated()) {
// Sign in section
[model addSectionWithIdentifier:SettingsSectionIdentifierSignIn];
if ([SigninPromoViewMediator
Expand Down Expand Up @@ -558,6 +565,26 @@ - (TableViewItem*)signInTextItem {
return signInTextItem;
}

- (TableViewItem*)signinDisabledTextItem {
TableViewInfoButtonItem* signinDisabledItem = [[TableViewInfoButtonItem alloc]
initWithType:SettingsItemTypeSigninDisabled];
signinDisabledItem.text =
l10n_util::GetNSString(IDS_IOS_SIGN_IN_TO_CHROME_SETTING_TITLE);
signinDisabledItem.detailText =
l10n_util::GetNSString(IDS_IOS_SETTINGS_SIGNIN_DISABLED);
signinDisabledItem.accessibilityHint =
l10n_util::GetNSString(IDS_IOS_TOGGLE_SETTING_MANAGED_ACCESSIBILITY_HINT);
signinDisabledItem.accessibilityIdentifier = kSettingsSignInDisabledCellId;
signinDisabledItem.image =
CircularImageFromImage(ios::GetChromeBrowserProvider()
->GetSigninResourcesProvider()
->GetDefaultAvatar(),
kAccountProfilePhotoDimension);
signinDisabledItem.textColor = [UIColor colorNamed:kTextSecondaryColor];
signinDisabledItem.tintColor = [UIColor colorNamed:kGrey300Color];
return signinDisabledItem;
}

- (TableViewItem*)googleServicesCellItem {
return [self detailItemWithType:SettingsItemTypeGoogleServices
text:l10n_util::GetNSString(
Expand Down Expand Up @@ -643,7 +670,8 @@ - (TableViewInfoButtonItem*)managedSearchEngineItem {
initWithType:SettingsItemTypeManagedDefaultSearchEngine];
managedDefaultSearchEngineItem.text =
l10n_util::GetNSString(IDS_IOS_SEARCH_ENGINE_SETTING_TITLE);
managedDefaultSearchEngineItem.iconImageName = kSettingsSearchEngineImageName;
managedDefaultSearchEngineItem.image =
[UIImage imageNamed:kSettingsSearchEngineImageName];
managedDefaultSearchEngineItem.accessibilityHint =
l10n_util::GetNSString(IDS_IOS_TOGGLE_SETTING_MANAGED_ACCESSIBILITY_HINT);

Expand Down Expand Up @@ -959,6 +987,15 @@ - (UITableViewCell*)tableView:(UITableView*)tableView
forControlEvents:UIControlEventTouchUpInside];
break;
}
case SettingsItemTypeSigninDisabled: {
TableViewInfoButtonCell* managedCell =
base::mac::ObjCCastStrict<TableViewInfoButtonCell>(cell);
[managedCell.trailingButton
addTarget:self
action:@selector(didTapSigninDisabledInfoButton:)
forControlEvents:UIControlEventTouchUpInside];
break;
}
default:
break;
}
Expand Down Expand Up @@ -1097,13 +1134,33 @@ - (void)tableView:(UITableView*)tableView

#pragma mark - Actions

// Called when the user clicks on the information button of a managed
// settings UI cell. Shows a contextual bubble with the information of the
// enterprise.
// Called when the user taps on the information button of a managed setting's UI
// cell.
- (void)didTapSigninDisabledInfoButton:(UIButton*)buttonView {
NSString* popoverMessage =
l10n_util::GetNSString(IDS_IOS_SETTINGS_SIGNIN_DISABLED_POPOVER_TEXT);
EnterpriseInfoPopoverViewController* popover =
[[EnterpriseInfoPopoverViewController alloc]
initWithMessage:popoverMessage
enterpriseName:nil];

[self showEnterprisePopover:popover forInfoButton:buttonView];
}

// Called when the user taps on the information button of the sign-in setting
// while sign-in is disabled by policy.
- (void)didTapManagedUIInfoButton:(UIButton*)buttonView {
EnterpriseInfoPopoverViewController* bubbleViewController =
EnterpriseInfoPopoverViewController* popover =
[[EnterpriseInfoPopoverViewController alloc] initWithEnterpriseName:nil];
bubbleViewController.delegate = self;

[self showEnterprisePopover:popover forInfoButton:buttonView];
}

// Shows a contextual bubble explaining that the tapped setting is managed and
// includes a link to the chrome://management page.
- (void)showEnterprisePopover:(EnterpriseInfoPopoverViewController*)popover
forInfoButton:(UIButton*)buttonView {
popover.delegate = self;

// Disable the button when showing the bubble.
// The button will be enabled when close the bubble in
Expand All @@ -1112,13 +1169,12 @@ - (void)didTapManagedUIInfoButton:(UIButton*)buttonView {
buttonView.enabled = NO;

// Set the anchor and arrow direction of the bubble.
bubbleViewController.popoverPresentationController.sourceView = buttonView;
bubbleViewController.popoverPresentationController.sourceRect =
buttonView.bounds;
bubbleViewController.popoverPresentationController.permittedArrowDirections =
popover.popoverPresentationController.sourceView = buttonView;
popover.popoverPresentationController.sourceRect = buttonView.bounds;
popover.popoverPresentationController.permittedArrowDirections =
UIPopoverArrowDirectionAny;

[self presentViewController:bubbleViewController animated:YES completion:nil];
[self presentViewController:popover animated:YES completion:nil];
}

#pragma mark Switch Actions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ typedef NS_ENUM(NSInteger, SettingsItemType) {
SettingsItemTypeArticlesForYou,
SettingsItemTypeSafetyCheck,
SettingsItemTypeDefaultBrowser,
SettingsItemTypeSigninDisabled,
};

// The accessibility identifier of the settings TableView.
Expand All @@ -54,6 +55,10 @@ extern NSString* const kSettingsTableViewId;
// The accessibility identifier of the sign in cell.
extern NSString* const kSettingsSignInCellId;

// The accessibility identifier of the sign in cell when sign-in is disabled by
// policy.
extern NSString* const kSettingsSignInDisabledCellId;

// The accessibility identifier of the account cell.
extern NSString* const kSettingsAccountCellId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

NSString* const kSettingsTableViewId = @"kSettingsTableViewId";
NSString* const kSettingsSignInCellId = @"kSettingsSignInCellId";
NSString* const kSettingsSignInDisabledCellId =
@"kSettingsSignInDisabledCellId";
NSString* const kSettingsAccountCellId = @"kSettingsAccountCellId";
NSString* const kSettingsSearchEngineCellId = @"kSettingsSearchEngineCellId";
NSString* const kSettingsManagedSearchEngineCellId =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,18 @@

#import "ios/chrome/browser/ui/settings/settings_table_view_controller.h"

#import "base/test/scoped_command_line.h"
#import "base/test/task_environment.h"
#import "components/pref_registry/pref_registry_syncable.h"
#import "components/prefs/pref_service.h"
#import "components/signin/public/base/signin_pref_names.h"
#import "components/sync/driver/mock_sync_service.h"
#import "components/sync_preferences/pref_service_mock_factory.h"
#import "components/sync_preferences/pref_service_syncable.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/chrome_switches.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/prefs/browser_prefs.h"
#import "ios/chrome/browser/search_engines/template_url_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_fake.h"
Expand All @@ -20,7 +28,9 @@
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/settings/cells/settings_image_detail_text_item.h"
#import "ios/chrome/browser/ui/settings/settings_table_view_controller_constants.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_info_button_item.h"
#import "ios/chrome/browser/ui/table_view/chrome_table_view_controller_test.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#import "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/ios_chrome_scoped_testing_local_state.h"
#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity.h"
Expand All @@ -37,6 +47,10 @@

using ::testing::NiceMock;
using ::testing::Return;
using sync_preferences::PrefServiceMockFactory;
using sync_preferences::PrefServiceSyncable;
using user_prefs::PrefRegistrySyncable;
using web::WebTaskEnvironment;

namespace {
std::unique_ptr<KeyedService> CreateMockSyncService(
Expand All @@ -63,6 +77,7 @@ void SetUp() override {
AuthenticationServiceFactory::GetInstance(),
base::BindRepeating(
&AuthenticationServiceFake::CreateAuthenticationService));
builder.SetPrefService(CreatePrefService());
chrome_browser_state_ = builder.Build();

WebStateList* web_state_list = nullptr;
Expand Down Expand Up @@ -91,6 +106,15 @@ void TearDown() override {
ChromeTableViewControllerTest::TearDown();
}

std::unique_ptr<PrefServiceSyncable> CreatePrefService() {
PrefServiceMockFactory factory;
scoped_refptr<PrefRegistrySyncable> registry(new PrefRegistrySyncable);
std::unique_ptr<PrefServiceSyncable> prefs =
factory.CreateSyncable(registry.get());
RegisterBrowserStatePrefs(registry.get());
return prefs;
}

ChromeTableViewController* InstantiateController() override {
return [[SettingsTableViewController alloc]
initWithBrowser:browser_.get()
Expand Down Expand Up @@ -151,3 +175,27 @@ void SetupSyncServiceEnabledExpectations() {
sync_item.detailText,
l10n_util::GetNSString(IDS_IOS_SIGN_IN_TO_CHROME_SETTING_SYNC_ON));
}

// Verifies that the sign-in setting item is replaced by the managed sign-in
// item if sign-in is disabled by policy.
TEST_F(SettingsTableViewControllerTest, SigninDisabled) {
base::test::ScopedCommandLine scoped_command_line;
scoped_command_line.GetProcessCommandLine()->AppendSwitch(
switches::kInstallBrowserSigninHandler);
chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
CreateController();
CheckController();

NSArray* signin_items = [controller().tableViewModel
itemsInSectionWithIdentifier:SettingsSectionIdentifier::
SettingsSectionIdentifierSignIn];
ASSERT_EQ(1U, signin_items.count);

TableViewInfoButtonItem* signin_item =
static_cast<TableViewInfoButtonItem*>(signin_items[0]);
ASSERT_NSEQ(signin_item.text,
l10n_util::GetNSString(IDS_IOS_SIGN_IN_TO_CHROME_SETTING_TITLE));
ASSERT_NSEQ(signin_item.detailText,
l10n_util::GetNSString(IDS_IOS_SETTINGS_SIGNIN_DISABLED));
ASSERT_NE(signin_item.image, nil);
}
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ - (void)loadModel {
initWithType:ItemTypeTableViewInfoButtonWithImage];
tableViewInfoButtonItemWithLeadingImage.text = @"Info button item";
tableViewInfoButtonItemWithLeadingImage.statusText = @"Status";
tableViewInfoButtonItemWithLeadingImage.iconImageName =
@"settings_article_suggestions";
tableViewInfoButtonItemWithLeadingImage.image =
[UIImage imageNamed:@"settings_article_suggestions"];
[model addItem:tableViewInfoButtonItemWithLeadingImage
toSectionWithIdentifier:SectionIdentifierSettings];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
// TableViewInfoButtonItem is a model class that uses TableViewInfoButtonCell.
@interface TableViewInfoButtonItem : TableViewItem

// The filename for the leading icon. If empty, no icon will be shown.
@property(nonatomic, copy) NSString* iconImageName;
// The UIImage for the leading image. If nil, no image will be shown.
@property(nonatomic, strong) UIImage* image;

// Tint color for the icon.
@property(nonatomic, strong) UIColor* tintColor;

// The main text string.
@property(nonatomic, copy) NSString* text;

// The color of the main text.
@property(nonatomic, strong) UIColor* textColor;

// The detail text string.
@property(nonatomic, copy) NSString* detailText;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ - (void)configureCell:(TableViewInfoButtonCell*)cell
withStyler:(ChromeTableViewStyler*)styler {
[super configureCell:cell withStyler:styler];
cell.textLabel.text = self.text;
if (self.textColor) {
cell.textLabel.textColor = self.textColor;
}
if (self.detailText) {
cell.detailTextLabel.text = self.detailText;
[cell updatePaddingForDetailText:YES];
Expand All @@ -39,11 +42,7 @@ - (void)configureCell:(TableViewInfoButtonCell*)cell
cell.selectionStyle = UITableViewCellSelectionStyleNone;

// Update the icon image, if one is present.
UIImage* iconImage = nil;
if ([self.iconImageName length]) {
iconImage = [UIImage imageNamed:self.iconImageName];
}
[cell setIconImage:iconImage withTintColor:self.tintColor];
[cell setIconImage:self.image withTintColor:self.tintColor];
}

@end

0 comments on commit 77847d0

Please sign in to comment.