Skip to content

Commit

Permalink
Refactored notification rationale dialog as a bottom sheet
Browse files Browse the repository at this point in the history
Android T+ requires apps to request for notification permission.
Chrome does this currently on first startup, and it shows a rationale
dialog before the OS prompt to educate users of the usefulness of
notifications.

Per UX advice this UI should be displayed as a bottom sheet instead of
a dialog. This CL introduces a bottom sheet that contains the same
UI as the existing rationale dialog. It's behind a flag exposed on
chrome://flags.

It also removes an alternative set of strings that were used when experimenting with this UI.

Bottomsheet screenshot:
https://screenshot.googleplex.com/84j6m6brZBz6uSp
HSV:
https://hsv.googleplex.com/6181362051579904

Bug: 1361424
Change-Id: I70b0d79b71643adc99affeeff264e19d443ce222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3896995
Commit-Queue: Salvador Guerrero Ramos <salg@google.com>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1095798}
  • Loading branch information
Salvador Guerrero authored and Chromium LUCI CQ committed Jan 23, 2023
1 parent 83f0341 commit 0af3699
Show file tree
Hide file tree
Showing 24 changed files with 686 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.night_mode.WebContentsDarkModeMessageController;
import org.chromium.chrome.browser.notifications.permissions.NotificationPermissionController;
import org.chromium.chrome.browser.notifications.permissions.NotificationPermissionController.RationaleDelegate;
import org.chromium.chrome.browser.notifications.permissions.NotificationPermissionRationaleBottomSheet;
import org.chromium.chrome.browser.notifications.permissions.NotificationPermissionRationaleDialogController;
import org.chromium.chrome.browser.ntp.NewTabPageLaunchOrigin;
import org.chromium.chrome.browser.ntp.NewTabPageUtils;
Expand Down Expand Up @@ -676,11 +678,22 @@ mActivity, new SettingsLauncherImpl(),
}

if (!didTriggerPromo) {
mNotificationPermissionController = new NotificationPermissionController(mWindowAndroid,
new NotificationPermissionRationaleDialogController(
mActivity, mModalDialogManagerSupplier.get()));
RationaleDelegate rationaleUIDelegate;

if (NotificationPermissionController.shouldUseBottomSheetRationaleUi()) {
rationaleUIDelegate = new NotificationPermissionRationaleBottomSheet(
mActivity, getBottomSheetController());
} else {
rationaleUIDelegate = new NotificationPermissionRationaleDialogController(
mActivity, mModalDialogManagerSupplier.get());
}

mNotificationPermissionController =
new NotificationPermissionController(mWindowAndroid, rationaleUIDelegate);

NotificationPermissionController.attach(
mWindowAndroid, mNotificationPermissionController);

didTriggerPromo = mNotificationPermissionController.requestPermissionIfNeeded(
false /* contextual */);
}
Expand Down
20 changes: 6 additions & 14 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2169,31 +2169,18 @@ const FeatureEntry::FeatureVariation kFeatureNotificationGuideVariations[] = {
std::size(kFeatureNotificationGuide_default_browser), nullptr},
};

const FeatureEntry::FeatureParam
kNotificationPermissionRationale_show_dialog_next_start_text_variant[] = {
{"always_show_rationale_before_requesting_permission", "true"},
{"notification_permission_dialog_text_variant_2", "true"},
{"permission_request_interval_days", "0"},
};

const FeatureEntry::FeatureParam
kNotificationPermissionRationale_show_dialog_next_start[] = {
{"always_show_rationale_before_requesting_permission", "true"},
{"notification_permission_dialog_text_variant_2", "false"},
{"permission_request_interval_days", "0"},
};

const FeatureEntry::FeatureVariation
kNotificationPermissionRationaleVariations[] = {
{"- Show rationale dialog on next startup",
{"- Show rationale UI on next startup",
kNotificationPermissionRationale_show_dialog_next_start,
std::size(kNotificationPermissionRationale_show_dialog_next_start),
nullptr},
{"- Show rationale dialog on next startup - alternative copy",
kNotificationPermissionRationale_show_dialog_next_start_text_variant,
std::size(
kNotificationPermissionRationale_show_dialog_next_start_text_variant),
nullptr},
};

const FeatureEntry::FeatureParam kWebFeed_accelerator[] = {
Expand Down Expand Up @@ -4446,6 +4433,11 @@ const FeatureEntry kFeatureEntries[] = {
chrome::android::kNotificationPermissionVariant,
kNotificationPermissionRationaleVariations,
"NotificationPermissionVariant")},
{"notification-permission-rationale-bottom-sheet",
flag_descriptions::kNotificationPermissionRationaleBottomSheetName,
flag_descriptions::kNotificationPermissionRationaleBottomSheetDescription,
kOsAndroid,
FEATURE_VALUE_TYPE(chrome::android::kNotificationPermissionBottomSheet)},
{"feature-notification-guide-skip-check-for-low-engaged-users",
flag_descriptions::
kFeatureNotificationGuideSkipCheckForLowEngagedUsersName,
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -4864,10 +4864,15 @@
],
"expiry_milestone": 115
},
{
"name": "notification-permission-rationale-bottom-sheet",
"owners": ["shaktisahu", "salg"],
"expiry_milestone": 114
},
{
"name": "notification-permission-rationale-dialog",
"owners": ["shaktisahu", "salg"],
"expiry_milestone": 110
"expiry_milestone": 114
},
{
"name": "notification-scheduler",
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3750,6 +3750,13 @@ const char kNotificationPermissionRationaleDescription[] =
"Configure the dialog shown before requesting notification permission. "
"Only works with builds targeting Android T.";

const char kNotificationPermissionRationaleBottomSheetName[] =
"Notification Permission Rationale Bottom Sheet UI";
const char kNotificationPermissionRationaleBottomSheetDescription[] =
"Enable the alternative bottom sheet UI for the notification permission "
"flow. "
"Only works with builds targeting Android T+.";

const char kOfflinePagesLivePageSharingName[] =
"Enables live page sharing of offline pages";
const char kOfflinePagesLivePageSharingDescription[] =
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,9 @@ extern const char kNetworkServiceInProcessDescription[];
extern const char kNotificationPermissionRationaleName[];
extern const char kNotificationPermissionRationaleDescription[];

extern const char kNotificationPermissionRationaleBottomSheetName[];
extern const char kNotificationPermissionRationaleBottomSheetDescription[];

extern const char kOfflinePagesLivePageSharingName[];
extern const char kOfflinePagesLivePageSharingDescription[];

Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&kLensCameraAssistedSearch,
&kLensOnQuickActionSearchWidget,
&kNotificationPermissionVariant,
&kNotificationPermissionBottomSheet,
&kPageAnnotationsService,
&kBookmarksImprovedSaveFlow,
&kBookmarksRefresh,
Expand Down Expand Up @@ -725,6 +726,10 @@ BASE_FEATURE(kNotificationPermissionVariant,
"NotificationPermissionVariant",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kNotificationPermissionBottomSheet,
"NotificationPermissionBottomSheet",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kInstanceSwitcher,
"InstanceSwitcher",
base::FEATURE_ENABLED_BY_DEFAULT);
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/flags/android/chrome_feature_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ BASE_DECLARE_FEATURE(kLensCameraAssistedSearch);
BASE_DECLARE_FEATURE(kLensOnQuickActionSearchWidget);
BASE_DECLARE_FEATURE(kLocationBarModelOptimizations);
BASE_DECLARE_FEATURE(kNotificationPermissionVariant);
BASE_DECLARE_FEATURE(kNotificationPermissionBottomSheet);
BASE_DECLARE_FEATURE(kOmahaMinSdkVersionAndroid);
BASE_DECLARE_FEATURE(kOmniboxModernizeVisualUpdate);
BASE_DECLARE_FEATURE(kOptimizeGeolocationHeaderGeneration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
public static final String METRICS_SETTINGS_ANDROID = "MetricsSettingsAndroid";
public static final String MODAL_PERMISSION_DIALOG_VIEW = "ModalPermissionDialogView";
public static final String NOTIFICATION_PERMISSION_VARIANT = "NotificationPermissionVariant";
public static final String NOTIFICATION_PERMISSION_BOTTOM_SHEET =
"NotificationPermissionBottomSheet";
public static final String OFFLINE_PAGES_DESCRIPTIVE_FAIL_STATUS =
"OfflinePagesDescriptiveFailStatus";
public static final String OFFLINE_PAGES_DESCRIPTIVE_PENDING_STATUS =
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/notifications/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ if (is_android) {
"android/java/src/org/chromium/chrome/browser/notifications/channels/SiteChannelsManager.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionChangeReceiver.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionController.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionRationaleBottomSheet.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionRationaleDialogController.java",
]

Expand All @@ -70,6 +71,7 @@ if (is_android) {
"//chrome/browser/offline_pages/android:java",
"//chrome/browser/preferences:java",
"//chrome/browser/profiles/android:java",
"//components/browser_ui/bottomsheet/android:java",
"//components/browser_ui/notifications/android:java",
"//components/browser_ui/settings/android:java",
"//components/browser_ui/site_settings/android:java",
Expand Down Expand Up @@ -112,6 +114,7 @@ if (is_android) {
"android/java/src/org/chromium/chrome/browser/notifications/channels/ChromeChannelDefinitionsTest.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionChangeReceiverTest.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionControllerTest.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionRationaleBottomSheetTest.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/NotificationPermissionRationaleDialogControllerTest.java",
"android/java/src/org/chromium/chrome/browser/notifications/permissions/TestRationaleDelegate.java",
]
Expand All @@ -124,6 +127,7 @@ if (is_android) {
"//chrome/browser/flags:java",
"//chrome/browser/preferences:java",
"//chrome/test/android:chrome_java_unit_test_support",
"//components/browser_ui/bottomsheet/android:java",
"//components/browser_ui/notifications/android:java",
"//components/embedder_support/android:junit_test_support",
"//components/url_formatter/android:url_formatter_java",
Expand Down Expand Up @@ -175,6 +179,7 @@ if (is_android) {
sources = [
"android/java/res/drawable-ldrtl/notification_permission_rationale_dialog_header.xml",
"android/java/res/drawable/notification_permission_rationale_dialog_header.xml",
"android/java/res/layout/notification_permission_rationale_bottom_sheet.xml",
"android/java/res/layout/notification_permission_rationale_dialog.xml",
"android/java/res/values-night/colors.xml",
"android/java/res/values/colors.xml",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center"
android:orientation="vertical">

<ImageView
android:layout_width="250dp"
android:layout_height="0dp"
android:layout_weight="1"
android:layout_marginVertical="24dp"
android:layout_gravity="center_horizontal"
android:importantForAccessibility="no"
android:src="@drawable/notification_permission_rationale_dialog_header"
app:srcCompat="@drawable/notification_permission_rationale_dialog_header" />

<TextView
android:id="@+id/notification_permission_rationale_title"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginHorizontal="16dp"
android:text="@string/notification_permission_rationale_dialog_title"
android:textAppearance="@style/TextAppearance.Headline.Primary" />

<TextView
android:id="@+id/notification_permission_rationale_message"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="12dp"
android:layout_marginHorizontal="16dp"
android:text="@string/notification_permission_rationale_dialog_message"
android:textAppearance="@style/TextAppearance.TextMedium.Primary" />

<org.chromium.ui.widget.ButtonCompat
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:id="@+id/notification_permission_rationale_positive_button"
android:layout_marginTop="28dp"
android:layout_marginHorizontal="16dp"
style="@style/FilledButton.Flat"
android:text="@string/notification_permission_rationale_accept_button_text"/>

<org.chromium.ui.widget.ButtonCompat
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:id="@+id/notification_permission_rationale_negative_button"
android:layout_marginHorizontal="16dp"
style="@style/TextButton"
android:text="@string/notification_permission_rationale_reject_button_text"/>

</LinearLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ public class NotificationUmaTracker {
NotificationRationaleResult.NAVIGATE_BACK_OR_TOUCH_OUTSIDE,
NotificationRationaleResult.NOT_ATTACHED_TO_WINDOW,
NotificationRationaleResult.ACTIVITY_DESTROYED,
NotificationRationaleResult.BOTTOM_SHEET_BACK_PRESS,
NotificationRationaleResult.BOTTOM_SHEET_SWIPE,
NotificationRationaleResult.BOTTOM_SHEET_TAP_SCRIM,
NotificationRationaleResult.BOTTOM_SHEET_FAILED_TO_OPEN,
NotificationRationaleResult.BOTTOM_SHEET_DESTROYED,
NotificationRationaleResult.BOTTOM_SHEET_CLOSED_UNKNOWN,
NotificationRationaleResult.BOTTOM_SHEET_NEVER_OPENED,
NotificationRationaleResult.NUM_ENTRIES})
@Retention(RetentionPolicy.SOURCE)
public @interface NotificationRationaleResult {
Expand All @@ -202,8 +209,15 @@ public class NotificationUmaTracker {
int NAVIGATE_BACK_OR_TOUCH_OUTSIDE = 2;
int ACTIVITY_DESTROYED = 3;
int NOT_ATTACHED_TO_WINDOW = 4;

int NUM_ENTRIES = 5;
int BOTTOM_SHEET_BACK_PRESS = 5;
int BOTTOM_SHEET_SWIPE = 6;
int BOTTOM_SHEET_TAP_SCRIM = 7;
int BOTTOM_SHEET_FAILED_TO_OPEN = 8;
int BOTTOM_SHEET_DESTROYED = 9;
int BOTTOM_SHEET_CLOSED_UNKNOWN = 10;
int BOTTOM_SHEET_NEVER_OPENED = 11;

int NUM_ENTRIES = 12;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ public class NotificationPermissionController implements UnownedUserData {
public static final String FIELD_TRIAL_PERMISSION_REQUEST_MAX_COUNT =
"permission_request_max_count";

/**
* Returns whether the bottom sheet rationale UI should be used.
* @return true if the bottom sheet UI should be used, false if the dialog UI should be used.
*/
public static boolean shouldUseBottomSheetRationaleUi() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.NOTIFICATION_PERMISSION_BOTTOM_SHEET);
}

/** Refers to what type of permission UI should be shown. */
@IntDef({PermissionRequestMode.DO_NOT_REQUEST, PermissionRequestMode.REQUEST_ANDROID_PERMISSION,
PermissionRequestMode.REQUEST_PERMISSION_WITH_RATIONALE})
Expand All @@ -69,13 +77,27 @@ public class NotificationPermissionController implements UnownedUserData {
int REQUEST_PERMISSION_WITH_RATIONALE = 2;
}

/**
* Refers to the result of trying to show the rationale UI.
*/
@IntDef({RationaleUiResult.ACCEPTED, RationaleUiResult.REJECTED, RationaleUiResult.NOT_SHOWN})
public @interface RationaleUiResult {
/** Rationale UI was shown and user accepted. */
int ACCEPTED = 0;
/** Rationale UI was shown and user rejected or dismissed. */
int REJECTED = 1;
/** Rationale UI couldn't be shown. */
int NOT_SHOWN = 2;
}

/** A delegate to show an in-app UI demonstrating rationale behind the permission request. */
interface RationaleDelegate {
public interface RationaleDelegate {
/**
* Called to show the in-app UI.
* @param callback The callback to be invoked as part of the user action on the dialog UI.
* Its argument is a value from {@code RationaleUiResult}.
*/
void showRationaleUi(Callback<Boolean> callback);
void showRationaleUi(Callback<Integer> callback);
}

private static final UnownedUserDataKey<NotificationPermissionController> KEY =
Expand Down Expand Up @@ -153,25 +175,37 @@ public boolean requestPermissionIfNeeded(boolean contextual) {
int requestMode = shouldRequestPermission();
if (requestMode == PermissionRequestMode.DO_NOT_REQUEST) return false;

SharedPreferencesManager.getInstance().incrementInt(
ChromePreferenceKeys.NOTIFICATION_PERMISSION_REQUEST_COUNT);
NotificationUmaTracker.getInstance().onNotificationPermissionRequested();

if (requestMode == PermissionRequestMode.REQUEST_ANDROID_PERMISSION) {
requestAndroidPermission();
recordOsPromptShown();
} else if (requestMode == PermissionRequestMode.REQUEST_PERMISSION_WITH_RATIONALE) {
SharedPreferencesManager.getInstance().writeLong(
ChromePreferenceKeys.NOTIFICATION_PERMISSION_RATIONALE_TIMESTAMP_KEY,
System.currentTimeMillis());
mRationaleDelegate.showRationaleUi(accept -> {
if (accept) {
mRationaleDelegate.showRationaleUi(rationaleResult -> {
if (rationaleResult != RationaleUiResult.NOT_SHOWN) {
recordRationaleUiShown();
}
if (rationaleResult == RationaleUiResult.ACCEPTED) {
requestAndroidPermission();
}
});
}
return true;
}

private void recordOsPromptShown() {
SharedPreferencesManager.getInstance().incrementInt(
ChromePreferenceKeys.NOTIFICATION_PERMISSION_REQUEST_COUNT);
NotificationUmaTracker.getInstance().onNotificationPermissionRequested();
}

private void recordRationaleUiShown() {
SharedPreferencesManager.getInstance().incrementInt(
ChromePreferenceKeys.NOTIFICATION_PERMISSION_REQUEST_COUNT);
NotificationUmaTracker.getInstance().onNotificationPermissionRequested();
SharedPreferencesManager.getInstance().writeLong(
ChromePreferenceKeys.NOTIFICATION_PERMISSION_RATIONALE_TIMESTAMP_KEY,
System.currentTimeMillis());
}

@PermissionRequestMode
int shouldRequestPermission() {
// Notifications only require permission starting at Android T. And apps targeting < T can't
Expand Down

0 comments on commit 0af3699

Please sign in to comment.