Skip to content

Commit

Permalink
personalization: rename ambient UI flag to Personalization Jelly
Browse files Browse the repository at this point in the history
Rename the feature flag and update its use cases.

BUG=b:266014907
TEST=Check os://flags, toggle the flag on/off and check screensaver UI

Change-Id: I05fb513f0b89c8346b82624bbb965d2adbc5c780
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205050
Commit-Queue: Jerry Liu <pzliu@google.com>
Reviewed-by: Jeffrey Young <cowmoo@google.com>
Cr-Commit-Position: refs/heads/main@{#1099098}
  • Loading branch information
lpzjerry authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent ed852d0 commit 14ec85e
Show file tree
Hide file tree
Showing 18 changed files with 63 additions and 63 deletions.
18 changes: 10 additions & 8 deletions ash/constants/ash_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,6 @@ BASE_FEATURE(kAmbientModeThrottleAnimation,
"ChromeOSAmbientModeThrottleAnimation",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kAmbientSubpageUIChange,
"AmbientSubpageUIChange",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kApnRevamp, "ApnRevamp", base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kAppCollectionFolderRefresh,
Expand Down Expand Up @@ -1560,6 +1556,11 @@ BASE_FEATURE(kPcieBillboardNotification,
// currently active desk.
BASE_FEATURE(kPerDeskShelf, "PerDeskShelf", base::FEATURE_DISABLED_BY_DEFAULT);

// Enables Jelly features in Personalization App.
BASE_FEATURE(kPersonalizationJelly,
"PersonalizationJelly",
base::FEATURE_DISABLED_BY_DEFAULT);

// Provides a UI for users to view information about their Android phone
// and perform phone-side actions within ChromeOS.
BASE_FEATURE(kPhoneHub, "PhoneHub", base::FEATURE_ENABLED_BY_DEFAULT);
Expand Down Expand Up @@ -2304,10 +2305,6 @@ bool IsAmbientModeThrottleAnimationEnabled() {
return base::FeatureList::IsEnabled(kAmbientModeThrottleAnimation);
}

bool IsAmbientSubpageUIChangeEnabled() {
return base::FeatureList::IsEnabled(kAmbientSubpageUIChange);
}

bool IsApnRevampEnabled() {
return base::FeatureList::IsEnabled(kApnRevamp);
}
Expand Down Expand Up @@ -2908,6 +2905,11 @@ bool IsPerDeskShelfEnabled() {
return base::FeatureList::IsEnabled(kPerDeskShelf);
}

bool IsPersonalizationJellyEnabled() {
return base::FeatureList::IsEnabled(kPersonalizationJelly) &&
IsJellyEnabled();
}

bool IsPhoneHubCameraRollEnabled() {
return base::FeatureList::IsEnabled(kPhoneHubCameraRoll);
}
Expand Down
5 changes: 2 additions & 3 deletions ash/constants/ash_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kAmbientModePhotoPreviewFeature);
COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kAmbientModeThrottleAnimation);
COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kAmbientSubpageUIChange);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kApnRevamp);
COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kAppCollectionFolderRefresh);
Expand Down Expand Up @@ -455,6 +453,7 @@ COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kPcieBillboardNotification);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPerDeskShelf);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPerUserMetrics);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPersonalizationJelly);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPhoneHub);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPhoneHubCameraRoll);
COMPONENT_EXPORT(ASH_CONSTANTS)
Expand Down Expand Up @@ -640,7 +639,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAmbientModeDevUseProdEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAmbientModeEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAmbientModePhotoPreviewEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAmbientModeThrottleAnimationEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAmbientSubpageUIChangeEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsApnRevampEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAppCollectionFolderRefreshEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsAppNotificationsPageEnabled();
Expand Down Expand Up @@ -840,6 +838,7 @@ bool IsProjectorFoldShortGapIntoPreviousTranscriptEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsQsRevampEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsQuickDimEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsPerDeskZOrderEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsPersonalizationJellyEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsReleaseTrackUiEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsReverseScrollGesturesEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsRgbKeyboardEnabled();
Expand Down
4 changes: 2 additions & 2 deletions ash/webui/personalization_app/personalization_app_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ void PersonalizationAppUI::AddBooleans(content::WebUIDataSource* source) {
source->AddBoolean("isScreenSaverPreviewEnabled",
features::IsScreenSaverPreviewEnabled());

source->AddBoolean("isAmbientSubpageUiChangeEnabled",
features::IsAmbientSubpageUIChangeEnabled());
source->AddBoolean("isPersonalizationJellyEnabled",
features::IsPersonalizationJellyEnabled());

// TODO(b/258838122): update when the screen saver policy code is ready.
source->AddBoolean("isAmbientModeManaged", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ export class AmbientPreviewBase extends WithPersonalizationStore {
type: Array,
value: null,
},
isAmbientSubpageUiChangeEnabled_: {
isPersonalizationJellyEnabled_: {
type: Boolean,
value() {
return loadTimeData.getBoolean('isAmbientSubpageUiChangeEnabled');
return loadTimeData.getBoolean('isPersonalizationJellyEnabled');
},
},
isAmbientModeManaged_: {
Expand All @@ -75,7 +75,7 @@ export class AmbientPreviewBase extends WithPersonalizationStore {

protected ambientModeEnabled_: boolean|null;
protected googlePhotosAlbumsPreviews_: Url[]|null;
protected isAmbientSubpageUiChangeEnabled_: boolean;
protected isPersonalizationJellyEnabled_: boolean;
protected previewAlbums_: AmbientModeAlbum[]|null;
protected topicSource_: TopicSource|null;

Expand Down Expand Up @@ -149,7 +149,7 @@ export class AmbientPreviewBase extends WithPersonalizationStore {

/* TODO(b/253470553): Remove this condition after Ambient subpage UI change
* is released. */
if (!this.isAmbientSubpageUiChangeEnabled_) {
if (!this.isPersonalizationJellyEnabled_) {
classes.push('pre-ui-change');
}
return classes.join(' ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,15 @@ <h2>$i18n{screensaverLabel}</h2>
</template>
<template is="dom-if" if="[[loading_]]">
<div id="imagePlaceholder" class="placeholder"></div>
<template is="dom-if" if="[[isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[isPersonalizationJellyEnabled_]]">
<div id="thumbnailPlaceholder" class="placeholder"></div>
</template>
<div id="textPlaceholder"
class="preview-text-placeholder album-info-mainpage">
<div class="placeholder"></div>
<div class="placeholder"></div>
</div>
<template is="dom-if" if="[[!isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[!isPersonalizationJellyEnabled_]]">
<div id="collagePlaceholder" class="placeholder"></div>
</template>
</template>
Expand All @@ -335,7 +335,7 @@ <h2>$i18n{screensaverLabel}</h2>
<img class="preview-image disabled"
src="//personalization/images/slideshow.png">
</div>
<template is="dom-if" if="[[!isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[!isPersonalizationJellyEnabled_]]">
<div id="messageContainer" class="pre-ui-change">
<span class="text" id="turnOnDescription">
$i18n{ambientModeMainPageZeroStateMessage}
Expand All @@ -346,7 +346,7 @@ <h2>$i18n{screensaverLabel}</h2>
</cr-button>
</div>
</template>
<template is="dom-if" if="[[isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[isPersonalizationJellyEnabled_]]">
<div id="messageContainer">
<ambient-zero-state-svg></ambient-zero-state-svg>
<template is="dom-if" if="[[!isAmbientModeManaged_]]">
Expand Down Expand Up @@ -384,7 +384,7 @@ <h2>$i18n{screensaverLabel}</h2>
on-click="onClickPreviewImage_"
on-keypress="onClickPreviewImage_">
</div>
<template is="dom-if" if="[[isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[isPersonalizationJellyEnabled_]]">
<div id="thumbnailContainer" aria-hidden="true"
class$="[[getThumbnailContainerClass_(collageImages_)]]"
on-click="onClickPhotoCollage_"
Expand Down Expand Up @@ -418,7 +418,7 @@ <h2>$i18n{screensaverLabel}</h2>
</span>
</h3>
</template>
<template is="dom-if" if="[[!isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[!isPersonalizationJellyEnabled_]]">
<h3 id="textContainer"
class="preview-text-container album-info-mainpage pre-ui-change"
aria-label$="[[getPreviewTextAriaLabel_(firstPreviewAlbum_, topicSource_, previewAlbums_)]]">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ export class AmbientPreviewLarge extends AmbientPreviewBase {
* - if `previewAlbums_` contains fewer than 4 albums, return one of
* their previews; otherwise return the first 4.
*
* If isAmbientSubpageUiChangeEnabled flag is on, max number of collage image
* If isPersonalizationJellyEnabled flag is on, max number of collage image
* will be 3 instead of 4.
*/
private computeCollageImages_(): Url[] {
const maxLength = this.isAmbientSubpageUiChangeEnabled_ ? 3 : 4;
const maxLength = this.isPersonalizationJellyEnabled_ ? 3 : 4;
switch (this.topicSource_) {
case TopicSource.kArtGallery:
return (this.previewAlbums_ || [])
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<style include="common cros-button-style">
/* TODO(b/253470553): Remove after Ambient subpage UI change is released. */
#container.pre-ui-change {
border: none;
display: grid;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
}
</style>
<div id="container">
<template is="dom-if" if="[[isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[isPersonalizationJellyEnabled_]]">
<ambient-preview-small></ambient-preview-small>
</template>
<!-- restamp to avoid layout issues with iron-list resizing while hidden -->
Expand All @@ -116,7 +116,7 @@
</template>
<template is="dom-if" if="[[ambientModeEnabled_]]">
<!-- TODO(b/253470553): Remove after Ambient subpage UI change is released. -->
<template is="dom-if" if="[[!isAmbientSubpageUiChangeEnabled_]]">
<template is="dom-if" if="[[!isPersonalizationJellyEnabled_]]">
<ambient-preview-small class="pre-ui-change"></ambient-preview-small>
</template>
<template is="dom-if" if="[[loadingSettings_]]">
Expand Down Expand Up @@ -172,7 +172,7 @@ <h3 id="weatherTitle" class="ambient-subpage-element-title" aria-hidden="true">
</ambient-weather-unit>
</template>
</template>
<template is="dom-if" if="[[shouldShowZeroState_(ambientModeEnabled_, isAmbientSubpageUiChangeEnabled_)]]">
<template is="dom-if" if="[[shouldShowZeroState_(ambientModeEnabled_, isPersonalizationJellyEnabled_)]]">
<ambient-zero-state id="zeroState"></ambient-zero-state>
</template>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ export class AmbientSubpage extends WithPersonalizationStore {
computed:
'computeLoadingSettings_(albums_, temperatureUnit_, topicSource_)',
},
isAmbientSubpageUiChangeEnabled_: {
isPersonalizationJellyEnabled_: {
type: Boolean,
value() {
return loadTimeData.getBoolean('isAmbientSubpageUiChangeEnabled');
return loadTimeData.getBoolean('isPersonalizationJellyEnabled');
},
},
};
Expand All @@ -84,7 +84,7 @@ export class AmbientSubpage extends WithPersonalizationStore {
private animationTheme_: AnimationTheme|null;
private temperatureUnit_: TemperatureUnit|null;
private topicSource_: TopicSource|null;
private isAmbientSubpageUiChangeEnabled_: boolean;
private isPersonalizationJellyEnabled_: boolean;

// Refetch albums if the user is currently viewing ambient subpage, focuses
// another window, and then re-focuses personalization app.
Expand Down Expand Up @@ -194,7 +194,7 @@ export class AmbientSubpage extends WithPersonalizationStore {
private shouldShowZeroState_(): boolean {
// TODO(b/253470693): Remove after Ambient subpage UI change is released.
return this.ambientModeEnabled_ === false &&
!this.isAmbientSubpageUiChangeEnabled_;
!this.isPersonalizationJellyEnabled_;
}

private isLoadingAmbientMode_(): boolean {
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3752,10 +3752,9 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kDarkLightModeKMeansColorName,
flag_descriptions::kDarkLightModeKMeansColorDescription, kOsCrOS,
FEATURE_VALUE_TYPE(ash::features::kDarkLightModeKMeansColor)},
{"ambient-subpage-ui-change",
flag_descriptions::kAmbientSubpageUIChangeName,
flag_descriptions::kAmbientSubpageUIChangeDescription, kOsCrOS,
FEATURE_VALUE_TYPE(ash::features::kAmbientSubpageUIChange)},
{"personalization-jelly", flag_descriptions::kPersonalizationJellyName,
flag_descriptions::kPersonalizationJellyDescription, kOsCrOS,
FEATURE_VALUE_TYPE(ash::features::kPersonalizationJelly)},
{"screen-saver-preview", flag_descriptions::kScreenSaverPreviewName,
flag_descriptions::kScreenSaverPreviewDescription, kOsCrOS,
FEATURE_VALUE_TYPE(ash::features::kScreenSaverPreview)},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,11 @@ void PersonalizationAppAmbientProviderImpl::MaybeUpdateTopicSource(

void PersonalizationAppAmbientProviderImpl::FetchGooglePhotosAlbumsPreviews(
const std::vector<std::string>& album_ids) {
const int num_previews = features::IsAmbientSubpageUIChangeEnabled() ? 3 : 4;
const int num_previews = features::IsPersonalizationJellyEnabled() ? 3 : 4;
const int preview_width =
features::IsAmbientSubpageUIChangeEnabled() ? 360 : kBannerWidthPx;
features::IsPersonalizationJellyEnabled() ? 360 : kBannerWidthPx;
const int preview_height =
features::IsAmbientSubpageUIChangeEnabled() ? 130 : kBannerHeightPx;
features::IsPersonalizationJellyEnabled() ? 130 : kBannerHeightPx;
DCHECK(!album_ids.empty());
google_photos_albums_previews_weak_factory_.InvalidateWeakPtrs();
ash::AmbientBackendController::Get()->GetGooglePhotosAlbumsPreview(
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,6 @@
// be removed.
"expiry_milestone": -1
},
{
"name": "ambient-subpage-ui-change",
"owners": [ "assistive-eng@google.com" ],
"expiry_milestone": 120
},
{
"name": "android-force-app-language-prompt",
"owners": [ "perrier", "chrome-language@google.com" ],
Expand Down Expand Up @@ -5684,6 +5679,11 @@
"owners": [ "elklm", "engedy"],
"expiry_milestone": 110
},
{
"name": "personalization-jelly",
"owners": ["assistive-eng@google.com" ],
"expiry_milestone": 120
},
{
"name": "phone-hub-nudge",
"owners": [
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4653,11 +4653,9 @@ const char kAlwaysEnableHdcpDefault[] = "Default";
const char kAlwaysEnableHdcpType0[] = "Type 0";
const char kAlwaysEnableHdcpType1[] = "Type 1";

const char kAmbientSubpageUIChangeName[] =
"Show the revised UI for ambient mode subpage in Personalization Hub";
const char kAmbientSubpageUIChangeDescription[] =
"Implement the new UI design for the ambient mode subpage in "
"Personalization Hub.";
const char kPersonalizationJellyName[] = "Jelly design for Personalization App";
const char kPersonalizationJellyDescription[] =
"Feature to enable the Jelly design in Personalization App.";

const char kAmbientModeThrottleAnimationName[] =
"Throttle the frame rate of Lottie animations in ambient mode";
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -2677,9 +2677,6 @@ extern const char kAlwaysEnableHdcpType1[];
extern const char kAmbientModeThrottleAnimationName[];
extern const char kAmbientModeThrottleAnimationDescription[];

extern const char kAmbientSubpageUIChangeName[];
extern const char kAmbientSubpageUIChangeDescription[];

extern const char kApnRevampName[];
extern const char kApnRevampDescription[];

Expand Down Expand Up @@ -3302,6 +3299,9 @@ extern const char kPcieBillboardNotificationDescription[];
extern const char kPerformantSplitViewResizing[];
extern const char kPerformantSplitViewResizingDescription[];

extern const char kPersonalizationJellyName[];
extern const char kPersonalizationJellyDescription[];

extern const char kPhoneHubCallNotificationName[];
extern const char kPhoneHubCallNotificationDescription[];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ suite('AmbientPreviewLargeTest', function() {

test(
'displays zero state message when ambient mode is disabled', async () => {
loadTimeData.overrideValues({isPersonalizationJellyEnabled: true});
personalizationStore.data.ambient.albums = ambientProvider.albums;
personalizationStore.data.ambient.topicSource = TopicSource.kArtGallery;
personalizationStore.data.ambient.ambientModeEnabled = false;
Expand Down Expand Up @@ -135,8 +136,8 @@ suite('AmbientPreviewLargeTest', function() {
});

test('click ambient collage goes to ambient albums subpage', async () => {
// Disables `isAmbientSubpageUiChangeEnabled` to show the previous UI.
loadTimeData.overrideValues({isAmbientSubpageUiChangeEnabled: false});
// Disables `isPersonalizationJellyEnabled` to show the previous UI.
loadTimeData.overrideValues({isPersonalizationJellyEnabled: false});

personalizationStore.data.ambient = {
...personalizationStore.data.ambient,
Expand Down Expand Up @@ -189,7 +190,7 @@ suite('AmbientPreviewLargeTest', function() {
});

test('click ambient thumbnail goes to ambient albums subpage', async () => {
loadTimeData.overrideValues({isAmbientSubpageUiChangeEnabled: true});
loadTimeData.overrideValues({isPersonalizationJellyEnabled: true});

personalizationStore.data.ambient = {
...personalizationStore.data.ambient,
Expand Down Expand Up @@ -242,8 +243,8 @@ suite('AmbientPreviewLargeTest', function() {
});

test('displays zero state message before UI change', async () => {
// Disables `isAmbientSubpageUiChangeEnabled` to show the previous UI.
loadTimeData.overrideValues({isAmbientSubpageUiChangeEnabled: false});
// Disables `isPersonalizationJellyEnabled` to show the previous UI.
loadTimeData.overrideValues({isPersonalizationJellyEnabled: false});

personalizationStore.data.ambient.albums = ambientProvider.albums;
personalizationStore.data.ambient.topicSource = TopicSource.kArtGallery;
Expand Down Expand Up @@ -271,7 +272,7 @@ suite('AmbientPreviewLargeTest', function() {
async () => {
// Enable `isAmbientModeManaged` to mock an enterprise controlled user.
loadTimeData.overrideValues({
isAmbientSubpageUiChangeEnabled: true,
isPersonalizationJellyEnabled: true,
isAmbientModeManaged: true,
});

Expand Down

0 comments on commit 14ec85e

Please sign in to comment.